Resolving ILLink warnings on System.Private.Xml (Part 1) (#49413)

* Resolving ILLink warnings on System.Private.Xml (Part 1) * Only annotate the unsafe Load overload methods from XslCompiledTransform type * Address feedback and fix one more warning * Update src/libraries/System.Private.Xml/src/System/Xml/Xsl/Runtime/EarlyBoundInfo.cs Co-authored-by: Eric Erhardt <[email protected]> * PR Feedback Co-authored-by: Eric Erhardt <[email protected]>
dotnet · Mar 19, 2021 · fbb6a1c · fbb6a1c
1 parent ded66fc
commit fbb6a1c
Show file tree

Hide file tree

Showing 13 changed files with 106 additions and 53 deletions.
diff --git a/src/libraries/System.Private.Xml/src/ILLink/ILLink.Suppressions.xml b/src/libraries/System.Private.Xml/src/ILLink/ILLink.Suppressions.xml
@@ -139,18 +139,6 @@
       <property name="Scope">member</property>
       <property name="Target">M:System.Xml.Serialization.XmlReflectionImporter.GetMethodFromSchemaProvider(System.Xml.Serialization.XmlSchemaProviderAttribute,System.Type)</property>
     </attribute>
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2070</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.Xml.Xsl.Runtime.EarlyBoundInfo.#ctor(System.String,System.Type)</property>
-    </attribute>
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2070</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.Xml.Xsl.XslCompiledTransform.Load(System.Type)</property>
-    </attribute>
     <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
       <argument>ILLink</argument>
       <argument>IL2072</argument>
@@ -175,12 +163,6 @@
       <property name="Scope">member</property>
       <property name="Target">M:System.Xml.Serialization.XmlSerializationILGen.GenerateTypedSerializer(System.String,System.String,System.Xml.Serialization.XmlMapping,System.Xml.Serialization.CodeIdentifiers,System.String,System.String,System.String)</property>
     </attribute>
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2072</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.Xml.Xsl.XsltOld.Processor.#ctor(System.Xml.XPath.XPathNavigator,System.Xml.Xsl.XsltArgumentList,System.Xml.XmlResolver,System.Xml.Xsl.XsltOld.Stylesheet,System.Collections.Generic.List{System.Xml.Xsl.XsltOld.TheQuery},System.Xml.Xsl.XsltOld.RootAction,System.Xml.Xsl.XsltOld.Debugger.IXsltDebugger)</property>
-    </attribute>
     <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
       <argument>ILLink</argument>
       <argument>IL2075</argument>
@@ -319,18 +301,6 @@
       <property name="Scope">member</property>
       <property name="Target">M:System.Xml.Serialization.XmlSerializationWriterILGen.WriteElement(System.Xml.Serialization.SourceInfo,System.Xml.Serialization.ElementAccessor,System.String,System.Boolean)</property>
     </attribute>
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2075</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.Xml.Xsl.IlGen.XmlILModule.BakeMethods</property>
-    </attribute>
-    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
-      <argument>ILLink</argument>
-      <argument>IL2075</argument>
-      <property name="Scope">member</property>
-      <property name="Target">M:System.Xml.Xsl.XsltOld.XsltCompileContext.GetExtentionMethod(System.String,System.String,System.Xml.XPath.XPathResultType[],System.Object@)</property>
-    </attribute>
     <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
       <argument>ILLink</argument>
       <argument>IL2077</argument>
@@ -345,15 +315,21 @@
     </attribute>
     <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
       <argument>ILLink</argument>
-      <argument>IL2080</argument>
+      <argument>IL2067</argument>
       <property name="Scope">member</property>
-      <property name="Target">M:System.Xml.Xsl.Runtime.XmlExtensionFunction.Bind</property>
+      <property name="Target">M:System.Xml.Xsl.Runtime.XmlExtensionFunction.#ctor(System.String,System.String,System.Int32,System.Type,System.Reflection.BindingFlags)</property>
     </attribute>
     <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
       <argument>ILLink</argument>
-      <argument>IL2080</argument>
+      <argument>IL2067</argument>
       <property name="Scope">member</property>
-      <property name="Target">M:System.Xml.Xsl.Runtime.XmlExtensionFunction.CanBind</property>
+      <property name="Target">M:System.Xml.Xsl.Runtime.XmlExtensionFunctionTable.Bind(System.String,System.String,System.Int32,System.Type,System.Reflection.BindingFlags)</property>
+    </attribute>
+    <attribute fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
+      <argument>ILLink</argument>
+      <argument>IL2075</argument>
+      <property name="Scope">member</property>
+      <property name="Target">M:System.Xml.Xsl.XsltOld.XsltCompileContext.GetExtentionMethod(System.String,System.String,System.Xml.XPath.XPathResultType[],System.Object@)</property>
     </attribute>
   </assembly>
 </linker>
diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/IlGen/StaticDataManager.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/IlGen/StaticDataManager.cs
@@ -4,6 +4,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
 using System.Xml.Xsl.Qil;
 using System.Xml.Xsl.Runtime;
@@ -172,7 +173,7 @@ public string[]? GlobalNames
         /// Add early bound information to a list that is used by this query.  Return the index of
         /// the early bound information in the list.
         /// </summary>
-        public int DeclareEarlyBound(string namespaceUri, Type ebType)
+        public int DeclareEarlyBound(string namespaceUri, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] Type ebType)
         {
             if (_earlyInfo == null)
                 _earlyInfo = new UniqueList<EarlyBoundInfo>();

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/IlGen/XmlIlVisitor.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/IlGen/XmlIlVisitor.cs
@@ -35,6 +35,9 @@ internal class XmlILVisitor : QilVisitor
         private IteratorDescriptor? _iterNested;
         private int _indexId;
 
+        [RequiresUnreferencedCode("Method VisitXsltInvokeEarlyBound will require code that cannot be statically analyzed.")]
+        public XmlILVisitor()
+        { }
 
         //-----------------------------------------------
         // Entry
@@ -3597,6 +3600,9 @@ protected override QilNode VisitXsltInvokeLateBound(QilInvokeLateBound ndInvoke)
         /// <summary>
         /// Generate code for QilNodeType.XsltInvokeEarlyBound.
         /// </summary>
+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:RequiresUnreferencedCode",
+            Justification = "Supressing warning about not having the RequiresUnreferencedCode attribute since we added " +
+            "the attribute to this subclass' constructor. This allows us to not have to annotate the whole QilNode hirerarchy.")]
         protected override QilNode VisitXsltInvokeEarlyBound(QilInvokeEarlyBound ndInvoke)
         {
             QilName ndName = ndInvoke.Name;

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Runtime/EarlyBoundInfo.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Runtime/EarlyBoundInfo.cs
@@ -3,6 +3,7 @@
 
 #nullable disable
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
 
 namespace System.Xml.Xsl.Runtime
@@ -14,13 +15,16 @@ internal sealed class EarlyBoundInfo
     {
         private readonly string _namespaceUri;            // Namespace Uri mapped to these early bound functions
         private readonly ConstructorInfo _constrInfo;     // Constructor for the early bound function object
+        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
+        private readonly Type _ebType;
 
-        public EarlyBoundInfo(string namespaceUri, Type ebType)
+        public EarlyBoundInfo(string namespaceUri, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] Type ebType)
         {
             Debug.Assert(namespaceUri != null && ebType != null);
 
             // Get the default constructor
             _namespaceUri = namespaceUri;
+            _ebType = ebType;
             _constrInfo = ebType.GetConstructor(Type.EmptyTypes);
             Debug.Assert(_constrInfo != null, "The early bound object type " + ebType.FullName + " must have a public default constructor");
         }
@@ -33,7 +37,11 @@ public EarlyBoundInfo(string namespaceUri, Type ebType)
         /// <summary>
         /// Return the Clr Type of the early bound object.
         /// </summary>
-        public Type EarlyBoundType { get { return _constrInfo.DeclaringType; } }
+        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
+        public Type EarlyBoundType
+        {
+            get { return _ebType; }
+        }
 
         /// <summary>
         /// Create an instance of the early bound object.

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Runtime/XmlExtensionFunction.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Runtime/XmlExtensionFunction.cs
@@ -9,6 +9,7 @@
 using System.Reflection;
 using System.Globalization;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 
 namespace System.Xml.Xsl.Runtime
 {
@@ -57,6 +58,7 @@ internal class XmlExtensionFunction
         private string _namespaceUri;                // Extension object identifier
         private string _name;                        // Name of this method
         private int _numArgs;                        // Argument count
+        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)]
         private Type _objectType;                    // Type of the object which will be searched for matching methods
         private BindingFlags _flags;                 // Modifiers that were used to search for a matching signature
         private int _hashCode;                       // Pre-computed hashcode
