Implement checking for Requires* on attributes for NativeAOT (#83550)

Any of the RUC/RDC/RAF attributes can be added to an attribute (either the type or the .ctor or properties). The compiler needs to check for these and produce warnings if such attribute is instantiated somewhere. This was added to "Dataflow" existing class, while it's not exactly data flow, adding a new abstraction didn't seem worth it. Also does a simple dedupe of RUC/RDC/RAF checks in some places. Adapt the tests to the new behavior. The most notable change is that NativeAOT will only look at attributes for reflection enabled members, so had to modify the tests to reflection access basically everything. Use GetDisplayName for anything which is supported by that extension method - this fixes wrong origin printed out for property/event (we would print out 'PropertyPseudoDescriptor'). Mapping IL offset to source file/line number - if the PDB specifies that line as "hidden", then don't use it, instead look for the begining of the method - so first non-hidden sequence point in the method's body. This is a copy of the logic implemented in illink already.
dotnet · Mar 27, 2023 · 9b38f2a · 9b38f2a
1 parent eb044f1
commit 9b38f2a
Show file tree

Hide file tree

Showing 9 changed files with 150 additions and 57 deletions.
diff --git a/src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs b/src/coreclr/tools/Common/Compiler/DisplayNameHelpers.cs
@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
 using System.Diagnostics;
 using System.Text;
 
@@ -23,7 +22,7 @@ public static string GetDisplayName(this TypeSystemEntity entity)
                 PropertyPseudoDesc property => property.GetDisplayName(),
                 EventPseudoDesc @event => @event.GetDisplayName(),
 #endif
-                _ => throw new InvalidOperationException(),
+                _ => null,
             };
         }
 

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/AttributeDataFlow.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/AttributeDataFlow.cs
@@ -28,19 +28,29 @@ namespace ILCompiler.Dataflow
         private readonly NodeFactory _factory;
         private readonly FlowAnnotations _annotations;
         private readonly MessageOrigin _origin;
+        private readonly DiagnosticContext _diagnosticContext;
 
         public AttributeDataFlow(Logger logger, NodeFactory factory, FlowAnnotations annotations, in MessageOrigin origin)
         {
             _annotations = annotations;
             _factory = factory;
             _logger = logger;
             _origin = origin;
+
+            _diagnosticContext = new DiagnosticContext(
+                _origin,
+                _logger.ShouldSuppressAnalysisWarningsForRequires(_origin.MemberDefinition, DiagnosticUtilities.RequiresUnreferencedCodeAttribute),
+                _logger.ShouldSuppressAnalysisWarningsForRequires(_origin.MemberDefinition, DiagnosticUtilities.RequiresDynamicCodeAttribute),
+                _logger.ShouldSuppressAnalysisWarningsForRequires(_origin.MemberDefinition, DiagnosticUtilities.RequiresAssemblyFilesAttribute),
+                _logger);
         }
 
         public DependencyList? ProcessAttributeDataflow(MethodDesc method, CustomAttributeValue arguments)
         {
             DependencyList? result = null;
 
+            ReflectionMethodBodyScanner.CheckAndReportAllRequires(_diagnosticContext, method);
+
             // First do the dataflow for the constructor parameters if necessary.
             if (_annotations.RequiresDataflowAnalysisDueToSignature(method))
             {
@@ -62,6 +72,8 @@ public AttributeDataFlow(Logger logger, NodeFactory factory, FlowAnnotations ann
                     FieldDesc field = attributeType.GetField(namedArgument.Name);
                     if (field != null)
                     {
+                        ReflectionMethodBodyScanner.CheckAndReportAllRequires(_diagnosticContext, field);
+
                         ProcessAttributeDataflow(field, namedArgument.Value, ref result);
                     }
                 }
@@ -72,6 +84,8 @@ public AttributeDataFlow(Logger logger, NodeFactory factory, FlowAnnotations ann
                     MethodDesc setter = property.SetMethod;
                     if (setter != null && setter.Signature.Length > 0 && !setter.Signature.IsStatic)
                     {
+                        ReflectionMethodBodyScanner.CheckAndReportAllRequires(_diagnosticContext, setter);
+
                         ProcessAttributeDataflow(setter, ImmutableArray.Create(namedArgument.Value), ref result);
                     }
                 }
@@ -88,21 +102,19 @@ private void ProcessAttributeDataflow(MethodDesc method, ImmutableArray<object?>
                 if (parameterValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None)
                 {
                     MultiValue value = GetValueForCustomAttributeArgument(arguments[parameter.MetadataIndex]);
-                    var diagnosticContext = new DiagnosticContext(_origin, diagnosticsEnabled: true, _logger);
-                    RequireDynamicallyAccessedMembers(diagnosticContext, value, parameterValue, method.GetDisplayName(), ref result);
+                    RequireDynamicallyAccessedMembers(_diagnosticContext, value, parameterValue, method.GetDisplayName(), ref result);
                 }
             }
         }
 
-        public void ProcessAttributeDataflow(FieldDesc field, object? value, ref DependencyList? result)
+        private void ProcessAttributeDataflow(FieldDesc field, object? value, ref DependencyList? result)
         {
             var fieldValueCandidate = _annotations.GetFieldValue(field);
             if (fieldValueCandidate is ValueWithDynamicallyAccessedMembers fieldValue
                 && fieldValue.DynamicallyAccessedMemberTypes != DynamicallyAccessedMemberTypes.None)
             {
                 MultiValue valueNode = GetValueForCustomAttributeArgument(value);
-                var diagnosticContext = new DiagnosticContext(_origin, diagnosticsEnabled: true, _logger);
-                RequireDynamicallyAccessedMembers(diagnosticContext, valueNode, fieldValue, field.GetDisplayName(), ref result);
+                RequireDynamicallyAccessedMembers(_diagnosticContext, valueNode, fieldValue, field.GetDisplayName(), ref result);
             }
         }
 

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs
@@ -57,6 +57,13 @@ public static bool RequiresReflectionMethodBodyScannerForAccess(FlowAnnotations
                 field.DoesFieldRequire(DiagnosticUtilities.RequiresDynamicCodeAttribute, out _);
         }
 
+        internal static void CheckAndReportAllRequires(in DiagnosticContext diagnosticContext, TypeSystemEntity calledMember)
+        {
+            CheckAndReportRequires(diagnosticContext, calledMember, DiagnosticUtilities.RequiresUnreferencedCodeAttribute);
+            CheckAndReportRequires(diagnosticContext, calledMember, DiagnosticUtilities.RequiresDynamicCodeAttribute);
+            CheckAndReportRequires(diagnosticContext, calledMember, DiagnosticUtilities.RequiresAssemblyFilesAttribute);
+        }
+
         internal static void CheckAndReportRequires(in DiagnosticContext diagnosticContext, TypeSystemEntity calledMember, string requiresAttributeName)
         {
             if (!calledMember.DoesMemberRequire(requiresAttributeName, out var requiresAttribute))
@@ -358,9 +365,7 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet
                             }
                         }
 
