From 84e8574d109c0ac0603e286756cf82277a5eb7f0 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Thu, 27 Oct 2022 13:26:40 -0700 Subject: [PATCH 1/5] Fix branch removal in compiler generated code Changes to processing of compiler generated methods lead to a state where we don't call constant prop and branch removal in all cases before we mark instructions of the method. This can lead to overmarking This change fixes this by making sure that the branch removal executes on the method in all cases before we mark instructions of the method. To make the code simple I added a hashset to prevent multiple optimization runs on the same method. Technically this can be guaranteed by the caller, but it's really complex and very fragile. Added tests. Note that there's still a whole in analysis of compiler generated code around state machines, see https://github.com/dotnet/linker/issues/3087 --- src/linker/Linker.Steps/MarkStep.cs | 4 + .../UnreachableBlocksOptimizer.cs | 4 + .../CompilerGeneratedCodeSubstitutions.cs | 246 ++++++++++++++++++ 3 files changed, 254 insertions(+) create mode 100644 test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 5d4065d35dd9..ede3b5efbbc3 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -3543,6 +3543,10 @@ bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodBody body) if (_compilerGeneratedMethodRequiresScanner.TryGetValue (body, out bool requiresReflectionMethodBodyScanner)) return requiresReflectionMethodBodyScanner; + // Make sure the method's body is processed, for compiler generated code we can get here before ProcessMethod is called + // and thus we need to make sure we operate on the optimized method body (to avoid marking code which is otherwise optimized away). + UnreachableBlocksOptimizer.ProcessMethod (body.Method); + foreach (VariableDefinition var in body.Variables) MarkType (var.VariableType, new DependencyInfo (DependencyKind.VariableType, body.Method)); diff --git a/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs b/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs index d6bc01798566..08484841f87a 100644 --- a/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs +++ b/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs @@ -24,6 +24,7 @@ public class UnreachableBlocksOptimizer readonly LinkContext _context; readonly Dictionary _cache_method_results = new (2048); readonly Stack _resursion_guard = new (); + readonly HashSet _processed_methods = new (2048); MethodDefinition? IntPtrSize, UIntPtrSize; @@ -39,6 +40,9 @@ public UnreachableBlocksOptimizer (LinkContext context) /// The method to process public void ProcessMethod (MethodDefinition method) { + if (!_processed_methods.Add (method)) + return; + if (!IsMethodSupported (method)) return; diff --git a/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs b/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs new file mode 100644 index 000000000000..38661c3b656e --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs @@ -0,0 +1,246 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.UnreachableBlock +{ + [SetupLinkerArgument ("--enable-opt", "ipconstprop")] + + // Using Kept validation on compiler generated code is tricky as we would have to describe + // all of the compiler generated classes and members which are expected to be kept. + // So not using that here (at least until we come up with a better way to do this). + // Instead this test relies on RUC and warnings to detect "kept" and "removed" calls. + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + class CompilerGeneratedCodeSubstitutions + { + static void Main () + { + Lambda.Test (); + LocalFunction.Test (); + Iterator.Test (); + Async.Test (); + } + + class Lambda + { + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static void TestBranchInLambda () + { + var a = () => { + if (AlwaysFalse) { + RemovedMethod (); + } else { + UsedMethod (); + } + }; + + a (); + } + + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static void TestBranchAroundLambda () + { + Action a; + if (AlwaysFalse) { + a = () => RemovedMethod (); + } + else { + a = () => UsedMethod (); + } + + a (); + } + + public static void Test () + { + TestBranchInLambda (); + TestBranchAroundLambda (); + } + } + + class LocalFunction + { + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static void TestBranchInLocalFunction () + { + void LocalFunction () + { + if (AlwaysFalse) { + RemovedMethod (); + } else { + UsedMethod (); + } + } + + LocalFunction (); + } + + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static void TestBranchAroundLocalFunction () + { + Action a; + if (AlwaysFalse) { + void RemovedLocalFunction () + { + RemovedMethod (); + } + + RemovedLocalFunction (); + } else { + void UsedLocalFunction () + { + UsedMethod (); + } + + UsedLocalFunction (); + } + } + + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static void TestBranchAroundUsageOfLocalFunction () + { + Action a; + if (AlwaysFalse) { + RemovedLocalFunction (); + } else { + UsedLocalFunction (); + } + + void RemovedLocalFunction () + { + RemovedMethod (); + } + + void UsedLocalFunction () + { + UsedMethod (); + } + } + + public static void Test () + { + TestBranchInLocalFunction (); + TestBranchAroundLocalFunction (); + TestBranchAroundUsageOfLocalFunction (); + } + } + + class Iterator + { + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static IEnumerable TestBranchWithNormalCall () + { + if (AlwaysFalse) { + RemovedMethod (); + } else { + UsedMethod (); + } + + yield return 1; + } + + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static IEnumerable TestBranchWithYieldAfter () + { + if (AlwaysFalse) { + RemovedMethod (); + yield return 1; + } else { + UsedMethod (); + yield return 1; + } + + yield return 1; + } + + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + // https://github.com/dotnet/linker/issues/3087 + [ExpectedWarning ("IL2026", "--RemovedMethod--", CompilerGeneratedCode = true)] + static IEnumerable TestBranchWithYieldBefore () + { + if (AlwaysFalse) { + yield return 1; + RemovedMethod (); + } else { + yield return 1; + UsedMethod (); + } + + yield return 1; + } + + public static void Test () + { + TestBranchWithNormalCall (); + TestBranchWithYieldAfter (); + TestBranchWithYieldBefore (); + } + } + + class Async + { + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + static async Task TestBranchWithNormalCall () + { + if (AlwaysFalse) { + RemovedMethod (); + } else { + UsedMethod (); + } + + await Task.FromResult (0); + } + + [ExpectedWarning ("IL2026", "--UsedMethod--", CompilerGeneratedCode = true)] + // https://github.com/dotnet/linker/issues/3087 + [ExpectedWarning ("IL2026", "--RemovedMethod--", CompilerGeneratedCode = true)] + static async Task TestBranchWithNormalCallAfterWAwait () + { + if (AlwaysFalse) { + await Task.FromResult (0); + RemovedMethod (); + } else { + await Task.FromResult (0); + UsedMethod (); + } + + await Task.FromResult (0); + } + + [ExpectedWarning ("IL2026", "--UsedAsyncMethod--", CompilerGeneratedCode = true)] + static async Task TestBranchWithAsyncCall () + { + if (AlwaysFalse) { + await RemovedAsyncMethod (); + } else { + await UsedAsyncMethod (); + } + } + + public static void Test () + { + TestBranchWithNormalCall ().RunSynchronously (); ; + TestBranchWithNormalCallAfterWAwait ().RunSynchronously (); + TestBranchWithAsyncCall ().RunSynchronously (); + } + } + + static bool AlwaysFalse => false; + + [RequiresUnreferencedCode ("--UsedAsyncMethod--")] + static async Task UsedAsyncMethod () => await Task.FromResult (0); + + [RequiresUnreferencedCode ("--RemovedAsyncMethod--")] + static async Task RemovedAsyncMethod () => await Task.FromResult (-1); + + [RequiresUnreferencedCode ("--UsedMethod--")] + static void UsedMethod () { } + + [RequiresUnreferencedCode ("--RemovedMethod--")] + static void RemovedMethod () { } + } +} From 67aeb3b7c5a3bc33ada2598621652e2b75c97bcf Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Thu, 27 Oct 2022 13:34:22 -0700 Subject: [PATCH 2/5] Formatting --- .../UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs b/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs index 38661c3b656e..5b1edc37671a 100644 --- a/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs +++ b/test/Mono.Linker.Tests.Cases/UnreachableBlock/CompilerGeneratedCodeSubstitutions.cs @@ -47,8 +47,7 @@ static void TestBranchAroundLambda () Action a; if (AlwaysFalse) { a = () => RemovedMethod (); - } - else { + } else { a = () => UsedMethod (); } From 69dcb6f4419bf820742327c7c493fa957683472f Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Mon, 31 Oct 2022 06:36:36 -0700 Subject: [PATCH 3/5] Rework the change to guarantee that all accessed to Body are after the constant prop/branch removal happened on the method. This does have one possibly negative impact: the issue described in https://github.com/dotnet/linker/issues/2937 is now consistent and happens always. Basically if there's a local function which is going to be removed due to branch removal and if the body of that method contains code which produces a warning due to generic parameter validation, such warning will always be generated even though it's "dead" code and even if it's suppressed via RUC or similar. Basically in such case the analysis can't figure out to which method the local function belongs (since the call site has been removed). --- src/linker/BannedSymbols.txt | 3 + .../Linker.Dataflow/CompilerGeneratedState.cs | 4 +- src/linker/Linker.Dataflow/FlowAnnotations.cs | 2 +- .../Linker.Dataflow/InterproceduralState.cs | 16 +++-- .../Linker.Dataflow/MethodBodyScanner.cs | 18 +++--- .../ReflectionMethodBodyScanner.cs | 4 +- .../Linker.Dataflow/ScannerExtensions.cs | 3 +- src/linker/Linker.Steps/AddBypassNGenStep.cs | 2 + src/linker/Linker.Steps/CodeRewriterStep.cs | 13 ++-- src/linker/Linker.Steps/MarkStep.cs | 60 ++++++++----------- .../MethodBodyInstructionsProvider.cs | 52 ++++++++++++++++ .../UnreachableBlocksOptimizer.cs | 39 ++++++++---- src/linker/Linker/LinkContext.cs | 3 + src/linker/Linker/MethodBodyScanner.cs | 13 ++-- src/linker/Linker/TypeReferenceWalker.cs | 2 + ...ratedCodeInPreservedAssemblyWithWarning.cs | 14 ++--- 16 files changed, 162 insertions(+), 86 deletions(-) create mode 100644 src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs diff --git a/src/linker/BannedSymbols.txt b/src/linker/BannedSymbols.txt index d7c70af181c2..31a20cc8bfc8 100644 --- a/src/linker/BannedSymbols.txt +++ b/src/linker/BannedSymbols.txt @@ -4,3 +4,6 @@ M:Mono.Cecil.MethodReference.Resolve();Use LinkContext.Resolve and LinkContext.T M:Mono.Cecil.ExportedType.Resolve();Use LinkContext.Resolve and LinkContext.TryResolve helpers instead P:Mono.Collections.Generic.Collection`1{Mono.Cecil.ParameterDefinition}.Item(System.Int32); use x P:Mono.Cecil.ParameterDefinitionCollection.Item(System.Int32); use x +P:Mono.Cecil.Cil.MethodBody.Instructions;Use LinkContext.MethodBodyInstructionProvider instead +P:Mono.Cecil.Cil.MethodBody.ExceptionHandlers;Use LinkContext.MethodBodyInstructionProvider instead +P:Mono.Cecil.Cil.MethodBody.Variable;Use LinkContext.MethodBodyInstructionProvider instead diff --git a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs index 3033cef70153..7e980f06b09e 100644 --- a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs +++ b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs @@ -145,7 +145,7 @@ void ProcessMethod (MethodDefinition method) // Discover calls or references to lambdas or local functions. This includes // calls to local functions, and lambda assignments (which use ldftn). if (method.Body != null) { - foreach (var instruction in method.Body.Instructions) { + foreach (var instruction in _context.MethodBodyInstructionsProvider.GetMethodBody (method.Body).Instructions) { switch (instruction.OpCode.OperandType) { case OperandType.InlineMethod: { MethodDefinition? referencedMethod = _context.TryResolve ((MethodReference) instruction.Operand); @@ -354,7 +354,7 @@ void MapGeneratedTypeTypeParameters (TypeDefinition generatedType) GenericInstanceType? ScanForInit (TypeDefinition compilerGeneratedType, MethodBody body) { - foreach (var instr in body.Instructions) { + foreach (var instr in _context.MethodBodyInstructionsProvider.GetMethodBody (body).Instructions) { bool handled = false; switch (instr.OpCode.Code) { case Code.Initobj: diff --git a/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/linker/Linker.Dataflow/FlowAnnotations.cs index e14b1928c66b..c548932e2405 100644 --- a/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -453,7 +453,7 @@ bool ScanMethodBodyForFieldAccess (MethodBody body, bool write, out FieldDefinit FieldReference? foundReference = null; - foreach (Instruction instruction in body.Instructions) { + foreach (Instruction instruction in _context.MethodBodyInstructionsProvider.GetMethodBody (body).Instructions) { switch (instruction.OpCode.Code) { case Code.Ldsfld when !write: case Code.Ldfld when !write: diff --git a/src/linker/Linker.Dataflow/InterproceduralState.cs b/src/linker/Linker.Dataflow/InterproceduralState.cs index 09bc3be22630..2ab2d795d745 100644 --- a/src/linker/Linker.Dataflow/InterproceduralState.cs +++ b/src/linker/Linker.Dataflow/InterproceduralState.cs @@ -7,6 +7,7 @@ using ILLink.Shared.DataFlow; using Mono.Cecil; using Mono.Cecil.Cil; +using Mono.Linker.Steps; using HoistedLocalState = ILLink.Shared.DataFlow.DefaultValueDictionary< Mono.Linker.Dataflow.HoistedLocalKey, ILLink.Shared.DataFlow.ValueSet>; @@ -15,7 +16,7 @@ namespace Mono.Linker.Dataflow { // Wrapper that implements IEquatable for MethodBody. - readonly record struct MethodBodyValue (MethodBody MethodBody); + readonly record struct MethodBodyValue (MethodBodyInstructionsProvider.ProcessedMethodBody MethodBody); // Tracks the set of methods which get analyzer together during interprocedural analysis, // and the possible states of hoisted locals in state machine methods and lambdas/local functions. @@ -43,6 +44,11 @@ public void TrackMethod (MethodDefinition method) } public void TrackMethod (MethodBody methodBody) + { + TrackMethod (lattice.Context.MethodBodyInstructionsProvider.GetMethodBody (methodBody)); + } + + public void TrackMethod (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) { // Work around the fact that ValueSet is readonly var methodsList = new List (MethodBodies); @@ -55,7 +61,7 @@ public void TrackMethod (MethodBody methodBody) foreach (var stateMachineMethod in stateMachineType.Methods) { Debug.Assert (!CompilerGeneratedNames.IsLambdaOrLocalFunction (stateMachineMethod.Name)); if (stateMachineMethod.Body is MethodBody stateMachineMethodBody) - methodsList.Add (new MethodBodyValue (stateMachineMethodBody)); + methodsList.Add (new MethodBodyValue (lattice.Context.MethodBodyInstructionsProvider.GetMethodBody(stateMachineMethodBody))); } } @@ -80,11 +86,13 @@ struct InterproceduralStateLattice : ILattice { public readonly ValueSetLattice MethodBodyLattice; public readonly DictionaryLattice> HoistedLocalsLattice; + public readonly LinkContext Context; public InterproceduralStateLattice ( ValueSetLattice methodBodyLattice, - DictionaryLattice> hoistedLocalsLattice) - => (MethodBodyLattice, HoistedLocalsLattice) = (methodBodyLattice, hoistedLocalsLattice); + DictionaryLattice> hoistedLocalsLattice, + LinkContext context) + => (MethodBodyLattice, HoistedLocalsLattice, Context) = (methodBodyLattice, hoistedLocalsLattice, context); public InterproceduralState Top => new InterproceduralState (MethodBodyLattice.Top, HoistedLocalsLattice.Top, this); diff --git a/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/linker/Linker.Dataflow/MethodBodyScanner.cs index 0216252cc7df..023eebd21e13 100644 --- a/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -12,6 +12,7 @@ using Mono.Cecil; using Mono.Cecil.Cil; using Mono.Collections.Generic; +using Mono.Linker.Steps; using static Mono.Linker.ParameterHelpers; using LocalVariableStore = System.Collections.Generic.Dictionary< Mono.Cecil.Cil.VariableDefinition, @@ -52,7 +53,7 @@ abstract partial class MethodBodyScanner protected MethodBodyScanner (LinkContext context) { this._context = context; - this.InterproceduralStateLattice = default; + this.InterproceduralStateLattice = new InterproceduralStateLattice(default, default, context); } internal MultiValue ReturnValue { private set; get; } @@ -152,7 +153,7 @@ private struct BasicBlockIterator int _currentBlockIndex; bool _foundEndOfPrevBlock; - public BasicBlockIterator (MethodBody methodBody) + public BasicBlockIterator (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) { _methodBranchTargets = methodBody.ComputeBranchTargets (); _currentBlockIndex = -1; @@ -227,7 +228,7 @@ protected static void StoreMethodLocalValue ( // Scans the method as well as any nested functions (local functions or lambdas) and state machines // reachable from it. - public virtual void InterproceduralScan (MethodBody startingMethodBody) + public virtual void InterproceduralScan (MethodBodyInstructionsProvider.ProcessedMethodBody startingMethodBody) { MethodDefinition startingMethod = startingMethodBody.Method; @@ -275,8 +276,9 @@ void TrackNestedFunctionReference (MethodReference referencedMethod, ref Interpr interproceduralState.TrackMethod (method); } - protected virtual void Scan (MethodBody methodBody, ref InterproceduralState interproceduralState) + protected virtual void Scan (MethodBodyInstructionsProvider.ProcessedMethodBody processedMethodBody, ref InterproceduralState interproceduralState) { + MethodBody methodBody = processedMethodBody.Body; MethodDefinition thisMethod = methodBody.Method; LocalVariableStore locals = new (methodBody.Variables.Count); @@ -284,12 +286,12 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int Dictionary> knownStacks = new Dictionary> (); Stack? currentStack = new Stack (methodBody.MaxStackSize); - ScanExceptionInformation (knownStacks, methodBody); + ScanExceptionInformation (knownStacks, processedMethodBody); - BasicBlockIterator blockIterator = new BasicBlockIterator (methodBody); + BasicBlockIterator blockIterator = new BasicBlockIterator (processedMethodBody); ReturnValue = new (); - foreach (Instruction operation in methodBody.Instructions) { + foreach (Instruction operation in processedMethodBody.Instructions) { int curBasicBlock = blockIterator.MoveNext (operation); if (knownStacks.ContainsKey (operation.Offset)) { @@ -700,7 +702,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int } } - private static void ScanExceptionInformation (Dictionary> knownStacks, MethodBody methodBody) + private static void ScanExceptionInformation (Dictionary> knownStacks, MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) { foreach (ExceptionHandler exceptionClause in methodBody.ExceptionHandlers) { Stack catchStack = new Stack (1); diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 15c5f83d8843..e7c17ca666b3 100644 --- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -61,7 +61,7 @@ public ReflectionMethodBodyScanner (LinkContext context, MarkStep parent, Messag TrimAnalysisPatterns = new TrimAnalysisPatternStore (MultiValueLattice, context); } - public override void InterproceduralScan (MethodBody methodBody) + public override void InterproceduralScan (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) { base.InterproceduralScan (methodBody); @@ -69,7 +69,7 @@ public override void InterproceduralScan (MethodBody methodBody) TrimAnalysisPatterns.MarkAndProduceDiagnostics (reflectionMarker, _markStep); } - protected override void Scan (MethodBody methodBody, ref InterproceduralState interproceduralState) + protected override void Scan (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody, ref InterproceduralState interproceduralState) { _origin = new MessageOrigin (methodBody.Method); base.Scan (methodBody, ref interproceduralState); diff --git a/src/linker/Linker.Dataflow/ScannerExtensions.cs b/src/linker/Linker.Dataflow/ScannerExtensions.cs index c445f7d61c20..8062d95bf944 100644 --- a/src/linker/Linker.Dataflow/ScannerExtensions.cs +++ b/src/linker/Linker.Dataflow/ScannerExtensions.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using Mono.Cecil.Cil; +using Mono.Linker.Steps; namespace Mono.Linker.Dataflow { @@ -16,7 +17,7 @@ public static bool IsControlFlowInstruction (in this OpCode opcode) || (opcode.FlowControl == FlowControl.Return && opcode.Code != Code.Ret); } - public static HashSet ComputeBranchTargets (this MethodBody methodBody) + public static HashSet ComputeBranchTargets (this MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) { HashSet branchTargets = new HashSet (); foreach (Instruction operation in methodBody.Instructions) { diff --git a/src/linker/Linker.Steps/AddBypassNGenStep.cs b/src/linker/Linker.Steps/AddBypassNGenStep.cs index e8706e1d6cc2..9a6443173a5d 100644 --- a/src/linker/Linker.Steps/AddBypassNGenStep.cs +++ b/src/linker/Linker.Steps/AddBypassNGenStep.cs @@ -94,7 +94,9 @@ private void EnsureBypassNGenAttribute (ModuleDefinition targetModule) const MethodAttributes ctorAttributes = MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.SpecialName | MethodAttributes.RTSpecialName; bypassNGenAttributeDefaultConstructor = new MethodDefinition (".ctor", ctorAttributes, coreLibAssembly.MainModule.TypeSystem.Void); +#pragma warning disable RS0030 // Anything after MarkStep should use Cecil directly as all method bodies should be processed by this point var instructions = bypassNGenAttributeDefaultConstructor.Body.Instructions; +#pragma warning restore RS0030 instructions.Add (Instruction.Create (OpCodes.Ldarg_0)); instructions.Add (Instruction.Create (OpCodes.Call, systemAttributeDefaultConstructor)); instructions.Add (Instruction.Create (OpCodes.Ret)); diff --git a/src/linker/Linker.Steps/CodeRewriterStep.cs b/src/linker/Linker.Steps/CodeRewriterStep.cs index c71624451ff4..edefa53004ad 100644 --- a/src/linker/Linker.Steps/CodeRewriterStep.cs +++ b/src/linker/Linker.Steps/CodeRewriterStep.cs @@ -64,12 +64,15 @@ void AddFieldsInitializations (TypeDefinition type) ret = Instruction.Create (OpCodes.Ret); processor.Append (ret); } else { - ret = cctor.Body.Instructions.Last (l => l.OpCode.Code == Code.Ret); var body = cctor.Body; - processor = cctor.Body.GetLinkerILProcessor (); - - for (int i = 0; i < body.Instructions.Count; ++i) { - var instr = body.Instructions[i]; +#pragma warning disable RS0030 // After MarkStep all methods should be processed and thus accessing Cecil directly is the right approach + var instructions = body.Instructions; +#pragma warning restore RS0030 + ret = instructions.Last (l => l.OpCode.Code == Code.Ret); + processor = body.GetLinkerILProcessor (); + + for (int i = 0; i < instructions.Count; ++i) { + var instr = instructions[i]; if (instr.OpCode.Code != Code.Stsfld) continue; diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index ede3b5efbbc3..317d652ba15d 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -75,13 +75,6 @@ protected LinkContext Context { // method body scanner. readonly Dictionary _compilerGeneratedMethodRequiresScanner; - UnreachableBlocksOptimizer? _unreachableBlocksOptimizer; - UnreachableBlocksOptimizer UnreachableBlocksOptimizer { - get { - Debug.Assert (_unreachableBlocksOptimizer != null); - return _unreachableBlocksOptimizer; - } - } MarkStepContext? _markContext; MarkStepContext MarkContext { get { @@ -245,7 +238,6 @@ public MarkStep () public virtual void Process (LinkContext context) { _context = context; - _unreachableBlocksOptimizer = new UnreachableBlocksOptimizer (_context); _markContext = new MarkStepContext (); _scopeStack = new MarkScopeStack (); _dynamicallyAccessedMembersTypeHierarchy = new DynamicallyAccessedMembersTypeHierarchy (_context, this); @@ -2556,7 +2548,7 @@ void MarkICustomMarshalerMethods (TypeDefinition inputType, in DependencyInfo re } while ((type = Context.TryResolve (type.BaseType)) != null); } - static bool IsNonEmptyStaticConstructor (MethodDefinition method) + bool IsNonEmptyStaticConstructor (MethodDefinition method) { if (!method.IsStaticConstructor ()) return false; @@ -2564,10 +2556,12 @@ static bool IsNonEmptyStaticConstructor (MethodDefinition method) if (!method.HasBody || !method.IsIL) return true; - if (method.Body.CodeSize != 1) + var body = Context.MethodBodyInstructionsProvider.GetMethodBody (method.Body); + + if (body.Body.CodeSize != 1) return true; - return method.Body.Instructions[0].OpCode.Code != Code.Ret; + return body.Instructions[0].OpCode.Code != Code.Ret; } static bool HasOnSerializeOrDeserializeAttribute (MethodDefinition method) @@ -2843,14 +2837,14 @@ protected bool MarkFields (TypeDefinition type, bool includeStatic, in Dependenc return true; } - static PropertyDefinition? SearchPropertiesForMatchingFieldDefinition (FieldDefinition field) + PropertyDefinition? SearchPropertiesForMatchingFieldDefinition (FieldDefinition field) { foreach (var property in field.DeclaringType.Properties) { - var instr = property.GetMethod?.Body?.Instructions; - if (instr == null) + var body = property.GetMethod?.Body; + if (body == null) continue; - foreach (var ins in instr) { + foreach (var ins in Context.MethodBodyInstructionsProvider.GetMethodBody (body).Instructions) { if (ins?.Operand == field) return property; } @@ -2942,7 +2936,7 @@ bool ShouldWarnForReflectionAccessToCompilerGeneratedCode (MethodDefinition meth // the reflection scanner. Checking this will also mark direct dependencies of the method body, if it // hasn't been marked already. A cache ensures this only happens once for the method, whether or not // it is accessed via reflection. - return CheckRequiresReflectionMethodBodyScanner (method.Body); + return CheckRequiresReflectionMethodBodyScanner (Context.MethodBodyInstructionsProvider.GetMethodBody (method.Body)); } void ProcessAnalysisAnnotationsForMethod (MethodDefinition method, DependencyKind dependencyKind, in MessageOrigin origin) @@ -3124,8 +3118,6 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo if (CheckProcessed (method)) return; - UnreachableBlocksOptimizer.ProcessMethod (method); - foreach (Action handleMarkMethod in MarkContext.MarkMethodActions) handleMarkMethod (method); @@ -3477,7 +3469,9 @@ internal void MarkMethodIfNotNull (MethodReference method, in DependencyInfo rea protected virtual void MarkMethodBody (MethodBody body) { - if (Context.IsOptimizationEnabled (CodeOptimizations.UnreachableBodies, body.Method) && IsUnreachableBody (body)) { + var processedMethodBody = Context.MethodBodyInstructionsProvider.GetMethodBody (body); + + if (Context.IsOptimizationEnabled (CodeOptimizations.UnreachableBodies, body.Method) && IsUnreachableBody (processedMethodBody)) { MarkAndCacheConvertToThrowExceptionCtor (new DependencyInfo (DependencyKind.UnreachableBodyRequirement, body.Method)); _unreachableBodies.Add ((body, ScopeStack.CurrentScope)); return; @@ -3488,17 +3482,17 @@ protected virtual void MarkMethodBody (MethodBody body) // But for compiler-generated methods we only do dataflow analysis if they're used through their // corresponding user method, so we will skip dataflow for compiler-generated methods which // are only accessed via reflection. - bool requiresReflectionMethodBodyScanner = MarkAndCheckRequiresReflectionMethodBodyScanner (body); + bool requiresReflectionMethodBodyScanner = MarkAndCheckRequiresReflectionMethodBodyScanner (processedMethodBody); // Data-flow (reflection scanning) for compiler-generated methods will happen as part of the // data-flow scan of the user-defined method which uses this compiler-generated method. if (CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (body.Method)) return; - MarkReflectionLikeDependencies (body, requiresReflectionMethodBodyScanner); + MarkReflectionLikeDependencies (processedMethodBody, requiresReflectionMethodBodyScanner); } - bool CheckRequiresReflectionMethodBodyScanner (MethodBody body) + bool CheckRequiresReflectionMethodBodyScanner (MethodBodyInstructionsProvider.ProcessedMethodBody body) { // This method is only called on reflection access to compiler-generated methods. // This should be uncommon, so don't cache the result. @@ -3525,7 +3519,7 @@ bool CheckRequiresReflectionMethodBodyScanner (MethodBody body) // It computes the same value, while also marking as it goes, as an optimization. // This should only be called behind a check to IsProcessed for the method or corresponding user method, // to avoid recursion. - bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodBody body) + bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodBodyInstructionsProvider.ProcessedMethodBody body) { #if DEBUG if (!Annotations.IsProcessed (body.Method)) { @@ -3540,13 +3534,9 @@ bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodBody body) // This may get called multiple times for compiler-generated code: once for // reflection access, and once as part of the interprocedural scan of the user method. // This check ensures that we only do the work and produce warnings once. - if (_compilerGeneratedMethodRequiresScanner.TryGetValue (body, out bool requiresReflectionMethodBodyScanner)) + if (_compilerGeneratedMethodRequiresScanner.TryGetValue (body.Body, out bool requiresReflectionMethodBodyScanner)) return requiresReflectionMethodBodyScanner; - // Make sure the method's body is processed, for compiler generated code we can get here before ProcessMethod is called - // and thus we need to make sure we operate on the optimized method body (to avoid marking code which is otherwise optimized away). - UnreachableBlocksOptimizer.ProcessMethod (body.Method); - foreach (VariableDefinition var in body.Variables) MarkType (var.VariableType, new DependencyInfo (DependencyKind.VariableType, body.Method)); @@ -3563,15 +3553,15 @@ bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodBody body) MarkInterfacesNeededByBodyStack (body); if (CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (body.Method)) - _compilerGeneratedMethodRequiresScanner.Add (body, requiresReflectionMethodBodyScanner); + _compilerGeneratedMethodRequiresScanner.Add (body.Body, requiresReflectionMethodBodyScanner); - PostMarkMethodBody (body); + PostMarkMethodBody (body.Body); Debug.Assert (requiresReflectionMethodBodyScanner == CheckRequiresReflectionMethodBodyScanner (body)); return requiresReflectionMethodBodyScanner; } - bool IsUnreachableBody (MethodBody body) + bool IsUnreachableBody (MethodBodyInstructionsProvider.ProcessedMethodBody body) { return !body.Method.IsStatic && !Annotations.IsInstantiated (body.Method.DeclaringType) @@ -3581,7 +3571,7 @@ bool IsUnreachableBody (MethodBody body) partial void PostMarkMethodBody (MethodBody body); - void MarkInterfacesNeededByBodyStack (MethodBody body) + void MarkInterfacesNeededByBodyStack (MethodBodyInstructionsProvider.ProcessedMethodBody body) { // If a type could be on the stack in the body and an interface it implements could be on the stack on the body // then we need to mark that interface implementation. When this occurs it is not safe to remove the interface implementation from the type @@ -3740,7 +3730,7 @@ protected internal virtual bool ProcessReflectionDependency (MethodBody body, In // // Tries to mark additional dependencies used in reflection like calls (e.g. typeof (MyClass).GetField ("fname")) // - protected virtual void MarkReflectionLikeDependencies (MethodBody body, bool requiresReflectionMethodBodyScanner) + protected virtual void MarkReflectionLikeDependencies (MethodBodyInstructionsProvider.ProcessedMethodBody body, bool requiresReflectionMethodBodyScanner) { Debug.Assert (!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (body.Method)); // requiresReflectionMethodBodyScanner tells us whether the method body itself requires a dataflow scan. @@ -3752,12 +3742,12 @@ protected virtual void MarkReflectionLikeDependencies (MethodBody body, bool req switch (compilerGeneratedCallee) { case MethodDefinition nestedFunction: if (nestedFunction.Body is MethodBody nestedBody) - requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (nestedBody); + requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (Context.MethodBodyInstructionsProvider.GetMethodBody (nestedBody)); break; case TypeDefinition stateMachineType: foreach (var method in stateMachineType.Methods) { if (method.Body is MethodBody stateMachineBody) - requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (stateMachineBody); + requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (Context.MethodBodyInstructionsProvider.GetMethodBody (stateMachineBody)); } break; default: diff --git a/src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs b/src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs new file mode 100644 index 000000000000..7d02ccff3c26 --- /dev/null +++ b/src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs @@ -0,0 +1,52 @@ +// 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 Mono.Cecil; +using Mono.Cecil.Cil; +using Mono.Collections.Generic; + +namespace Mono.Linker.Steps +{ + public class MethodBodyInstructionsProvider + { + readonly UnreachableBlocksOptimizer _unreachableBlocksOptimizer; + + public readonly struct ProcessedMethodBody + { + private ProcessedMethodBody (MethodBody body) => this.Body = body; + + public readonly MethodBody Body; + + public MethodDefinition Method => Body.Method; + +#pragma warning disable RS0030 // Wrapper which provides safe access to the property + public Collection Instructions => Body.Instructions; +#pragma warning restore RS0030 + +#pragma warning disable RS0030 // Wrapper which provides safe access to the property + public Collection ExceptionHandlers => Body.ExceptionHandlers; +#pragma warning restore RS0030 + +#pragma warning disable RS0030 // Wrapper which provides safe access to the property + public Collection Variables => Body.Variables; +#pragma warning restore RS0030 + + public static ProcessedMethodBody Create (MethodBody body) => new ProcessedMethodBody (body); + } + + public MethodBodyInstructionsProvider(LinkContext context) + { + _unreachableBlocksOptimizer = new UnreachableBlocksOptimizer(context); + } + + public ProcessedMethodBody GetMethodBody (MethodBody methodBody) + => GetMethodBody (methodBody.Method); + + public ProcessedMethodBody GetMethodBody (MethodDefinition method) + { + _unreachableBlocksOptimizer.ProcessMethod (method); + + return ProcessedMethodBody.Create (method.Body); + } + } +} diff --git a/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs b/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs index 08484841f87a..18d55349b0ec 100644 --- a/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs +++ b/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs @@ -437,9 +437,11 @@ public bool RewriteBody () { bool changed = false; LinkerILProcessor processor = body.GetLinkerILProcessor (); +#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly Collection instrs = body.Instructions; +#pragma warning restore RS0030 - for (int i = 0; i < body.Instructions.Count; ++i) { + for (int i = 0; i < instrs.Count; ++i) { Instruction instr = instrs[i]; switch (instr.OpCode.Code) { @@ -458,7 +460,7 @@ public bool RewriteBody () if (md.NoInlining) break; - var cpl = new CalleePayload (md, GetArgumentsOnStack (md, body.Instructions, i)); + var cpl = new CalleePayload (md, GetArgumentsOnStack (md, instrs, i)); MethodResult? call_result = optimizer.TryGetMethodCallResult (cpl); if (call_result is not MethodResult result) break; @@ -553,6 +555,10 @@ public BodyReducer (MethodBody body, LinkContext context) public MethodBody Body { get; } +#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly + private Collection Instructions => Body.Instructions; +#pragma warning restore RS0030 + public int InstructionsReplaced { get; set; } Collection? FoldedInstructions { get; set; } @@ -561,7 +567,7 @@ public BodyReducer (MethodBody body, LinkContext context) [MemberNotNull ("mapping")] void InitializeFoldedInstruction () { - FoldedInstructions = new Collection (Body.Instructions); + FoldedInstructions = new Collection (Instructions); mapping = new Dictionary (); } @@ -574,7 +580,7 @@ public void Rewrite (int index, Instruction newInstruction) // Tracks mapping for replaced instructions for easier // branch targets resolution later - mapping[Body.Instructions[index]] = index; + mapping[Instructions[index]] = index; FoldedInstructions[index] = newInstruction; } @@ -737,7 +743,7 @@ public bool RewriteBody () public bool ApplyTemporaryInlining (in UnreachableBlocksOptimizer optimizer) { bool changed = false; - var instructions = Body.Instructions; + var instructions = Instructions; Instruction? targetResult; for (int i = 0; i < instructions.Count; ++i) { @@ -827,10 +833,10 @@ void RemoveUnreachableInstructions (BitArray reachable) // inject "ldnull; throw;" at the end - this branch should never be reachable and it's always valid // (ret may need to return a value of the right type if the method has a return value which is complicated // to construct out of nothing). - if (index == Body.Instructions.Count - 1 && Body.Instructions[index].OpCode == OpCodes.Ret && - index > 0 && IsConditionalBranch (Body.Instructions[index - 1].OpCode)) { + if (index == Instructions.Count - 1 && Instructions[index].OpCode == OpCodes.Ret && + index > 0 && IsConditionalBranch (Instructions[index - 1].OpCode)) { processor.Replace (index, Instruction.Create (OpCodes.Ldnull)); - processor.InsertAfter (Body.Instructions[index], Instruction.Create (OpCodes.Throw)); + processor.InsertAfter (Instructions[index], Instruction.Create (OpCodes.Throw)); } else { processor.RemoveAt (index); ++removed; @@ -1046,7 +1052,8 @@ BitArray GetReachableInstructionsMap (out List? unreachableHan if (!exceptionHandlersChecked) { exceptionHandlersChecked = true; - var instrs = Body.Instructions; + var instrs = Instructions; +#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly foreach (var handler in Body.ExceptionHandlers) { int start = instrs.IndexOf (handler.TryStart); int end = instrs.IndexOf (handler.TryEnd) - 1; @@ -1064,6 +1071,7 @@ BitArray GetReachableInstructionsMap (out List? unreachableHan if (handler.FilterStart != null) condBranches.Push (GetInstructionIndex (handler.FilterStart)); } +#pragma warning restore RS0030 if (condBranches?.Count > 0) { i = condBranches.Pop (); @@ -1158,6 +1166,9 @@ bool IsJumpTargetRange (int firstInstr, int lastInstr) struct BodySweeper { readonly MethodBody body; +#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly + private Collection Instructions => body.Instructions; +#pragma warning restore RS0030 readonly BitArray reachable; readonly List? unreachableExceptionHandlers; readonly LinkContext context; @@ -1184,7 +1195,7 @@ public BodySweeper (MethodBody body, BitArray reachable, List? public void Initialize () { - var instrs = body.Instructions; + var instrs = Instructions; // // Reusing same reachable map and altering it at indexes @@ -1219,7 +1230,7 @@ public void Process (List? conditionInstrsToRemove, out List? // Initial pass which replaces unreachable instructions with nops or // ret to keep the body verifiable // - var instrs = body.Instructions; + var instrs = Instructions; for (int i = 0; i < instrs.Count; ++i) { if (reachable[i]) continue; @@ -1306,7 +1317,7 @@ public void Process (List? conditionInstrsToRemove, out List? void CleanRemovedVariables (List variables) { - foreach (var instr in body.Instructions) { + foreach (var instr in Instructions) { VariableDefinition? variable = GetVariableReference (instr); if (variable == null) continue; @@ -1343,8 +1354,10 @@ void CleanExceptionHandlers () if (unreachableExceptionHandlers == null) return; +#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly foreach (var eh in unreachableExceptionHandlers) body.ExceptionHandlers.Remove (eh); +#pragma warning restore RS0030 } VariableDefinition? GetVariableReference (Instruction instruction) @@ -1406,7 +1419,9 @@ public bool Analyze (in CalleePayload callee, Stack callStack) { MethodDefinition method = callee.Method; Instruction[]? arguments = callee.Arguments; +#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly Collection instructions = callee.Method.Body.Instructions; +#pragma warning restore RS0030 MethodBody body = method.Body; VariableReference vr; diff --git a/src/linker/Linker/LinkContext.cs b/src/linker/Linker/LinkContext.cs index baa348ff1f0d..abddf3b8aba4 100644 --- a/src/linker/Linker/LinkContext.cs +++ b/src/linker/Linker/LinkContext.cs @@ -189,6 +189,8 @@ internal TypeNameResolver TypeNameResolver { public SerializationMarker SerializationMarker { get; } + public MethodBodyInstructionsProvider MethodBodyInstructionsProvider { get; } + public LinkContext (Pipeline pipeline, ILogger logger, string outputDirectory) { _pipeline = pipeline; @@ -223,6 +225,7 @@ public LinkContext (Pipeline pipeline, ILogger logger, string outputDirectory) GeneralSingleWarn = false; SingleWarn = new Dictionary (); AssembliesWithGeneratedSingleWarning = new HashSet (); + MethodBodyInstructionsProvider = new MethodBodyInstructionsProvider (this); const CodeOptimizations defaultOptimizations = CodeOptimizations.BeforeFieldInit | diff --git a/src/linker/Linker/MethodBodyScanner.cs b/src/linker/Linker/MethodBodyScanner.cs index 318c0930b5a8..0594b1ab38db 100644 --- a/src/linker/Linker/MethodBodyScanner.cs +++ b/src/linker/Linker/MethodBodyScanner.cs @@ -7,12 +7,13 @@ using System.Linq; using Mono.Cecil; using Mono.Cecil.Cil; +using Mono.Linker.Steps; namespace Mono.Linker { public static class MethodBodyScanner { - public static bool IsWorthConvertingToThrow (MethodBody body) + public static bool IsWorthConvertingToThrow (MethodBodyInstructionsProvider.ProcessedMethodBody body) { // Some bodies are cheaper size wise to leave alone than to convert to a throw Instruction? previousMeaningful = null; @@ -64,9 +65,9 @@ public InterfacesOnStackScanner (LinkContext context) this.context = context; } - public IEnumerable<(InterfaceImplementation, TypeDefinition)>? GetReferencedInterfaces (MethodBody body) + public IEnumerable<(InterfaceImplementation, TypeDefinition)>? GetReferencedInterfaces (MethodBodyInstructionsProvider.ProcessedMethodBody body) { - var possibleStackTypes = AllPossibleStackTypes (body.Method); + var possibleStackTypes = AllPossibleStackTypes (body); if (possibleStackTypes.Count == 0) return null; @@ -95,12 +96,8 @@ public InterfacesOnStackScanner (LinkContext context) return interfaceImplementations; } - HashSet AllPossibleStackTypes (MethodDefinition method) + HashSet AllPossibleStackTypes (MethodBodyInstructionsProvider.ProcessedMethodBody body) { - if (!method.HasBody) - throw new ArgumentException ("Method does not have body", nameof (method)); - - var body = method.Body; var types = new HashSet (); foreach (VariableDefinition var in body.Variables) diff --git a/src/linker/Linker/TypeReferenceWalker.cs b/src/linker/Linker/TypeReferenceWalker.cs index c4ad8e58d987..71ffbd1e521e 100644 --- a/src/linker/Linker/TypeReferenceWalker.cs +++ b/src/linker/Linker/TypeReferenceWalker.cs @@ -153,6 +153,7 @@ void WalkTypeScope (Collection forwarders) void WalkTypeScope (MethodBody body) { +#pragma warning disable RS0030 // Processing type references should not trigger method marking/processing, so access Cecil directly if (body.HasVariables) { foreach (var v in body.Variables) { WalkScopeOfTypeReference (v.VariableType); @@ -204,6 +205,7 @@ void WalkTypeScope (MethodBody body) } } } +#pragma warning restore RS0030 // Do not used banned APIs } void WalkMethodReference (MethodReference mr) diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs index b66b0cd30929..f05f1b7fbb9c 100644 --- a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs +++ b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeInPreservedAssemblyWithWarning.cs @@ -26,12 +26,13 @@ public static void Main () WithLocalFunction (); } + // The compiler generated state will see the modified body, + // and will not associate the local function with the user method. + // Generic argument warnings from the local function will not be suppressed + // by RUC on the user method. + class Inner { - // In this case the compiler generated state will see the modified body, - // and will not associate the local function with the user method. - // Generic argument warnings from the local function will not be suppressed - // by RUC on the user method. [RequiresUnreferencedCode ("--" + nameof (Inner) + "." + nameof (WithLocalFunctionInner) + "--")] public static void WithLocalFunctionInner () { @@ -49,10 +50,6 @@ void LocalWithWarning () } } - // In this case the compiler generated state will see the original body, - // and will associate the local function with the user method. - // Generic argument warnings from the local function will be suppressed - // by RUC on the user method. [RequiresUnreferencedCode ("--" + nameof (WithLocalFunction) + "--")] public static void WithLocalFunction () { @@ -61,6 +58,7 @@ public static void WithLocalFunction () } // https://github.com/dotnet/linker/issues/2937 + [ExpectedWarning ("IL2091", ProducedBy = ProducedBy.Trimmer)] void LocalWithWarning () { // No warning From e63c115f0d4344d16f8893ff93e35d4af8dc5ff2 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 1 Nov 2022 03:33:52 -0700 Subject: [PATCH 4/5] PR feedback and other cleanup Basically just refactored the helper class into a separate MethodIL and moved the logic onto LinkContext. The rest is just renames and cleanup. --- src/linker/BannedSymbols.txt | 3 +- .../Linker.Dataflow/CompilerGeneratedState.cs | 4 +- src/linker/Linker.Dataflow/FlowAnnotations.cs | 2 +- .../Linker.Dataflow/InterproceduralState.cs | 33 ++++---- .../Linker.Dataflow/MethodBodyScanner.cs | 52 ++++++------- .../ReflectionMethodBodyScanner.cs | 14 ++-- .../Linker.Dataflow/ScannerExtensions.cs | 7 +- src/linker/Linker.Steps/CodeRewriterStep.cs | 2 + src/linker/Linker.Steps/MarkStep.cs | 78 +++++++++---------- .../MethodBodyInstructionsProvider.cs | 52 ------------- .../UnreachableBlocksOptimizer.cs | 35 ++++----- src/linker/Linker/LinkContext.cs | 27 ++++++- src/linker/Linker/MethodBodyScanner.cs | 18 ++--- src/linker/Linker/MethodIL.cs | 39 ++++++++++ 14 files changed, 186 insertions(+), 180 deletions(-) delete mode 100644 src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs create mode 100644 src/linker/Linker/MethodIL.cs diff --git a/src/linker/BannedSymbols.txt b/src/linker/BannedSymbols.txt index 70d0748e12d2..c92f6876a857 100644 --- a/src/linker/BannedSymbols.txt +++ b/src/linker/BannedSymbols.txt @@ -8,4 +8,5 @@ P:Mono.Collections.Generic.Collection`1{Mono.Cecil.ParameterDefinition}.Item(Sys P:Mono.Cecil.ParameterDefinitionCollection.Item(System.Int32); use x P:Mono.Cecil.Cil.MethodBody.Instructions;Use LinkContext.MethodBodyInstructionProvider instead P:Mono.Cecil.Cil.MethodBody.ExceptionHandlers;Use LinkContext.MethodBodyInstructionProvider instead -P:Mono.Cecil.Cil.MethodBody.Variable;Use LinkContext.MethodBodyInstructionProvider instead +P:Mono.Cecil.Cil.MethodBody.Variables;Use LinkContext.MethodBodyInstructionProvider instead +M:Mono.Linker.Steps.ILProvider/MethodIL.Create;Use ILProvider GetMethodIL instead diff --git a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs index 7e980f06b09e..d5bfc5d8d264 100644 --- a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs +++ b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs @@ -145,7 +145,7 @@ void ProcessMethod (MethodDefinition method) // Discover calls or references to lambdas or local functions. This includes // calls to local functions, and lambda assignments (which use ldftn). if (method.Body != null) { - foreach (var instruction in _context.MethodBodyInstructionsProvider.GetMethodBody (method.Body).Instructions) { + foreach (var instruction in _context.GetMethodIL (method.Body).Instructions) { switch (instruction.OpCode.OperandType) { case OperandType.InlineMethod: { MethodDefinition? referencedMethod = _context.TryResolve ((MethodReference) instruction.Operand); @@ -354,7 +354,7 @@ void MapGeneratedTypeTypeParameters (TypeDefinition generatedType) GenericInstanceType? ScanForInit (TypeDefinition compilerGeneratedType, MethodBody body) { - foreach (var instr in _context.MethodBodyInstructionsProvider.GetMethodBody (body).Instructions) { + foreach (var instr in _context.GetMethodIL (body).Instructions) { bool handled = false; switch (instr.OpCode.Code) { case Code.Initobj: diff --git a/src/linker/Linker.Dataflow/FlowAnnotations.cs b/src/linker/Linker.Dataflow/FlowAnnotations.cs index df1ea321e352..45d1bc073aee 100644 --- a/src/linker/Linker.Dataflow/FlowAnnotations.cs +++ b/src/linker/Linker.Dataflow/FlowAnnotations.cs @@ -418,7 +418,7 @@ bool ScanMethodBodyForFieldAccess (MethodBody body, bool write, out FieldDefinit FieldReference? foundReference = null; - foreach (Instruction instruction in _context.MethodBodyInstructionsProvider.GetMethodBody (body).Instructions) { + foreach (Instruction instruction in _context.GetMethodIL (body).Instructions) { switch (instruction.OpCode.Code) { case Code.Ldsfld when !write: case Code.Ldfld when !write: diff --git a/src/linker/Linker.Dataflow/InterproceduralState.cs b/src/linker/Linker.Dataflow/InterproceduralState.cs index 2ab2d795d745..bfb5cf5730d4 100644 --- a/src/linker/Linker.Dataflow/InterproceduralState.cs +++ b/src/linker/Linker.Dataflow/InterproceduralState.cs @@ -7,7 +7,6 @@ using ILLink.Shared.DataFlow; using Mono.Cecil; using Mono.Cecil.Cil; -using Mono.Linker.Steps; using HoistedLocalState = ILLink.Shared.DataFlow.DefaultValueDictionary< Mono.Linker.Dataflow.HoistedLocalKey, ILLink.Shared.DataFlow.ValueSet>; @@ -15,23 +14,25 @@ namespace Mono.Linker.Dataflow { - // Wrapper that implements IEquatable for MethodBody. - readonly record struct MethodBodyValue (MethodBodyInstructionsProvider.ProcessedMethodBody MethodBody); - // Tracks the set of methods which get analyzer together during interprocedural analysis, // and the possible states of hoisted locals in state machine methods and lambdas/local functions. struct InterproceduralState : IEquatable { - public ValueSet MethodBodies; + public ValueSet MethodBodies; public HoistedLocalState HoistedLocals; readonly InterproceduralStateLattice lattice; - public InterproceduralState (ValueSet methodBodies, HoistedLocalState hoistedLocals, InterproceduralStateLattice lattice) + public InterproceduralState (ValueSet methodBodies, HoistedLocalState hoistedLocals, InterproceduralStateLattice lattice) => (MethodBodies, HoistedLocals, this.lattice) = (methodBodies, hoistedLocals, lattice); public bool Equals (InterproceduralState other) => MethodBodies.Equals (other.MethodBodies) && HoistedLocals.Equals (other.HoistedLocals); + public override bool Equals (object? obj) + => obj is InterproceduralState state && Equals (state); + + public override int GetHashCode () => base.GetHashCode (); + public InterproceduralState Clone () => new (MethodBodies.Clone (), HoistedLocals.Clone (), lattice); @@ -45,27 +46,27 @@ public void TrackMethod (MethodDefinition method) public void TrackMethod (MethodBody methodBody) { - TrackMethod (lattice.Context.MethodBodyInstructionsProvider.GetMethodBody (methodBody)); + TrackMethod (lattice.Context.GetMethodIL (methodBody)); } - public void TrackMethod (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) + public void TrackMethod (MethodIL methodIL) { // Work around the fact that ValueSet is readonly - var methodsList = new List (MethodBodies); - methodsList.Add (new MethodBodyValue (methodBody)); + var methodsList = new List (MethodBodies); + methodsList.Add (methodIL); // For state machine methods, also scan the state machine members. // Simplification: assume that all generated methods of the state machine type are // reached at the point where the state machine method is reached. - if (CompilerGeneratedState.TryGetStateMachineType (methodBody.Method, out TypeDefinition? stateMachineType)) { + if (CompilerGeneratedState.TryGetStateMachineType (methodIL.Method, out TypeDefinition? stateMachineType)) { foreach (var stateMachineMethod in stateMachineType.Methods) { Debug.Assert (!CompilerGeneratedNames.IsLambdaOrLocalFunction (stateMachineMethod.Name)); if (stateMachineMethod.Body is MethodBody stateMachineMethodBody) - methodsList.Add (new MethodBodyValue (lattice.Context.MethodBodyInstructionsProvider.GetMethodBody(stateMachineMethodBody))); + methodsList.Add (lattice.Context.GetMethodIL (stateMachineMethodBody)); } } - MethodBodies = new ValueSet (methodsList); + MethodBodies = new ValueSet (methodsList); } public void SetHoistedLocal (HoistedLocalKey key, MultiValue value) @@ -82,14 +83,14 @@ public MultiValue GetHoistedLocal (HoistedLocalKey key) => HoistedLocals.Get (key); } - struct InterproceduralStateLattice : ILattice + readonly struct InterproceduralStateLattice : ILattice { - public readonly ValueSetLattice MethodBodyLattice; + public readonly ValueSetLattice MethodBodyLattice; public readonly DictionaryLattice> HoistedLocalsLattice; public readonly LinkContext Context; public InterproceduralStateLattice ( - ValueSetLattice methodBodyLattice, + ValueSetLattice methodBodyLattice, DictionaryLattice> hoistedLocalsLattice, LinkContext context) => (MethodBodyLattice, HoistedLocalsLattice, Context) = (methodBodyLattice, hoistedLocalsLattice, context); diff --git a/src/linker/Linker.Dataflow/MethodBodyScanner.cs b/src/linker/Linker.Dataflow/MethodBodyScanner.cs index af9e05d7df70..fab08ede8a7f 100644 --- a/src/linker/Linker.Dataflow/MethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/MethodBodyScanner.cs @@ -12,8 +12,6 @@ using Mono.Cecil; using Mono.Cecil.Cil; using Mono.Collections.Generic; -using Mono.Linker.Steps; -using static Mono.Linker.ParameterHelpers; using LocalVariableStore = System.Collections.Generic.Dictionary< Mono.Cecil.Cil.VariableDefinition, Mono.Linker.Dataflow.ValueBasicBlockPair>; @@ -53,7 +51,7 @@ abstract partial class MethodBodyScanner protected MethodBodyScanner (LinkContext context) { this._context = context; - this.InterproceduralStateLattice = new InterproceduralStateLattice(default, default, context); + this.InterproceduralStateLattice = new InterproceduralStateLattice (default, default, context); } internal MultiValue ReturnValue { private set; get; } @@ -153,9 +151,9 @@ private struct BasicBlockIterator int _currentBlockIndex; bool _foundEndOfPrevBlock; - public BasicBlockIterator (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) + public BasicBlockIterator (MethodIL methodIL) { - _methodBranchTargets = methodBody.ComputeBranchTargets (); + _methodBranchTargets = methodIL.ComputeBranchTargets (); _currentBlockIndex = -1; _foundEndOfPrevBlock = true; } @@ -228,9 +226,9 @@ protected static void StoreMethodLocalValue ( // Scans the method as well as any nested functions (local functions or lambdas) and state machines // reachable from it. - public virtual void InterproceduralScan (MethodBodyInstructionsProvider.ProcessedMethodBody startingMethodBody) + public virtual void InterproceduralScan (MethodIL startingMethodIL) { - MethodDefinition startingMethod = startingMethodBody.Method; + MethodDefinition startingMethod = startingMethodIL.Method; // Note that the default value of a hoisted local will be MultiValueLattice.Top, not UnknownValue.Instance. // This ensures that there are no warnings for the "unassigned state" of a parameter. @@ -238,15 +236,15 @@ public virtual void InterproceduralScan (MethodBodyInstructionsProvider.Processe var interproceduralState = InterproceduralStateLattice.Top; var oldInterproceduralState = interproceduralState.Clone (); - interproceduralState.TrackMethod (startingMethodBody); + interproceduralState.TrackMethod (startingMethodIL); while (!interproceduralState.Equals (oldInterproceduralState)) { oldInterproceduralState = interproceduralState.Clone (); // Flow state through all methods encountered so far, as long as there // are changes discovered in the hoisted local state on entry to any method. - foreach (var methodBodyValue in oldInterproceduralState.MethodBodies) - Scan (methodBodyValue.MethodBody, ref interproceduralState); + foreach (var methodIL in oldInterproceduralState.MethodBodies) + Scan (methodIL, ref interproceduralState); } #if DEBUG @@ -276,22 +274,22 @@ void TrackNestedFunctionReference (MethodReference referencedMethod, ref Interpr interproceduralState.TrackMethod (method); } - protected virtual void Scan (MethodBodyInstructionsProvider.ProcessedMethodBody processedMethodBody, ref InterproceduralState interproceduralState) + protected virtual void Scan (MethodIL methodIL, ref InterproceduralState interproceduralState) { - MethodBody methodBody = processedMethodBody.Body; + MethodBody methodBody = methodIL.Body; MethodDefinition thisMethod = methodBody.Method; - LocalVariableStore locals = new (methodBody.Variables.Count); + LocalVariableStore locals = new (methodIL.Variables.Count); Dictionary> knownStacks = new Dictionary> (); Stack? currentStack = new Stack (methodBody.MaxStackSize); - ScanExceptionInformation (knownStacks, processedMethodBody); + ScanExceptionInformation (knownStacks, methodIL); - BasicBlockIterator blockIterator = new BasicBlockIterator (processedMethodBody); + BasicBlockIterator blockIterator = new BasicBlockIterator (methodIL); ReturnValue = new (); - foreach (Instruction operation in processedMethodBody.Instructions) { + foreach (Instruction operation in methodIL.Instructions) { int curBasicBlock = blockIterator.MoveNext (operation); if (knownStacks.ContainsKey (operation.Offset)) { @@ -414,7 +412,7 @@ protected virtual void Scan (MethodBodyInstructionsProvider.ProcessedMethodBody case Code.Ldloc_S: case Code.Ldloca: case Code.Ldloca_S: - ScanLdloc (operation, currentStack, methodBody, locals); + ScanLdloc (operation, currentStack, methodIL, locals); ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset); break; @@ -579,7 +577,7 @@ protected virtual void Scan (MethodBodyInstructionsProvider.ProcessedMethodBody case Code.Stloc_1: case Code.Stloc_2: case Code.Stloc_3: - ScanStloc (operation, currentStack, methodBody, locals, curBasicBlock); + ScanStloc (operation, currentStack, methodIL, locals, curBasicBlock); ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset); break; @@ -702,9 +700,9 @@ protected virtual void Scan (MethodBodyInstructionsProvider.ProcessedMethodBody } } - private static void ScanExceptionInformation (Dictionary> knownStacks, MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) + private static void ScanExceptionInformation (Dictionary> knownStacks, MethodIL methodIL) { - foreach (ExceptionHandler exceptionClause in methodBody.ExceptionHandlers) { + foreach (ExceptionHandler exceptionClause in methodIL.ExceptionHandlers) { Stack catchStack = new Stack (1); catchStack.Push (new StackSlot ()); @@ -758,12 +756,12 @@ private void ScanStarg ( private void ScanLdloc ( Instruction operation, Stack currentStack, - MethodBody methodBody, + MethodIL methodIL, LocalVariableStore locals) { - VariableDefinition localDef = GetLocalDef (operation, methodBody.Variables); + VariableDefinition localDef = GetLocalDef (operation, methodIL.Variables); if (localDef == null) { - PushUnknownAndWarnAboutInvalidIL (currentStack, methodBody, operation.Offset); + PushUnknownAndWarnAboutInvalidIL (currentStack, methodIL.Body, operation.Offset); return; } @@ -821,14 +819,14 @@ void ScanLdtoken (Instruction operation, Stack currentStack) private void ScanStloc ( Instruction operation, Stack currentStack, - MethodBody methodBody, + MethodIL methodIL, LocalVariableStore locals, int curBasicBlock) { - StackSlot valueToStore = PopUnknown (currentStack, 1, methodBody, operation.Offset); - VariableDefinition localDef = GetLocalDef (operation, methodBody.Variables); + StackSlot valueToStore = PopUnknown (currentStack, 1, methodIL.Body, operation.Offset); + VariableDefinition localDef = GetLocalDef (operation, methodIL.Variables); if (localDef == null) { - WarnAboutInvalidILInMethod (methodBody, operation.Offset); + WarnAboutInvalidILInMethod (methodIL.Body, operation.Offset); return; } diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs index 6c27bc62e95d..804e5a21430d 100644 --- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs +++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs @@ -61,21 +61,21 @@ public ReflectionMethodBodyScanner (LinkContext context, MarkStep parent, Messag TrimAnalysisPatterns = new TrimAnalysisPatternStore (MultiValueLattice, context); } - public override void InterproceduralScan (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) + public override void InterproceduralScan (MethodIL methodIL) { - base.InterproceduralScan (methodBody); + base.InterproceduralScan (methodIL); var reflectionMarker = new ReflectionMarker (_context, _markStep, enabled: true); TrimAnalysisPatterns.MarkAndProduceDiagnostics (reflectionMarker, _markStep); } - protected override void Scan (MethodBodyInstructionsProvider.ProcessedMethodBody methodBody, ref InterproceduralState interproceduralState) + protected override void Scan (MethodIL methodIL, ref InterproceduralState interproceduralState) { - _origin = new MessageOrigin (methodBody.Method); - base.Scan (methodBody, ref interproceduralState); + _origin = new MessageOrigin (methodIL.Method); + base.Scan (methodIL, ref interproceduralState); - if (!methodBody.Method.ReturnsVoid ()) { - var method = methodBody.Method; + if (!methodIL.Method.ReturnsVoid ()) { + var method = methodIL.Method; var methodReturnValue = _annotations.GetMethodReturnValue (method); if (methodReturnValue.DynamicallyAccessedMemberTypes != 0) HandleAssignmentPattern (_origin, ReturnValue, methodReturnValue); diff --git a/src/linker/Linker.Dataflow/ScannerExtensions.cs b/src/linker/Linker.Dataflow/ScannerExtensions.cs index 8062d95bf944..7b8dbba87eb6 100644 --- a/src/linker/Linker.Dataflow/ScannerExtensions.cs +++ b/src/linker/Linker.Dataflow/ScannerExtensions.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using Mono.Cecil.Cil; -using Mono.Linker.Steps; namespace Mono.Linker.Dataflow { @@ -17,10 +16,10 @@ public static bool IsControlFlowInstruction (in this OpCode opcode) || (opcode.FlowControl == FlowControl.Return && opcode.Code != Code.Ret); } - public static HashSet ComputeBranchTargets (this MethodBodyInstructionsProvider.ProcessedMethodBody methodBody) + public static HashSet ComputeBranchTargets (this MethodIL methodIL) { HashSet branchTargets = new HashSet (); - foreach (Instruction operation in methodBody.Instructions) { + foreach (Instruction operation in methodIL.Instructions) { if (!operation.OpCode.IsControlFlowInstruction ()) continue; Object value = operation.Operand; @@ -32,7 +31,7 @@ public static HashSet ComputeBranchTargets (this MethodBodyInstructionsProv } } } - foreach (ExceptionHandler einfo in methodBody.ExceptionHandlers) { + foreach (ExceptionHandler einfo in methodIL.ExceptionHandlers) { if (einfo.HandlerType == ExceptionHandlerType.Filter) { branchTargets.Add (einfo.FilterStart.Offset); } diff --git a/src/linker/Linker.Steps/CodeRewriterStep.cs b/src/linker/Linker.Steps/CodeRewriterStep.cs index 5877f8470f43..566a7ab1ae13 100644 --- a/src/linker/Linker.Steps/CodeRewriterStep.cs +++ b/src/linker/Linker.Steps/CodeRewriterStep.cs @@ -204,7 +204,9 @@ static void StubComplexBody (MethodDefinition method, MethodBody body, LinkerILP case MetadataType.MVar: case MetadataType.ValueType: var vd = new VariableDefinition (method.ReturnType); +#pragma warning disable RS0030 // Anything after MarkStep should not use ILProvider since all methods are guaranteed processed body.Variables.Add (vd); +#pragma warning restore RS0030 body.InitLocals = true; il.Emit (OpCodes.Ldloca_S, vd); diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 4600759c1573..40a14181ea3a 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2569,7 +2569,7 @@ bool IsNonEmptyStaticConstructor (MethodDefinition method) if (!method.HasBody || !method.IsIL) return true; - var body = Context.MethodBodyInstructionsProvider.GetMethodBody (method.Body); + var body = Context.GetMethodIL (method.Body); if (body.Body.CodeSize != 1) return true; @@ -2857,7 +2857,7 @@ protected bool MarkFields (TypeDefinition type, bool includeStatic, in Dependenc if (body == null) continue; - foreach (var ins in Context.MethodBodyInstructionsProvider.GetMethodBody (body).Instructions) { + foreach (var ins in Context.GetMethodIL (body).Instructions) { if (ins?.Operand == field) return property; } @@ -2949,7 +2949,7 @@ bool ShouldWarnForReflectionAccessToCompilerGeneratedCode (MethodDefinition meth // the reflection scanner. Checking this will also mark direct dependencies of the method body, if it // hasn't been marked already. A cache ensures this only happens once for the method, whether or not // it is accessed via reflection. - return CheckRequiresReflectionMethodBodyScanner (Context.MethodBodyInstructionsProvider.GetMethodBody (method.Body)); + return CheckRequiresReflectionMethodBodyScanner (Context.GetMethodIL (method.Body)); } void ProcessAnalysisAnnotationsForMethod (MethodDefinition method, DependencyKind dependencyKind, in MessageOrigin origin) @@ -3480,7 +3480,7 @@ internal void MarkMethodIfNotNull (MethodReference method, in DependencyInfo rea protected virtual void MarkMethodBody (MethodBody body) { - var processedMethodBody = Context.MethodBodyInstructionsProvider.GetMethodBody (body); + var processedMethodBody = Context.GetMethodIL (body); if (Context.IsOptimizationEnabled (CodeOptimizations.UnreachableBodies, body.Method) && IsUnreachableBody (processedMethodBody)) { MarkAndCacheConvertToThrowExceptionCtor (new DependencyInfo (DependencyKind.UnreachableBodyRequirement, body.Method)); @@ -3503,14 +3503,14 @@ protected virtual void MarkMethodBody (MethodBody body) MarkReflectionLikeDependencies (processedMethodBody, requiresReflectionMethodBodyScanner); } - bool CheckRequiresReflectionMethodBodyScanner (MethodBodyInstructionsProvider.ProcessedMethodBody body) + bool CheckRequiresReflectionMethodBodyScanner (MethodIL methodIL) { // This method is only called on reflection access to compiler-generated methods. // This should be uncommon, so don't cache the result. - if (ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForMethodBody (Context, body.Method)) + if (ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForMethodBody (Context, methodIL.Method)) return true; - foreach (Instruction instruction in body.Instructions) { + foreach (Instruction instruction in methodIL.Instructions) { switch (instruction.OpCode.OperandType) { case OperandType.InlineField: if (InstructionRequiresReflectionMethodBodyScannerForFieldAccess (instruction)) @@ -3530,64 +3530,64 @@ bool CheckRequiresReflectionMethodBodyScanner (MethodBodyInstructionsProvider.Pr // It computes the same value, while also marking as it goes, as an optimization. // This should only be called behind a check to IsProcessed for the method or corresponding user method, // to avoid recursion. - bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodBodyInstructionsProvider.ProcessedMethodBody body) + bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodIL methodIL) { #if DEBUG - if (!Annotations.IsProcessed (body.Method)) { - Debug.Assert (CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (body.Method)); - MethodDefinition owningMethod = body.Method; + if (!Annotations.IsProcessed (methodIL.Method)) { + Debug.Assert (CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (methodIL.Method)); + MethodDefinition owningMethod = methodIL.Method; while (Context.CompilerGeneratedState.TryGetOwningMethodForCompilerGeneratedMember (owningMethod, out var owner)) owningMethod = owner; - Debug.Assert (owningMethod != body.Method); + Debug.Assert (owningMethod != methodIL.Method); Debug.Assert (Annotations.IsProcessed (owningMethod)); } #endif // This may get called multiple times for compiler-generated code: once for // reflection access, and once as part of the interprocedural scan of the user method. // This check ensures that we only do the work and produce warnings once. - if (_compilerGeneratedMethodRequiresScanner.TryGetValue (body.Body, out bool requiresReflectionMethodBodyScanner)) + if (_compilerGeneratedMethodRequiresScanner.TryGetValue (methodIL.Body, out bool requiresReflectionMethodBodyScanner)) return requiresReflectionMethodBodyScanner; - foreach (VariableDefinition var in body.Variables) - MarkType (var.VariableType, new DependencyInfo (DependencyKind.VariableType, body.Method)); + foreach (VariableDefinition var in methodIL.Variables) + MarkType (var.VariableType, new DependencyInfo (DependencyKind.VariableType, methodIL.Method)); - foreach (ExceptionHandler eh in body.ExceptionHandlers) + foreach (ExceptionHandler eh in methodIL.ExceptionHandlers) if (eh.HandlerType == ExceptionHandlerType.Catch) - MarkType (eh.CatchType, new DependencyInfo (DependencyKind.CatchType, body.Method)); + MarkType (eh.CatchType, new DependencyInfo (DependencyKind.CatchType, methodIL.Method)); requiresReflectionMethodBodyScanner = - ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForMethodBody (Context, body.Method); - using var _ = ScopeStack.PushScope (new MessageOrigin (body.Method)); - foreach (Instruction instruction in body.Instructions) - MarkInstruction (instruction, body.Method, ref requiresReflectionMethodBodyScanner); + ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForMethodBody (Context, methodIL.Method); + using var _ = ScopeStack.PushScope (new MessageOrigin (methodIL.Method)); + foreach (Instruction instruction in methodIL.Instructions) + MarkInstruction (instruction, methodIL.Method, ref requiresReflectionMethodBodyScanner); - MarkInterfacesNeededByBodyStack (body); + MarkInterfacesNeededByBodyStack (methodIL); - if (CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (body.Method)) - _compilerGeneratedMethodRequiresScanner.Add (body.Body, requiresReflectionMethodBodyScanner); + if (CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (methodIL.Method)) + _compilerGeneratedMethodRequiresScanner.Add (methodIL.Body, requiresReflectionMethodBodyScanner); - PostMarkMethodBody (body.Body); + PostMarkMethodBody (methodIL.Body); - Debug.Assert (requiresReflectionMethodBodyScanner == CheckRequiresReflectionMethodBodyScanner (body)); + Debug.Assert (requiresReflectionMethodBodyScanner == CheckRequiresReflectionMethodBodyScanner (methodIL)); return requiresReflectionMethodBodyScanner; } - bool IsUnreachableBody (MethodBodyInstructionsProvider.ProcessedMethodBody body) + bool IsUnreachableBody (MethodIL methodIL) { - return !body.Method.IsStatic - && !Annotations.IsInstantiated (body.Method.DeclaringType) - && MethodBodyScanner.IsWorthConvertingToThrow (body); + return !methodIL.Method.IsStatic + && !Annotations.IsInstantiated (methodIL.Method.DeclaringType) + && MethodBodyScanner.IsWorthConvertingToThrow (methodIL); } partial void PostMarkMethodBody (MethodBody body); - void MarkInterfacesNeededByBodyStack (MethodBodyInstructionsProvider.ProcessedMethodBody body) + void MarkInterfacesNeededByBodyStack (MethodIL methodIL) { // If a type could be on the stack in the body and an interface it implements could be on the stack on the body // then we need to mark that interface implementation. When this occurs it is not safe to remove the interface implementation from the type // even if the type is never instantiated - var implementations = new InterfacesOnStackScanner (Context).GetReferencedInterfaces (body); + var implementations = new InterfacesOnStackScanner (Context).GetReferencedInterfaces (methodIL); if (implementations == null) return; @@ -3720,24 +3720,24 @@ protected internal virtual bool ProcessReflectionDependency (MethodBody body, In // // Tries to mark additional dependencies used in reflection like calls (e.g. typeof (MyClass).GetField ("fname")) // - protected virtual void MarkReflectionLikeDependencies (MethodBodyInstructionsProvider.ProcessedMethodBody body, bool requiresReflectionMethodBodyScanner) + protected virtual void MarkReflectionLikeDependencies (MethodIL methodIL, bool requiresReflectionMethodBodyScanner) { - Debug.Assert (!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (body.Method)); + Debug.Assert (!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (methodIL.Method)); // requiresReflectionMethodBodyScanner tells us whether the method body itself requires a dataflow scan. // If the method body owns any compiler-generated code, we might still need to do a scan of it together with // all of the compiler-generated code it owns, so first check any compiler-generated callees. - if (Context.CompilerGeneratedState.TryGetCompilerGeneratedCalleesForUserMethod (body.Method, out List? compilerGeneratedCallees)) { + if (Context.CompilerGeneratedState.TryGetCompilerGeneratedCalleesForUserMethod (methodIL.Method, out List? compilerGeneratedCallees)) { foreach (var compilerGeneratedCallee in compilerGeneratedCallees) { switch (compilerGeneratedCallee) { case MethodDefinition nestedFunction: if (nestedFunction.Body is MethodBody nestedBody) - requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (Context.MethodBodyInstructionsProvider.GetMethodBody (nestedBody)); + requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (Context.GetMethodIL (nestedBody)); break; case TypeDefinition stateMachineType: foreach (var method in stateMachineType.Methods) { if (method.Body is MethodBody stateMachineBody) - requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (Context.MethodBodyInstructionsProvider.GetMethodBody (stateMachineBody)); + requiresReflectionMethodBodyScanner |= MarkAndCheckRequiresReflectionMethodBodyScanner (Context.GetMethodIL (stateMachineBody)); } break; default: @@ -3749,9 +3749,9 @@ protected virtual void MarkReflectionLikeDependencies (MethodBodyInstructionsPro if (!requiresReflectionMethodBodyScanner) return; - Debug.Assert (ScopeStack.CurrentScope.Origin.Provider == body.Method); + Debug.Assert (ScopeStack.CurrentScope.Origin.Provider == methodIL.Method); var scanner = new ReflectionMethodBodyScanner (Context, this, ScopeStack.CurrentScope.Origin); - scanner.InterproceduralScan (body); + scanner.InterproceduralScan (methodIL); } protected class AttributeProviderPair diff --git a/src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs b/src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs deleted file mode 100644 index 7d02ccff3c26..000000000000 --- a/src/linker/Linker.Steps/MethodBodyInstructionsProvider.cs +++ /dev/null @@ -1,52 +0,0 @@ -// 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 Mono.Cecil; -using Mono.Cecil.Cil; -using Mono.Collections.Generic; - -namespace Mono.Linker.Steps -{ - public class MethodBodyInstructionsProvider - { - readonly UnreachableBlocksOptimizer _unreachableBlocksOptimizer; - - public readonly struct ProcessedMethodBody - { - private ProcessedMethodBody (MethodBody body) => this.Body = body; - - public readonly MethodBody Body; - - public MethodDefinition Method => Body.Method; - -#pragma warning disable RS0030 // Wrapper which provides safe access to the property - public Collection Instructions => Body.Instructions; -#pragma warning restore RS0030 - -#pragma warning disable RS0030 // Wrapper which provides safe access to the property - public Collection ExceptionHandlers => Body.ExceptionHandlers; -#pragma warning restore RS0030 - -#pragma warning disable RS0030 // Wrapper which provides safe access to the property - public Collection Variables => Body.Variables; -#pragma warning restore RS0030 - - public static ProcessedMethodBody Create (MethodBody body) => new ProcessedMethodBody (body); - } - - public MethodBodyInstructionsProvider(LinkContext context) - { - _unreachableBlocksOptimizer = new UnreachableBlocksOptimizer(context); - } - - public ProcessedMethodBody GetMethodBody (MethodBody methodBody) - => GetMethodBody (methodBody.Method); - - public ProcessedMethodBody GetMethodBody (MethodDefinition method) - { - _unreachableBlocksOptimizer.ProcessMethod (method); - - return ProcessedMethodBody.Create (method.Body); - } - } -} diff --git a/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs b/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs index f551ab58bd8c..179f15d87efb 100644 --- a/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs +++ b/src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs @@ -24,7 +24,6 @@ public class UnreachableBlocksOptimizer readonly LinkContext _context; readonly Dictionary _cache_method_results = new (2048); readonly Stack _resursion_guard = new (); - readonly HashSet _processed_methods = new (2048); MethodDefinition? IntPtrSize, UIntPtrSize; @@ -40,9 +39,6 @@ public UnreachableBlocksOptimizer (LinkContext context) /// The method to process public void ProcessMethod (MethodDefinition method) { - if (!_processed_methods.Add (method)) - return; - if (!IsMethodSupported (method)) return; @@ -556,7 +552,8 @@ public BodyReducer (MethodBody body, LinkContext context) public MethodBody Body { get; } #pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly - private Collection Instructions => Body.Instructions; + Collection Instructions => Body.Instructions; + Collection ExceptionHandlers => Body.ExceptionHandlers; #pragma warning restore RS0030 public int InstructionsReplaced { get; set; } @@ -1053,8 +1050,7 @@ BitArray GetReachableInstructionsMap (out List? unreachableHan exceptionHandlersChecked = true; var instrs = Instructions; -#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly - foreach (var handler in Body.ExceptionHandlers) { + foreach (var handler in ExceptionHandlers) { int start = instrs.IndexOf (handler.TryStart); int end = instrs.IndexOf (handler.TryEnd) - 1; @@ -1071,7 +1067,6 @@ BitArray GetReachableInstructionsMap (out List? unreachableHan if (handler.FilterStart != null) condBranches.Push (GetInstructionIndex (handler.FilterStart)); } -#pragma warning restore RS0030 if (condBranches?.Count > 0) { i = condBranches.Pop (); @@ -1167,7 +1162,9 @@ struct BodySweeper { readonly MethodBody body; #pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly - private Collection Instructions => body.Instructions; + Collection Instructions => body.Instructions; + Collection Variables => body.Variables; + Collection ExceptionHandlers => body.ExceptionHandlers; #pragma warning restore RS0030 readonly BitArray reachable; readonly List? unreachableExceptionHandlers; @@ -1330,7 +1327,7 @@ void CleanRemovedVariables (List variables) } variables.Sort ((a, b) => b.Index.CompareTo (a.Index)); - var body_variables = body.Variables; + var body_variables = Variables; foreach (var variable in variables) { var index = body_variables.IndexOf (variable); @@ -1354,10 +1351,8 @@ void CleanExceptionHandlers () if (unreachableExceptionHandlers == null) return; -#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly foreach (var eh in unreachableExceptionHandlers) - body.ExceptionHandlers.Remove (eh); -#pragma warning restore RS0030 + ExceptionHandlers.Remove (eh); } VariableDefinition? GetVariableReference (Instruction instruction) @@ -1365,16 +1360,16 @@ void CleanExceptionHandlers () switch (instruction.OpCode.Code) { case Code.Stloc_0: case Code.Ldloc_0: - return body.Variables[0]; + return Variables[0]; case Code.Stloc_1: case Code.Ldloc_1: - return body.Variables[1]; + return Variables[1]; case Code.Stloc_2: case Code.Ldloc_2: - return body.Variables[2]; + return Variables[2]; case Code.Stloc_3: case Code.Ldloc_3: - return body.Variables[3]; + return Variables[3]; } if (instruction.Operand is VariableReference vr) @@ -1855,8 +1850,12 @@ bool ConvertStackToResult () if (!body.InitLocals) return null; +#pragma warning disable RS0030 // This optimizer is the reason for the banned API, so it needs to use the Cecil directly + var variables = body.Variables; +#pragma warning restore RS0030 + // local variables don't need to be explicitly initialized - return CodeRewriterStep.CreateConstantResultInstruction (context, body.Variables[index].VariableType); + return CodeRewriterStep.CreateConstantResultInstruction (context, variables[index].VariableType); } bool GetOperandConstantValue ([NotNullWhen (true)] out object? value) diff --git a/src/linker/Linker/LinkContext.cs b/src/linker/Linker/LinkContext.cs index abddf3b8aba4..6bd0ff73b748 100644 --- a/src/linker/Linker/LinkContext.cs +++ b/src/linker/Linker/LinkContext.cs @@ -80,6 +80,7 @@ public class LinkContext : IMetadataResolver, ITryResolveMetadata, IDisposable readonly List _cachedWarningMessageContainers; readonly ILogger _logger; readonly Dictionary _isTrimmable; + readonly UnreachableBlocksOptimizer _unreachableBlocksOptimizer; public Pipeline Pipeline { get { return _pipeline; } @@ -189,8 +190,6 @@ internal TypeNameResolver TypeNameResolver { public SerializationMarker SerializationMarker { get; } - public MethodBodyInstructionsProvider MethodBodyInstructionsProvider { get; } - public LinkContext (Pipeline pipeline, ILogger logger, string outputDirectory) { _pipeline = pipeline; @@ -225,7 +224,7 @@ public LinkContext (Pipeline pipeline, ILogger logger, string outputDirectory) GeneralSingleWarn = false; SingleWarn = new Dictionary (); AssembliesWithGeneratedSingleWarning = new HashSet (); - MethodBodyInstructionsProvider = new MethodBodyInstructionsProvider (this); + _unreachableBlocksOptimizer = new UnreachableBlocksOptimizer (this); const CodeOptimizations defaultOptimizations = CodeOptimizations.BeforeFieldInit | @@ -942,6 +941,28 @@ public int GetTargetRuntimeVersion () : null; } + readonly HashSet _processed_bodies_for_method = new HashSet (2048); + + /// + /// Linker applies some optimization on method bodies. For example it can remove dead branches of code + /// based on constant propagation. To avoid overmarking, all code which processes the method's IL + /// should only view the IL after it's been optimized. + /// As such typically MethodDefinition.MethodBody should not be accessed directly on the Cecil object model + /// instead all accesses to method body should go through the ILProvider here + /// which will make sure the IL of the method is fully optimized before it's handed out. + /// + public MethodIL GetMethodIL (Cecil.Cil.MethodBody methodBody) + => GetMethodIL (methodBody.Method); + + public MethodIL GetMethodIL (MethodDefinition method) + { + if (_processed_bodies_for_method.Add (method)) { + _unreachableBlocksOptimizer.ProcessMethod (method); + } + + return MethodIL.Create (method.Body); + } + readonly HashSet unresolved_reported = new (); readonly HashSet unresolved_exported_types_reported = new (); diff --git a/src/linker/Linker/MethodBodyScanner.cs b/src/linker/Linker/MethodBodyScanner.cs index 22060cc8a0e5..535b1244c527 100644 --- a/src/linker/Linker/MethodBodyScanner.cs +++ b/src/linker/Linker/MethodBodyScanner.cs @@ -1,19 +1,17 @@ // 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.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; using Mono.Cecil; using Mono.Cecil.Cil; -using Mono.Linker.Steps; namespace Mono.Linker { public static class MethodBodyScanner { - public static bool IsWorthConvertingToThrow (MethodBodyInstructionsProvider.ProcessedMethodBody body) + public static bool IsWorthConvertingToThrow (MethodIL body) { // Some bodies are cheaper size wise to leave alone than to convert to a throw Instruction? previousMeaningful = null; @@ -65,9 +63,9 @@ public InterfacesOnStackScanner (LinkContext context) this.context = context; } - public IEnumerable<(InterfaceImplementation, TypeDefinition)>? GetReferencedInterfaces (MethodBodyInstructionsProvider.ProcessedMethodBody body) + public IEnumerable<(InterfaceImplementation, TypeDefinition)>? GetReferencedInterfaces (MethodIL methodIL) { - var possibleStackTypes = AllPossibleStackTypes (body); + var possibleStackTypes = AllPossibleStackTypes (methodIL); if (possibleStackTypes.Count == 0) return null; @@ -96,23 +94,23 @@ public InterfacesOnStackScanner (LinkContext context) return interfaceImplementations; } - HashSet AllPossibleStackTypes (MethodBodyInstructionsProvider.ProcessedMethodBody body) + HashSet AllPossibleStackTypes (MethodIL methodIL) { var types = new HashSet (); - foreach (VariableDefinition var in body.Variables) + foreach (VariableDefinition var in methodIL.Variables) AddIfResolved (types, var.VariableType); - foreach (var param in body.Method.GetParameters ()) + foreach (var param in methodIL.Method.GetParameters ()) AddIfResolved (types, param.ParameterType); - foreach (ExceptionHandler eh in body.ExceptionHandlers) { + foreach (ExceptionHandler eh in methodIL.ExceptionHandlers) { if (eh.HandlerType == ExceptionHandlerType.Catch) { AddIfResolved (types, eh.CatchType); } } - foreach (Instruction instruction in body.Instructions) { + foreach (Instruction instruction in methodIL.Instructions) { if (instruction.Operand is FieldReference fieldReference) { if (context.TryResolve (fieldReference)?.FieldType is TypeReference fieldType) AddIfResolved (types, fieldType); diff --git a/src/linker/Linker/MethodIL.cs b/src/linker/Linker/MethodIL.cs new file mode 100644 index 000000000000..1bb3ff2b0792 --- /dev/null +++ b/src/linker/Linker/MethodIL.cs @@ -0,0 +1,39 @@ +// 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 Mono.Cecil; +using Mono.Cecil.Cil; +using Mono.Collections.Generic; + +namespace Mono.Linker +{ + /// + /// This is a wrapper which should be used by anything accessing method's body - see LinkContext.GetMethodIL for more details. + /// Any accesses made throught this wrapper are considered "safe"/OK since the wrapper is only created + /// once all of the optimizations are applied. + /// + public readonly record struct MethodIL + { + public MethodIL () => throw new InvalidOperationException (); + MethodIL (MethodBody body) => this.Body = body; + + public readonly MethodBody Body; + + public MethodDefinition Method => Body.Method; + +#pragma warning disable RS0030 // Wrapper which provides safe access to the property + public Collection Instructions => Body.Instructions; +#pragma warning restore RS0030 + +#pragma warning disable RS0030 // Wrapper which provides safe access to the property + public Collection ExceptionHandlers => Body.ExceptionHandlers; +#pragma warning restore RS0030 + +#pragma warning disable RS0030 // Wrapper which provides safe access to the property + public Collection Variables => Body.Variables; +#pragma warning restore RS0030 + + public static MethodIL Create (MethodBody body) => new MethodIL (body); + } +} From 1337c34626971caa523a397cade05ed4c85738bf Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 1 Nov 2022 10:14:54 -0700 Subject: [PATCH 5/5] PR feedback --- src/linker/Linker.Dataflow/CompilerGeneratedState.cs | 2 +- src/linker/Linker.Steps/MarkStep.cs | 4 ++-- src/linker/Linker/MethodIL.cs | 8 -------- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs index d5bfc5d8d264..36950fb9c03f 100644 --- a/src/linker/Linker.Dataflow/CompilerGeneratedState.cs +++ b/src/linker/Linker.Dataflow/CompilerGeneratedState.cs @@ -145,7 +145,7 @@ void ProcessMethod (MethodDefinition method) // Discover calls or references to lambdas or local functions. This includes // calls to local functions, and lambda assignments (which use ldftn). if (method.Body != null) { - foreach (var instruction in _context.GetMethodIL (method.Body).Instructions) { + foreach (var instruction in _context.GetMethodIL (method).Instructions) { switch (instruction.OpCode.OperandType) { case OperandType.InlineMethod: { MethodDefinition? referencedMethod = _context.TryResolve ((MethodReference) instruction.Operand); diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 40a14181ea3a..646e2e6bd96d 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2569,7 +2569,7 @@ bool IsNonEmptyStaticConstructor (MethodDefinition method) if (!method.HasBody || !method.IsIL) return true; - var body = Context.GetMethodIL (method.Body); + var body = Context.GetMethodIL (method); if (body.Body.CodeSize != 1) return true; @@ -2949,7 +2949,7 @@ bool ShouldWarnForReflectionAccessToCompilerGeneratedCode (MethodDefinition meth // the reflection scanner. Checking this will also mark direct dependencies of the method body, if it // hasn't been marked already. A cache ensures this only happens once for the method, whether or not // it is accessed via reflection. - return CheckRequiresReflectionMethodBodyScanner (Context.GetMethodIL (method.Body)); + return CheckRequiresReflectionMethodBodyScanner (Context.GetMethodIL (method)); } void ProcessAnalysisAnnotationsForMethod (MethodDefinition method, DependencyKind dependencyKind, in MessageOrigin origin) diff --git a/src/linker/Linker/MethodIL.cs b/src/linker/Linker/MethodIL.cs index 1bb3ff2b0792..fc97a6e24334 100644 --- a/src/linker/Linker/MethodIL.cs +++ b/src/linker/Linker/MethodIL.cs @@ -1,7 +1,6 @@ // 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 Mono.Cecil; using Mono.Cecil.Cil; using Mono.Collections.Generic; @@ -15,7 +14,6 @@ namespace Mono.Linker /// public readonly record struct MethodIL { - public MethodIL () => throw new InvalidOperationException (); MethodIL (MethodBody body) => this.Body = body; public readonly MethodBody Body; @@ -24,13 +22,7 @@ public readonly record struct MethodIL #pragma warning disable RS0030 // Wrapper which provides safe access to the property public Collection Instructions => Body.Instructions; -#pragma warning restore RS0030 - -#pragma warning disable RS0030 // Wrapper which provides safe access to the property public Collection ExceptionHandlers => Body.ExceptionHandlers; -#pragma warning restore RS0030 - -#pragma warning disable RS0030 // Wrapper which provides safe access to the property public Collection Variables => Body.Variables; #pragma warning restore RS0030