[release/7.0] Fix branch removal in compiler generated code

src/linker/BannedSymbols.txt

@@ -1,2 +1,6 @@
 T:Mono.Cecil.Cil.ILProcessor;Use LinkerILProcessor instead
 M:Mono.Cecil.TypeReference.Resolve();Use LinkContext.Resolve and LinkContext.TryResolve helpers instead
+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.Variables;Use LinkContext.MethodBodyInstructionProvider instead
+M:Mono.Linker.Steps.ILProvider/MethodIL.Create;Use ILProvider GetMethodIL instead

src/linker/Linker.Dataflow/CompilerGeneratedState.cs

@@ -139,7 +139,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.GetMethodIL (method).Instructions) {
  switch (instruction.OpCode.OperandType) {
  case OperandType.InlineMethod: {
  MethodDefinition? referencedMethod = _context.TryResolve ((MethodReference) instruction.Operand);
@@ -348,7 +348,7 @@ void MapGeneratedTypeTypeParameters (TypeDefinition generatedType)
 
  GenericInstanceType? ScanForInit (TypeDefinition compilerGeneratedType, MethodBody body)
  {
- foreach (var instr in body.Instructions) {
+ foreach (var instr in _context.GetMethodIL (body).Instructions) {
  bool handled = false;
  switch (instr.OpCode.Code) {
  case Code.Initobj:

src/linker/Linker.Dataflow/FlowAnnotations.cs

@@ -422,7 +422,7 @@ bool ScanMethodBodyForFieldAccess (MethodBody body, bool write, out FieldDefinit
 
  FieldReference? foundReference = null;
 
- foreach (Instruction instruction in body.Instructions) {
+ foreach (Instruction instruction in _context.GetMethodIL (body).Instructions) {
  switch (instruction.OpCode.Code) {
  case Code.Ldsfld when !write:
  case Code.Ldfld when !write:

src/linker/Linker.Dataflow/InterproceduralState.cs

@@ -14,23 +14,25 @@
 
 namespace Mono.Linker.Dataflow
 {
- // Wrapper that implements IEquatable for MethodBody.
- readonly record struct MethodBodyValue (MethodBody 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<InterproceduralState>
  {
- public ValueSet<MethodBodyValue> MethodBodies;
+ public ValueSet<MethodIL> MethodBodies;
  public HoistedLocalState HoistedLocals;
  readonly InterproceduralStateLattice lattice;
 
- public InterproceduralState (ValueSet<MethodBodyValue> methodBodies, HoistedLocalState hoistedLocals, InterproceduralStateLattice lattice)
+ public InterproceduralState (ValueSet<MethodIL> 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);
 
@@ -43,23 +45,28 @@ public void TrackMethod (MethodDefinition method)
  }
 
  public void TrackMethod (MethodBody methodBody)
+ {
+ TrackMethod (lattice.Context.GetMethodIL (methodBody));
+ }
+
+ public void TrackMethod (MethodIL methodIL)
  {
  // Work around the fact that ValueSet is readonly
- var methodsList = new List<MethodBodyValue> (MethodBodies);
- methodsList.Add (new MethodBodyValue (methodBody));
+ var methodsList = new List<MethodIL> (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 (stateMachineMethodBody));
+ methodsList.Add (lattice.Context.GetMethodIL (stateMachineMethodBody));
  }
  }
 
- MethodBodies = new ValueSet<MethodBodyValue> (methodsList);
+ MethodBodies = new ValueSet<MethodIL> (methodsList);
  }
 
  public void SetHoistedLocal (HoistedLocalKey key, MultiValue value)
@@ -76,15 +83,17 @@ public MultiValue GetHoistedLocal (HoistedLocalKey key)
  => HoistedLocals.Get (key);
  }
 
- struct InterproceduralStateLattice : ILattice<InterproceduralState>
+ readonly struct InterproceduralStateLattice : ILattice<InterproceduralState>
  {
- public readonly ValueSetLattice<MethodBodyValue> MethodBodyLattice;
+ public readonly ValueSetLattice<MethodIL> MethodBodyLattice;
  public readonly DictionaryLattice<HoistedLocalKey, MultiValue, ValueSetLattice<SingleValue>> HoistedLocalsLattice;
+ public readonly LinkContext Context;
 
  public InterproceduralStateLattice (
- ValueSetLattice<MethodBodyValue> methodBodyLattice,
- DictionaryLattice<HoistedLocalKey, MultiValue, ValueSetLattice<SingleValue>> hoistedLocalsLattice)
- => (MethodBodyLattice, HoistedLocalsLattice) = (methodBodyLattice, hoistedLocalsLattice);
+ ValueSetLattice<MethodIL> methodBodyLattice,
+ DictionaryLattice<HoistedLocalKey, MultiValue, ValueSetLattice<SingleValue>> hoistedLocalsLattice,
+ LinkContext context)
+ => (MethodBodyLattice, HoistedLocalsLattice, Context) = (methodBodyLattice, hoistedLocalsLattice, context);
 
  public InterproceduralState Top => new InterproceduralState (MethodBodyLattice.Top, HoistedLocalsLattice.Top, this);
 

src/linker/Linker.Dataflow/MethodBodyScanner.cs

@@ -51,7 +51,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; }
@@ -151,9 +151,9 @@ private struct BasicBlockIterator
  int _currentBlockIndex;
  bool _foundEndOfPrevBlock;
 
- public BasicBlockIterator (MethodBody methodBody)
+ public BasicBlockIterator (MethodIL methodIL)
  {
- _methodBranchTargets = methodBody.ComputeBranchTargets ();
+ _methodBranchTargets = methodIL.ComputeBranchTargets ();
  _currentBlockIndex = -1;
  _foundEndOfPrevBlock = true;
  }
@@ -226,25 +226,25 @@ protected static void StoreMethodLocalValue<KeyType> (
 
  // 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 (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.
  // Definite assignment should ensure that there is no way for this to be an analysis hole.
  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
@@ -274,22 +274,23 @@ void TrackNestedFunctionReference (MethodReference referencedMethod, ref Interpr
  interproceduralState.TrackMethod (method);
  }
 
- protected virtual void Scan (MethodBody methodBody, ref InterproceduralState interproceduralState)
+ protected virtual void Scan (MethodIL methodIL, ref InterproceduralState interproceduralState)
  {
+ MethodBody methodBody = methodIL.Body;
  MethodDefinition thisMethod = methodBody.Method;
 
- LocalVariableStore locals = new (methodBody.Variables.Count);
+ LocalVariableStore locals = new (methodIL.Variables.Count);
 
  Dictionary<int, Stack<StackSlot>> knownStacks = new Dictionary<int, Stack<StackSlot>> ();
  Stack<StackSlot>? currentStack = new Stack<StackSlot> (methodBody.MaxStackSize);
 
- ScanExceptionInformation (knownStacks, methodBody);
+ ScanExceptionInformation (knownStacks, methodIL);
 
- BasicBlockIterator blockIterator = new BasicBlockIterator (methodBody);
+ BasicBlockIterator blockIterator = new BasicBlockIterator (methodIL);
 
  ReturnValue = new ();
- foreach (Instruction operation in methodBody.Instructions) {
- ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset);
+ foreach (Instruction operation in methodIL.Instructions) {
+ ValidateNoReferenceToReference (locals, methodBody.Method, operation.Offset); 
  int curBasicBlock = blockIterator.MoveNext (operation);
 
  if (knownStacks.ContainsKey (operation.Offset)) {
@@ -403,7 +404,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
  case Code.Ldarg_S:
  case Code.Ldarga:
  case Code.Ldarga_S:
- ScanLdarg (operation, currentStack, thisMethod, methodBody);
+ ScanLdarg (operation, currentStack, thisMethod, methodIL);
  break;
 
  case Code.Ldloc:
@@ -414,7 +415,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
  case Code.Ldloc_S:
  case Code.Ldloca:
  case Code.Ldloca_S:
- ScanLdloc (operation, currentStack, methodBody, locals);
+ ScanLdloc (operation, currentStack, methodIL, locals);
  break;
 
  case Code.Ldstr: {
@@ -577,7 +578,7 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
  case Code.Stloc_1:
  case Code.Stloc_2:
  case Code.Stloc_3:
- ScanStloc (operation, currentStack, methodBody, locals, curBasicBlock);
+ ScanStloc (operation, currentStack, methodIL, locals, curBasicBlock);
  break;
 
  case Code.Constrained:
@@ -697,9 +698,9 @@ protected virtual void Scan (MethodBody methodBody, ref InterproceduralState int
  }
  }
 
- private static void ScanExceptionInformation (Dictionary<int, Stack<StackSlot>> knownStacks, MethodBody methodBody)
+ private static void ScanExceptionInformation (Dictionary<int, Stack<StackSlot>> knownStacks, MethodIL methodIL)
  {
- foreach (ExceptionHandler exceptionClause in methodBody.ExceptionHandlers) {
+ foreach (ExceptionHandler exceptionClause in methodIL.ExceptionHandlers) {
  Stack<StackSlot> catchStack = new Stack<StackSlot> (1);
  catchStack.Push (new StackSlot ());
 
@@ -715,7 +716,7 @@ private static void ScanExceptionInformation (Dictionary<int, Stack<StackSlot>>
 
  protected abstract SingleValue GetMethodParameterValue (MethodDefinition method, int parameterIndex);
 
- private void ScanLdarg (Instruction operation, Stack<StackSlot> currentStack, MethodDefinition thisMethod, MethodBody methodBody)
+ private void ScanLdarg (Instruction operation, Stack<StackSlot> currentStack, MethodDefinition thisMethod, MethodIL methodBody)
  {
  Code code = operation.OpCode.Code;
 
@@ -741,7 +742,7 @@ private void ScanLdarg (Instruction operation, Stack<StackSlot> currentStack, Me
  } else {
  var paramDefinition = (ParameterDefinition) operation.Operand;
  if (thisMethod.HasImplicitThis ()) {
- if (paramDefinition == methodBody.ThisParameter) {
+ if (paramDefinition == methodBody.Body.ThisParameter) {
  paramNum = 0;
  } else {
  paramNum = paramDefinition.Index + 1;
@@ -781,12 +782,12 @@ private void ScanStarg (
  private void ScanLdloc (
  Instruction operation,
  Stack<StackSlot> 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;
  }
 
@@ -844,14 +845,14 @@ void ScanLdtoken (Instruction operation, Stack<StackSlot> currentStack)
  private void ScanStloc (
  Instruction operation,
  Stack<StackSlot> 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;
  }
 

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 (MethodBody 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 (MethodBody 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);

src/linker/Linker.Dataflow/ScannerExtensions.cs

@@ -16,10 +16,10 @@ public static bool IsControlFlowInstruction (in this OpCode opcode)
  || (opcode.FlowControl == FlowControl.Return && opcode.Code != Code.Ret);
  }
 
- public static HashSet<int> ComputeBranchTargets (this MethodBody methodBody)
+ public static HashSet<int> ComputeBranchTargets (this MethodIL methodIL)
  {
  HashSet<int> branchTargets = new HashSet<int> ();
- foreach (Instruction operation in methodBody.Instructions) {
+ foreach (Instruction operation in methodIL.Instructions) {
  if (!operation.OpCode.IsControlFlowInstruction ())
  continue;
  Object value = operation.Operand;
@@ -31,7 +31,7 @@ public static HashSet<int> ComputeBranchTargets (this MethodBody methodBody)
  }
  }
  }
- foreach (ExceptionHandler einfo in methodBody.ExceptionHandlers) {
+ foreach (ExceptionHandler einfo in methodIL.ExceptionHandlers) {
  if (einfo.HandlerType == ExceptionHandlerType.Filter) {
  branchTargets.Add (einfo.FilterStart.Offset);
  }

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));

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;
 
@@ -199,7 +202,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);