@@ -95,7 +97,7 @@ public XmlExtensionFunction(string name, string namespaceUri, int numArgs, Type
         /// <summary>
         /// Initialize, but do not bind.
         /// </summary>
-        public void Init(string name, string namespaceUri, int numArgs, Type objectType, BindingFlags flags)
+        public void Init(string name, string namespaceUri, int numArgs, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicMethods | DynamicallyAccessedMemberTypes.PublicMethods)] Type objectType, BindingFlags flags)
         {
             _name = name;
             _namespaceUri = namespaceUri;

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Runtime/XmlQueryStaticData.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Runtime/XmlQueryStaticData.cs
@@ -4,6 +4,7 @@
 #nullable disable
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Xml.Xsl.IlGen;
 using System.Xml.Xsl.Qil;
@@ -35,6 +36,7 @@ internal class XmlQueryStaticData
         /// <summary>
         /// Constructor.
         /// </summary>
+        [RequiresUnreferencedCode("This method will create a copy that uses earlybound types which cannot be statically analyzed.")]
         public XmlQueryStaticData(XmlWriterSettings defaultWriterSettings, IList<WhitespaceRule> whitespaceRules, StaticDataManager staticData)
         {
             Debug.Assert(defaultWriterSettings != null && staticData != null);
@@ -70,6 +72,7 @@ public XmlQueryStaticData(XmlWriterSettings defaultWriterSettings, IList<Whitesp
         /// <summary>
         /// Deserialize XmlQueryStaticData object from a byte array.
         /// </summary>
+        [RequiresUnreferencedCode("This method will create EarlyBoundInfo from passed in ebTypes array which cannot be statically analyzed.")]
         public XmlQueryStaticData(byte[] data, Type[] ebTypes)
         {
             MemoryStream dataStream = new MemoryStream(data, /*writable:*/false);

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/XmlIlGenerator.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/XmlIlGenerator.cs
@@ -70,6 +70,10 @@ public XmlILGenerator()
         // SxS Note: The way the trace file names are created (hardcoded) is NOT SxS safe. However the files are
         // created only for internal tracing purposes. In addition XmlILTrace class is not compiled into retail
         // builds. As a result it is fine to suppress the FxCop SxS warning.
+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
+            Justification = "This method will generate the IL methods using RefEmit at runtime, which will then try to call them " +
+            "using methods that are annotated as RequiresUnreferencedCode. In this case, these uses can be suppressed as the " +
+            "trimmer won't be able to trim any IL that gets generated at runtime.")]
         public XmlILCommand? Generate(QilExpression query, TypeBuilder? typeBldr)
         {
             _qil = query;

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Xslt/QilGenerator.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Xslt/QilGenerator.cs
@@ -196,13 +196,14 @@ private QilExpression Compile(Compiler compiler)
             }
 
             // Create list of all early bound objects
-            Dictionary<string, Type?> scriptClasses = compiler.Scripts.ScriptClasses;
+            Scripts.TrimSafeDictionary scriptClasses = compiler.Scripts.ScriptClasses;
             List<EarlyBoundInfo> ebTypes = new List<EarlyBoundInfo>(scriptClasses.Count);
-            foreach (KeyValuePair<string, Type?> pair in scriptClasses)
+            foreach (string key in scriptClasses.Keys)
             {
-                if (pair.Value != null)
+                Type? value = scriptClasses[key];
+                if (value != null)
                 {
-                    ebTypes.Add(new EarlyBoundInfo(pair.Key, pair.Value));
+                    ebTypes.Add(new EarlyBoundInfo(key, value));
                 }
             }
 

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Xslt/Scripts.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/Xslt/Scripts.cs
@@ -4,23 +4,25 @@
 // <spec>http://devdiv/Documents/Whidbey/CLR/CurrentSpecs/BCL/CodeDom%20Activation.doc</spec>
 //------------------------------------------------------------------------------
 
+using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Xml.Xsl.Runtime;
 
 namespace System.Xml.Xsl.Xslt
 {
     internal class Scripts
     {
         private readonly Compiler _compiler;
-        private readonly Dictionary<string, Type?> _nsToType = new Dictionary<string, Type?>();
+        private readonly TrimSafeDictionary _nsToType = new TrimSafeDictionary();
         private readonly XmlExtensionFunctionTable _extFuncs = new XmlExtensionFunctionTable();
 
         public Scripts(Compiler compiler)
         {
             _compiler = compiler;
         }
 
-        public Dictionary<string, Type?> ScriptClasses
+        public TrimSafeDictionary ScriptClasses
         {
             get { return _nsToType; }
         }
@@ -41,5 +43,29 @@ public Scripts(Compiler compiler)
             }
             return null;
         }
+
+        internal class TrimSafeDictionary
+        {
+            private readonly Dictionary<string, Type?> _backingDictionary = new Dictionary<string, Type?>();
+
+            public Type? this[string key]
+            {
+                [UnconditionalSuppressMessage("TrimAnalysis", "IL2073:MissingDynamicallyAccessedMembers",
+                    Justification = "The getter of the dictionary is not annotated to preserve the constructor, but the sources that are adding the items to " +
+                    "the dictionary are annotated so we can supress the message as we know the constructor will be preserved.")]
+                [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
+                get => _backingDictionary[key];
+                [param: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
+                set => _backingDictionary[key] = value;
+            }
+
+            public ICollection<string> Keys => _backingDictionary.Keys;
+
+            public int Count => _backingDictionary.Count;
+
+            public bool ContainsKey(string key) => _backingDictionary.ContainsKey(key);
+
+            public bool TryGetValue(string key, [MaybeNullWhen(false)] out Type? value) => _backingDictionary.TryGetValue(key, out value);
+        }
     }
 }
diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xsl/XsltOld/Processor.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xsl/XsltOld/Processor.cs
@@ -371,15 +371,10 @@ public Processor(
 
             _scriptExtensions = new Hashtable(_stylesheet.ScriptObjectTypes.Count);
             {
-                foreach (DictionaryEntry entry in _stylesheet.ScriptObjectTypes)
+                // Scripts are not supported on stylesheets
+                if (_stylesheet.ScriptObjectTypes.Count > 0)
                 {
-                    string namespaceUri = (string)entry.Key;
-                    if (GetExtensionObject(namespaceUri) != null)
-                    {
-                        throw XsltException.Create(SR.Xslt_ScriptDub, namespaceUri);
-                    }
-                    _scriptExtensions.Add(namespaceUri, Activator.CreateInstance((Type)entry.Value!,
-                        BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, null, null));
+                    throw new PlatformNotSupportedException(SR.CompilingScriptsNotSupported);
                 }
             }
 

diff --git a/src/libraries/System.Private.Xml/src/System/Xml/Xslt/XslCompiledTransform.cs b/src/libraries/System.Private.Xml/src/System/Xml/Xslt/XslCompiledTransform.cs
@@ -194,7 +194,7 @@ private void CompileQilToMsil(XsltSettings settings)
         //------------------------------------------------
         // Load compiled stylesheet from a Type
         //------------------------------------------------
-
+        [RequiresUnreferencedCode("This method will get fields and types from the assembly of the passed in compiledStylesheet and call their constructors which cannot be statically analyzed")]
         public void Load(Type compiledStylesheet)
         {
             Reset();
@@ -238,6 +238,7 @@ public void Load(Type compiledStylesheet)
                 throw new ArgumentException(SR.Format(SR.Xslt_NotCompiledStylesheet, compiledStylesheet.FullName), nameof(compiledStylesheet));
         }
 
+        [RequiresUnreferencedCode("This method will call into constructors of the earlyBoundTypes array which cannot be statically analyzed.")]
         public void Load(MethodInfo executeMethod, byte[] queryData, Type[]? earlyBoundTypes)
         {
             Reset();

diff --git a/src/libraries/System.Private.Xml/tests/Xslt/XslTransformApi/CXslTransform.cs b/src/libraries/System.Private.Xml/tests/Xslt/XslTransformApi/CXslTransform.cs
@@ -871,6 +871,34 @@ public void LoadGeneric12(InputType inputType, ReaderType readerType)
             _output.WriteLine("Did not throw compile exception for stylesheet");
             Assert.True(false);
         }
+
+        [Fact]
+        public void XslTransformThrowsPNSEWhenUsingScripts()
+        {
+            using StringReader xslFile = new StringReader(
+        @"<xsl:stylesheet version=""1.0"" xmlns:xsl=""http://www.w3.org/1999/XSL/Transform""
+  xmlns:msxsl=""urn:schemas-microsoft-com:xslt""
+  xmlns:user=""urn:my-scripts"">
+  <msxsl:script language=""C#"" implements-prefix=""user"">
+    <![CDATA[
+  public double modifyPrice(double price){
+    price*=0.9;
+    return price;
+  }
+  ]]>
+  </msxsl:script>
+  <xsl:template match=""Root"">
+    <Root xmlns="""">
+      <Price><xsl:value-of select=""user:modifyPrice(Price)""/></Price>
+    </Root>
+  </xsl:template>
+</xsl:stylesheet>");
+
+            using XmlReader reader = XmlReader.Create(xslFile); 
+            XslTransform xslt = new XslTransform();
+            XsltCompileException compilationException = Assert.Throws<XsltCompileException>(() => xslt.Load(reader));
+            Assert.True(compilationException.InnerException != null && compilationException.InnerException is PlatformNotSupportedException);
+        }
     }
 
     /**************************************************************************/

diff --git a/src/libraries/System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs b/src/libraries/System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs
@@ -2787,9 +2787,11 @@ public sealed partial class XslCompiledTransform
         public XslCompiledTransform() { }
         public XslCompiledTransform(bool enableDebug) { }
         public System.Xml.XmlWriterSettings? OutputSettings { get { throw null; } }
+        [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method will call into constructors of the earlyBoundTypes array which cannot be statically analyzed.")]
         public void Load(System.Reflection.MethodInfo executeMethod, byte[] queryData, System.Type[]? earlyBoundTypes) { }
         public void Load(string stylesheetUri) { }
         public void Load(string stylesheetUri, System.Xml.Xsl.XsltSettings? settings, System.Xml.XmlResolver? stylesheetResolver) { }
+        [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method will get fields and types from the assembly of the passed in compiledStylesheet and call their constructors which cannot be statically analyzed")]
         public void Load(System.Type compiledStylesheet) { }
         public void Load(System.Xml.XmlReader stylesheet) { }
         public void Load(System.Xml.XmlReader stylesheet, System.Xml.Xsl.XsltSettings? settings, System.Xml.XmlResolver? stylesheetResolver) { }