From 43acdf98594922c6e0cf9c05f06ff276f2f3755d Mon Sep 17 00:00:00 2001 From: Vitek Karas <10670590+vitek-karas@users.noreply.github.com> Date: Wed, 15 Mar 2023 02:38:29 -0700 Subject: [PATCH] Produce IL3000 and IL3001 from NativeAOT compiler (#83253) This also implements the suppression of the IL3002 on the affected methods both in the NativeAOT compiler and the analyzer. Adds single-file specific tests which are skipped for the trimmer. Fixes: https://github.com/dotnet/runtime/issues/83088 Fixes: https://github.com/dotnet/runtime/issues/82475 --- eng/testing/tests.singlefile.targets | 2 +- .../Dataflow/ReflectionMethodBodyScanner.cs | 21 +++++ .../TestCases/TestDatabase.cs | 5 + .../Mono.Linker.Tests/TestCases/TestSuites.cs | 7 ++ .../RequiresAssemblyFilesAnalyzer.cs | 16 +++- .../ILLink.Shared/TrimAnalysis/IntrinsicId.cs | 21 +++++ .../ILLink.Shared/TrimAnalysis/Intrinsics.cs | 23 +++++ .../ReflectionMethodBodyScanner.cs | 7 +- .../RequiresAssemblyFilesAnalyzerTests.cs | 2 - .../SingleFileTests.g.cs | 19 ++++ .../SingleFile/SingleFileIntrinsics.cs | 94 +++++++++++++++++++ .../TestCases/TestDatabase.cs | 5 + .../Mono.Linker.Tests/TestCases/TestSuites.cs | 6 ++ 13 files changed, 221 insertions(+), 7 deletions(-) create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SingleFileTests.g.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/SingleFile/SingleFileIntrinsics.cs diff --git a/eng/testing/tests.singlefile.targets b/eng/testing/tests.singlefile.targets index 2252731eae8..6bbbcc7834f 100644 --- a/eng/testing/tests.singlefile.targets +++ b/eng/testing/tests.singlefile.targets @@ -29,7 +29,7 @@ $(CoreCLRAotSdkDir) $(NetCoreAppCurrentTestHostSharedFrameworkPath) $(NetCoreAppCurrentTestHostSharedFrameworkPath) - $(NoWarn);IL1005;IL3002;IL3003 + $(NoWarn);IL1005;IL3000;IL3001;IL3002;IL3003 partial true true diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs index 6e559385d27..db0fdc4a645 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs @@ -533,6 +533,27 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet } break; + // + // string System.Reflection.Assembly.Location getter + // string System.Reflection.AssemblyName.CodeBase getter + // string System.Reflection.AssemblyName.EscapedCodeBase getter + // + case IntrinsicId.Assembly_get_Location: + case IntrinsicId.AssemblyName_get_CodeBase: + case IntrinsicId.AssemblyName_get_EscapedCodeBase: + diagnosticContext.AddDiagnostic(DiagnosticId.AvoidAssemblyLocationInSingleFile, calledMethod.GetDisplayName()); + break; + + // + // string System.Reflection.Assembly.GetFile(string) + // string System.Reflection.Assembly.GetFiles() + // string System.Reflection.Assembly.GetFiles(bool) + // + case IntrinsicId.Assembly_GetFile: + case IntrinsicId.Assembly_GetFiles: + diagnosticContext.AddDiagnostic(DiagnosticId.AvoidAssemblyGetFilesInSingleFile, calledMethod.GetDisplayName()); + break; + default: throw new NotImplementedException("Unhandled intrinsic"); } diff --git a/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestDatabase.cs b/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestDatabase.cs index ec395f21795..fc9b8f26650 100644 --- a/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestDatabase.cs +++ b/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestDatabase.cs @@ -44,6 +44,11 @@ public static IEnumerable RequiresCapability () return TestNamesBySuiteName (); } + public static IEnumerable SingleFile () + { + return TestNamesBySuiteName (); + } + public static IEnumerable Warnings () { return TestNamesBySuiteName (); diff --git a/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestSuites.cs b/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestSuites.cs index 4fe12fa6bac..add1386c9ba 100644 --- a/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestSuites.cs +++ b/src/coreclr/tools/aot/Mono.Linker.Tests/TestCases/TestSuites.cs @@ -51,6 +51,13 @@ public void RequiresCapability(string t) Run(t); } + [Theory] + [MemberData (nameof (TestDatabase.SingleFile), MemberType = typeof (TestDatabase))] + public void SingleFile (string t) + { + Run (t); + } + [Theory] [MemberData (nameof (TestDatabase.Warnings), MemberType = typeof (TestDatabase))] public void Warnings (string t) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs index d739bcc02f3..1b6a712f5d9 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs @@ -80,9 +80,19 @@ protected override ImmutableArray GetSpecialIncompatibleMembers (Compil protected override bool ReportSpecialIncompatibleMembersDiagnostic (OperationAnalysisContext operationContext, ImmutableArray dangerousPatterns, ISymbol member) { - if (member is IMethodSymbol && ImmutableArrayOperations.Contains (dangerousPatterns, member, SymbolEqualityComparer.Default)) { - operationContext.ReportDiagnostic (Diagnostic.Create (s_getFilesRule, operationContext.Operation.Syntax.GetLocation (), member.GetDisplayName ())); - return true; + if (member is IMethodSymbol method) { + if (ImmutableArrayOperations.Contains (dangerousPatterns, member, SymbolEqualityComparer.Default)) { + operationContext.ReportDiagnostic (Diagnostic.Create (s_getFilesRule, operationContext.Operation.Syntax.GetLocation (), member.GetDisplayName ())); + return true; + } + else if (method.AssociatedSymbol is ISymbol associatedSymbol && + ImmutableArrayOperations.Contains (dangerousPatterns, associatedSymbol, SymbolEqualityComparer.Default)) { + // The getters for CodeBase and EscapedCodeBase have RAF attribute on them + // so our caller will produce the RAF warning (IL3002) by default. Since we handle these properties specifically + // here and produce different warning (IL3000) we don't want the caller to produce IL3002. + // So we need to return true from here for the getters, to suppress the RAF warning. + return true; + } } else if (member is IPropertySymbol && ImmutableArrayOperations.Contains (dangerousPatterns, member, SymbolEqualityComparer.Default)) { operationContext.ReportDiagnostic (Diagnostic.Create (s_locationRule, operationContext.Operation.Syntax.GetLocation (), member.GetDisplayName ())); return true; diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs index 700ed6a391b..4392397cabf 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/IntrinsicId.cs @@ -288,6 +288,27 @@ internal enum IntrinsicId /// Assembly_CreateInstance, /// + /// + /// + Assembly_get_Location, + /// + /// + /// + Assembly_GetFile, + /// + /// + /// + /// + Assembly_GetFiles, + /// + /// + /// + AssemblyName_get_CodeBase, + /// + /// + /// + AssemblyName_get_EscapedCodeBase, + /// /// /// RuntimeReflectionExtensions_GetRuntimeEvent, diff --git a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs index fa71f747276..42c7d00d9d2 100644 --- a/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs +++ b/src/tools/illink/src/ILLink.Shared/TrimAnalysis/Intrinsics.cs @@ -356,6 +356,29 @@ public static IntrinsicId GetIntrinsicIdForMethod (MethodProxy calledMethod) && calledMethod.HasParameterOfType ((ParameterIndex) 1, "System.String") => IntrinsicId.Assembly_CreateInstance, + // System.Reflection.Assembly.Location getter + "get_Location" when calledMethod.IsDeclaredOnType ("System.Reflection.Assembly") + => IntrinsicId.Assembly_get_Location, + + // System.Reflection.Assembly.GetFile (string) + "GetFile" when calledMethod.IsDeclaredOnType ("System.Reflection.Assembly") + && calledMethod.HasParameterOfType ((ParameterIndex) 1, "System.String") + => IntrinsicId.Assembly_GetFile, + + // System.Reflection.Assembly.GetFiles () + // System.Reflection.Assembly.GetFiles (bool) + "GetFiles" when calledMethod.IsDeclaredOnType ("System.Reflection.Assembly") + && (calledMethod.HasMetadataParametersCount (0) || calledMethod.HasParameterOfType ((ParameterIndex) 1, "System.Boolean")) + => IntrinsicId.Assembly_GetFiles, + + // System.Reflection.AssemblyName.CodeBase getter + "get_CodeBase" when calledMethod.IsDeclaredOnType ("System.Reflection.AssemblyName") + => IntrinsicId.AssemblyName_get_CodeBase, + + // System.Reflection.AssemblyName.EscapedCodeBase getter + "get_EscapedCodeBase" when calledMethod.IsDeclaredOnType ("System.Reflection.AssemblyName") + => IntrinsicId.AssemblyName_get_EscapedCodeBase, + // System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor (RuntimeTypeHandle type) "RunClassConstructor" when calledMethod.IsDeclaredOnType ("System.Runtime.CompilerServices.RuntimeHelpers") && calledMethod.HasParameterOfType ((ParameterIndex) 0, "System.RuntimeTypeHandle") diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 1c77240c93c..0b357b56330 100644 --- a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -269,6 +269,11 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c case IntrinsicId.Marshal_PtrToStructure: case IntrinsicId.Marshal_DestroyStructure: case IntrinsicId.Marshal_GetDelegateForFunctionPointer: + case IntrinsicId.Assembly_get_Location: + case IntrinsicId.Assembly_GetFile: + case IntrinsicId.Assembly_GetFiles: + case IntrinsicId.AssemblyName_get_CodeBase: + case IntrinsicId.AssemblyName_get_EscapedCodeBase: // These intrinsics are not interesting for trimmer (they are interesting for AOT and that's why they are recognized) break; @@ -339,7 +344,7 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c // For all other cases, the trimming tools would have already produced a warning. default: - throw new NotImplementedException ("Unhandled intrinsic"); + throw new NotImplementedException ($"Unhandled intrinsic: {intrinsicId}"); } // If we get here, we handled this as an intrinsic. As a convenience, if the code above diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs index 89c8f461e3f..ecea646eb56 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs @@ -291,8 +291,6 @@ public void M() DiagnosticResult.CompilerWarning ("SYSLIB0044").WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.AssemblyName.CodeBase", "AssemblyName.CodeBase and AssemblyName.EscapedCodeBase are obsolete. Using them for loading an assembly is not supported."), // (8,7): warning SYSLIB0044: 'AssemblyName.EscapedCodeBase' is obsolete: 'AssemblyName.CodeBase and AssemblyName.EscapedCodeBase are obsolete. Using them for loading an assembly is not supported.' DiagnosticResult.CompilerWarning ("SYSLIB0044").WithSpan (8, 7, 8, 24).WithArguments ("System.Reflection.AssemblyName.EscapedCodeBase", "AssemblyName.CodeBase and AssemblyName.EscapedCodeBase are obsolete. Using them for loading an assembly is not supported."), - // (7,7): warning IL3002: Using member 'System.Reflection.AssemblyName.CodeBase.get' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. The code will return an empty string for assemblies embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.AssemblyName.CodeBase.get", " The code will return an empty string for assemblies embedded in a single-file app.", ""), // (7,7): warning IL3000: 'System.Reflection.AssemblyName.CodeBase' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.AssemblyName.CodeBase"), // (8,7): warning IL3000: 'System.Reflection.AssemblyName.EscapedCodeBase' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SingleFileTests.g.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SingleFileTests.g.cs new file mode 100644 index 00000000000..9c63f465e88 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/SingleFileTests.g.cs @@ -0,0 +1,19 @@ +using System; +using System.Threading.Tasks; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + public sealed partial class SingleFileTests : LinkerTestBase + { + + protected override string TestSuiteName => "SingleFile"; + + [Fact] + public Task SingleFileIntrinsics () + { + return RunTest (allowMissingWarnings: true); + } + + } +} \ No newline at end of file diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/SingleFile/SingleFileIntrinsics.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/SingleFile/SingleFileIntrinsics.cs new file mode 100644 index 00000000000..1a3b8b4284d --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/SingleFile/SingleFileIntrinsics.cs @@ -0,0 +1,94 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Diagnostics.CodeAnalysis; +using Mono.Linker.Tests.Cases.Expectations.Assertions; + +namespace Mono.Linker.Tests.Cases.SingleFile +{ + [IgnoreTestCase ("Ignore in illink since it doesn't implement any single-file related functionality", IgnoredBy = Tool.Trimmer)] + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + public class SingleFileIntrinsics + { + // Some of the test methods have RAF on them, it's not the point of this test to verify that behavior + [UnconditionalSuppressMessage("test", "IL3002")] + public static void Main () + { + TestAssemblyLocation (); + TestAssemblyLocationSuppressedByRAF (); + TestAssemblyNameCodeBase (); + TestAssemblyNameCodeBaseSuppressedByRAF (); + TestAssemblyNameEscapedCodeBase (); + TestAssemblyNameEscapedCodeBaseSuppressedByRAF (); + TestAssemblyGetFile (); + TestAssemblyGetFileSuppressedByRAF (); + TestAssemblyGetFiles (); + TestAssemblyGetFilesSuppressedByRAF (); + } + + [ExpectedWarning("IL3000", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestAssemblyLocation() + { + var a = typeof (SingleFileIntrinsics).Assembly.Location; + } + + [RequiresAssemblyFiles("test")] + static void TestAssemblyLocationSuppressedByRAF() + { + var a = typeof (SingleFileIntrinsics).Assembly.Location; + } + + [ExpectedWarning ("IL3000", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestAssemblyNameCodeBase() + { + var a = typeof (SingleFileIntrinsics).Assembly.GetName ().CodeBase; + } + + [RequiresAssemblyFiles ("test")] + static void TestAssemblyNameCodeBaseSuppressedByRAF () + { + var a = typeof (SingleFileIntrinsics).Assembly.GetName ().CodeBase; + } + + [ExpectedWarning ("IL3000", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestAssemblyNameEscapedCodeBase () + { + var a = typeof (SingleFileIntrinsics).Assembly.GetName ().EscapedCodeBase; + } + + [RequiresAssemblyFiles ("test")] + static void TestAssemblyNameEscapedCodeBaseSuppressedByRAF () + { + var a = typeof (SingleFileIntrinsics).Assembly.GetName ().EscapedCodeBase; + } + + [ExpectedWarning ("IL3001", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestAssemblyGetFile() + { + var a = typeof (SingleFileIntrinsics).Assembly.GetFile ("unknown"); + } + + [RequiresAssemblyFiles ("test")] + static void TestAssemblyGetFileSuppressedByRAF () + { + var a = typeof (SingleFileIntrinsics).Assembly.GetFile ("unknown"); + } + + [ExpectedWarning ("IL3001", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + [ExpectedWarning ("IL3001", ProducedBy = Tool.Analyzer | Tool.NativeAot)] + static void TestAssemblyGetFiles () + { + var a = typeof (SingleFileIntrinsics).Assembly.GetFiles (); + a = typeof (SingleFileIntrinsics).Assembly.GetFiles (true); + } + + [RequiresAssemblyFiles ("test")] + static void TestAssemblyGetFilesSuppressedByRAF () + { + var a = typeof (SingleFileIntrinsics).Assembly.GetFiles (); + a = typeof (SingleFileIntrinsics).Assembly.GetFiles (true); + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestDatabase.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestDatabase.cs index 6eec8ce77a7..855bc8c9606 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestDatabase.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestDatabase.cs @@ -174,6 +174,11 @@ public static IEnumerable SerializationTests () return NUnitCasesBySuiteName ("Serialization"); } + public static IEnumerable SingleFileTests () + { + return NUnitCasesBySuiteName ("SingleFile"); + } + public static IEnumerable StaticsTests () { return NUnitCasesBySuiteName ("Statics"); diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestSuites.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestSuites.cs index 91e174cebed..32f454063a1 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestSuites.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCases/TestSuites.cs @@ -208,6 +208,12 @@ public void SerializationTests (TestCase testCase) Run (testCase); } + [TestCaseSource (typeof (TestDatabase), nameof (TestDatabase.SingleFileTests))] + public void SingleFileTests (TestCase testCase) + { + Run (testCase); + } + [TestCaseSource (typeof (TestDatabase), nameof (TestDatabase.StaticsTests))] public void StaticsTests (TestCase testCase) { -- GitLab