-                        CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresUnreferencedCodeAttribute);
-                        CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresDynamicCodeAttribute);
-                        CheckAndReportRequires(diagnosticContext, calledMethod, DiagnosticUtilities.RequiresAssemblyFilesAttribute);
+                        CheckAndReportAllRequires(diagnosticContext, calledMethod);
 
                         return handleCallAction.Invoke(calledMethod, instanceValue, argumentValues, intrinsicId, out methodReturnValue);
                     }

diff --git a/...coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisFieldAccessPattern.cs b/...coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/TrimAnalysisFieldAccessPattern.cs
@@ -30,9 +30,7 @@ public void MarkAndProduceDiagnostics(ReflectionMarker reflectionMarker, Logger
                 logger.ShouldSuppressAnalysisWarningsForRequires(Origin.MemberDefinition, DiagnosticUtilities.RequiresAssemblyFilesAttribute),
                 logger);
 
-            ReflectionMethodBodyScanner.CheckAndReportRequires(diagnosticContext, Field, DiagnosticUtilities.RequiresUnreferencedCodeAttribute);
-            ReflectionMethodBodyScanner.CheckAndReportRequires(diagnosticContext, Field, DiagnosticUtilities.RequiresDynamicCodeAttribute);
-            ReflectionMethodBodyScanner.CheckAndReportRequires(diagnosticContext, Field, DiagnosticUtilities.RequiresAssemblyFilesAttribute);
+            ReflectionMethodBodyScanner.CheckAndReportAllRequires(diagnosticContext, Field);
         }
     }
 }
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logging/MessageContainer.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logging/MessageContainer.cs
@@ -300,11 +300,7 @@ public string ToMSBuildString()
 
             if (Origin?.MemberDefinition != null)
             {
-                if (Origin?.MemberDefinition is MethodDesc method)
-                    sb.Append(method.GetDisplayName());
-                else
-                    sb.Append(Origin?.MemberDefinition.ToString());
-
+                sb.Append(Origin?.MemberDefinition?.GetDisplayName() ?? Origin?.MemberDefinition?.ToString());
                 sb.Append(": ");
             }
 

diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logging/MessageOrigin.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logging/MessageOrigin.cs
@@ -25,6 +25,8 @@ public struct MessageOrigin :
         public int? SourceColumn { get; }
         public int? ILOffset { get; }
 
+        private const int HiddenLineNumber = 0xfeefee;
+
         public MessageOrigin(string fileName, int? sourceLine = null, int? sourceColumn = null)
         {
             FileName = fileName;
@@ -56,24 +58,38 @@ public MessageOrigin(string fileName, int sourceLine, int sourceColumn, ModuleDe
 
         public MessageOrigin(MethodIL methodBody, int ilOffset)
         {
-            string? document = null;
-            int? lineNumber = null;
-
+            ILSequencePoint? correspondingSequencePoint = null;
             IEnumerable<ILSequencePoint>? sequencePoints = methodBody.GetDebugInfo()?.GetSequencePoints();
             if (sequencePoints != null)
             {
                 foreach (var sequencePoint in sequencePoints)
                 {
                     if (sequencePoint.Offset <= ilOffset)
                     {
-                        document = sequencePoint.Document;
-                        lineNumber = sequencePoint.LineNumber;
+                        correspondingSequencePoint = sequencePoint;
+                    }
+                }
+
+                // If the warning comes from hidden line (compiler generated code typically)
+                // search for any sequence point with non-hidden line number and report that as a best effort.
+                if (correspondingSequencePoint?.LineNumber == HiddenLineNumber)
+                {
+                    // Reset the information as we don't want to use any of the info in the hidden sequence point
+                    correspondingSequencePoint = null;
+                    foreach (var sequencePoint in sequencePoints)
+                    {
+                        if (sequencePoint.LineNumber != HiddenLineNumber)
+                        {
+                            correspondingSequencePoint = sequencePoint;
+                            break;
+                        }
                     }
                 }
+
             }
-            FileName = document;
+            FileName = correspondingSequencePoint?.Document;
             MemberDefinition = methodBody.OwningMethod;
-            SourceLine = lineNumber;
+            SourceLine = correspondingSequencePoint?.LineNumber;
             SourceColumn = null;
             ILOffset = ilOffset;
         }