Implement a custom ILProcessor and make it auto-fix IL corruptions

src/linker/Linker.Steps/CodeRewriterStep.cs

@@ -39,7 +39,7 @@ void ProcessType (TypeDefinition type)
 		void AddFieldsInitializations (TypeDefinition type)
 		{
 			Instruction ret;
-			ILProcessor processor;
+			LinkerILProcessor processor;
 
 			var cctor = type.Methods.FirstOrDefault (MethodDefinitionExtensions.IsStaticConstructor);
 			if (cctor == null) {
@@ -51,13 +51,13 @@ void AddFieldsInitializations (TypeDefinition type)
 
 				type.Methods.Add (method);
 
-				processor = method.Body.GetILProcessor ();
+				processor = method.Body.GetLinkerILProcessor ();
 				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.GetILProcessor ();
+				processor = cctor.Body.GetLinkerILProcessor ();
 
 				for (int i = 0; i < body.Instructions.Count; ++i) {
 					var instr = body.Instructions[i];
@@ -122,7 +122,7 @@ protected virtual void RewriteBodyToStub (MethodDefinition method)
 		MethodBody CreateThrowLinkedAwayBody (MethodDefinition method)
 		{
 			var body = new MethodBody (method);
-			var il = body.GetILProcessor ();
+			var il = body.GetLinkerILProcessor ();
 			MethodReference ctor;
 
 			// Makes the body verifiable
@@ -151,7 +151,7 @@ MethodBody CreateStubBody (MethodDefinition method)
 			if (method.HasParameters && method.Parameters.Any (l => l.IsOut))
 				throw new NotSupportedException ($"Cannot replace body of method '{method.GetDisplayName ()}' because it has an out parameter.");
 
-			var il = body.GetILProcessor ();
+			var il = body.GetLinkerILProcessor ();
 			if (method.IsInstanceConstructor () && !method.DeclaringType.IsValueType) {
 				var baseType = Context.Resolve (method.DeclaringType.BaseType);
 				if (baseType is null)
@@ -184,7 +184,7 @@ MethodBody CreateStubBody (MethodDefinition method)
 			return body;
 		}
 
-		static void StubComplexBody (MethodDefinition method, MethodBody body, ILProcessor il)
+		static void StubComplexBody (MethodDefinition method, MethodBody body, LinkerILProcessor il)
 		{
 			switch (method.ReturnType.MetadataType) {
 			case MetadataType.MVar:

src/linker/Linker.Steps/MarkStep.cs

@@ -490,41 +490,14 @@ void ProcessPendingTypeChecks ()
 					continue;
 
 				Instruction instr = item.Instr;
-				ILProcessor ilProcessor = item.Body.GetILProcessor ();
+				LinkerILProcessor ilProcessor = item.Body.GetLinkerILProcessor ();
 
 				ilProcessor.InsertAfter (instr, Instruction.Create (OpCodes.Ldnull));
 				Instruction new_instr = Instruction.Create (OpCodes.Pop);
 				ilProcessor.Replace (instr, new_instr);
-				UpdateBranchTarget (item.Body, instr, new_instr);
 
 				_context.LogMessage ($"Removing typecheck of '{type.FullName}' inside {item.Body.Method.GetDisplayName ()} method");
 			}
-
-			static void UpdateBranchTarget (MethodBody body, Instruction oldTarget, Instruction newTarget)
-			{
-				foreach (var instr in body.Instructions) {
-					switch (instr.OpCode.FlowControl) {
-					case FlowControl.Branch:
-					case FlowControl.Cond_Branch:
-						if (instr.Operand == oldTarget)
-							instr.Operand = newTarget;
-						break;
-					}
-				}
-
-				foreach (var handler in body.ExceptionHandlers) {
-					if (handler.TryStart == oldTarget)
-						handler.TryStart = newTarget;
-					if (handler.TryEnd == oldTarget)
-						handler.TryEnd = newTarget;
-					if (handler.HandlerStart == oldTarget)
-						handler.HandlerStart = newTarget;
-					if (handler.HandlerEnd == oldTarget)
-						handler.HandlerEnd = newTarget;
-					if (handler.FilterStart == oldTarget)
-						handler.FilterStart = newTarget;
-				}
-			}
 		}
 
 		void ProcessQueue ()

src/linker/Linker.Steps/UnreachableBlocksOptimizer.cs

@@ -662,7 +662,7 @@ public bool RewriteBody ()
 					RemoveUnreachableInstructions (reachableInstrs);
 
 				if (nopInstructions != null) {
-					ILProcessor processor = Body.GetILProcessor ();
+					LinkerILProcessor processor = Body.GetLinkerILProcessor ();
 
 					foreach (var instr in nopInstructions)
 						processor.Remove (instr);
@@ -673,7 +673,7 @@ public bool RewriteBody ()
 
 			void RemoveUnreachableInstructions (BitArray reachable)
 			{
-				ILProcessor processor = Body.GetILProcessor ();
+				LinkerILProcessor processor = Body.GetLinkerILProcessor ();
 
 				int removed = 0;
 				for (int i = 0; i < reachable.Count; ++i) {
@@ -1114,7 +1114,7 @@ struct BodySweeper
 			readonly BitArray reachable;
 			readonly List<ExceptionHandler> unreachableExceptionHandlers;
 			readonly LinkContext context;
-			ILProcessor ilprocessor;
+			LinkerILProcessor ilprocessor;
 
 			public BodySweeper (MethodBody body, BitArray reachable, List<ExceptionHandler> unreachableEH, LinkContext context)
 			{
@@ -1155,7 +1155,7 @@ public bool Initialize ()
 					}
 				}
 
-				ilprocessor = body.GetILProcessor ();
+				ilprocessor = body.GetLinkerILProcessor ();
 				return true;
 			}
 

src/linker/Linker/LinkerILProcessor.cs

@@ -0,0 +1,127 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using Mono.Cecil;
+using Mono.Cecil.Cil;
+
+namespace Mono.Linker
+{
+	class LinkerILProcessor
+	{
+		readonly ILProcessor _ilProcessor;
+
+		Collections.Generic.Collection<Instruction> Instructions => _ilProcessor.Body.Instructions;
+
+		internal LinkerILProcessor (MethodBody body)
+		{
+			_ilProcessor = body.GetILProcessor ();
+		}
+
+		public void Emit (OpCode opcode) => Append (_ilProcessor.Create (opcode));
+		public void Emit (OpCode opcode, TypeReference type) => Append (_ilProcessor.Create (opcode, type));
+		public void Emit (OpCode opcode, MethodReference method) => Append (_ilProcessor.Create (opcode, method));
+		public void Emit (OpCode opcode, VariableDefinition variable) => Append (_ilProcessor.Create (opcode, variable));
+
+		public void Emit (OpCode opcode, string value) => Append (_ilProcessor.Create (opcode, value));
+		public void InsertBefore (Instruction target, Instruction instruction)
+		{
+			// When inserting before, all pointers have to be updated to the new "before" instruction.
+			RedirectScopeStart (target, instruction);
+			ReplaceInstructionReference (target, instruction);
+			_ilProcessor.InsertBefore (target, instruction);
+		}
+
+		// When inserting after, no redirection is necessary since it will naturally be "appended" to the same blocks
+		// as the target instruction.
+		public void InsertAfter (Instruction target, Instruction instruction)
+		{
+			RedirectScopeEnd (target, instruction);
+			_ilProcessor.InsertAfter (target, instruction);
+		}
+
+		public void Append (Instruction instruction)
+		{
+			Instruction lastInstruction = Instructions.Count == 0 ? null : Instructions[Instructions.Count - 1];
+			RedirectScopeEnd (lastInstruction, instruction);
+			_ilProcessor.Append (instruction);
+		}
+
+		public void Replace (Instruction target, Instruction instruction)
+		{
+			RedirectScopeStart (target, instruction);
+			RedirectScopeEnd (target, instruction);
+			ReplaceInstructionReference (target, instruction);
+			_ilProcessor.Replace (target, instruction);
+		}
+
+		public void Replace (int index, Instruction instruction) => Replace (_ilProcessor.Body.Instructions[index], instruction);
+
+		public void Remove (Instruction instruction)
+		{
+			int index = _ilProcessor.Body.Instructions.IndexOf (instruction);
+			if (index == -1)
+				throw new ArgumentOutOfRangeException (nameof (instruction));
+
+			Instruction nextInstruction = Instructions.Count == index + 1 ? null : Instructions[index + 1];
+			Instruction previousInstruction = index == 0 ? null : Instructions[index - 1];
+
+			RedirectScopeStart (instruction, nextInstruction);
+			RedirectScopeEnd (instruction, previousInstruction);
+			ReplaceInstructionReference (instruction, nextInstruction);
+			_ilProcessor.Remove (instruction);
+		}
+
+		public void RemoveAt (int index) => Remove (Instructions[index]);
+
+		void RedirectScopeStart (Instruction oldTarget, Instruction newTarget)
+		{
+			// In Cecil "start" pointers point to the first instruction in a given scope
+			// and the "end" pointers point to the first instruction after the given block
+			// so they need to effectively point to the start of the next scope.
+			// That's why both start and end are handled in the RedirectScopeStart.
+			foreach (var handler in _ilProcessor.Body.ExceptionHandlers) {
+				if (handler.TryStart == oldTarget)
+					handler.TryStart = newTarget;
+				if (handler.TryEnd == oldTarget)
+					handler.TryEnd = newTarget;
+				if (handler.HandlerStart == oldTarget)
+					handler.HandlerStart = newTarget;
+				if (handler.HandlerEnd == oldTarget)
+					handler.HandlerEnd = newTarget;
+				if (handler.FilterStart == oldTarget)
+					handler.FilterStart = newTarget;
+			}
+		}
+
+#pragma warning disable IDE0060 // Remove unused parameter
+		static void RedirectScopeEnd (Instruction oldTarget, Instruction newTarget)
+#pragma warning restore IDE0060 // Remove unused parameter
+		{
+			// Currently Cecil treats all block boundaries as "starts"
+			// so nothing to do here.
+		}
+
+		void ReplaceInstructionReference (Instruction oldTarget, Instruction newTarget)
+		{
+			foreach (var instr in Instructions) {
+				switch (instr.OpCode.FlowControl) {
+				case FlowControl.Branch:
+				case FlowControl.Cond_Branch:
+					if (instr.Operand == oldTarget)
+						instr.Operand = newTarget;
+					break;
+				}
+			}
+		}
+	}
+
+	static class ILProcessorExtensions
+	{
+		public static LinkerILProcessor GetLinkerILProcessor (this MethodBody body)
+		{
+			return new LinkerILProcessor (body);
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/BCLFeatures/ETW/StubbedMethodWithExceptionHandlers.cs

@@ -45,7 +45,6 @@ public class Keywords
 		{
 			"ret"
 		})]
-		[ExpectedExceptionHandlerSequence (new string[0])]
 		protected override void OnEventCommand (EventCommandEventArgs command)
 		{
 			try {

test/Mono.Linker.Tests.Cases/UnreachableBlock/Dependencies/EndScopeOnMethod.il

@@ -0,0 +1,40 @@
+.assembly extern mscorlib
+{
+}
+.assembly 'library'
+{
+  .hash algorithm 0x00008004
+  .ver  0:0:0:0
+}
+.module library.dll
+
+.namespace Mono.Linker.Tests.Cases.UnreachableBlock.Dependencies
+{
+
+  .class public auto ansi beforefieldinit EndScopeOnMethod
+    extends [mscorlib]System.Object
+  {
+
+    .method public hidebysig specialname rtspecialname
+           instance default void '.ctor' ()  cil managed
+    {
+      IL_0000:  ldarg.0
+      IL_0001:  call instance void class [mscorlib]System.Object::'.ctor'()
+      IL_0006:  ret
+    }
+
+    .method public static hidebysig object TryFinally() cil managed
+    {
+      .try
+      {
+        ldnull
+        ret
+      }
+      finally
+      {
+        ldnull
+        ret
+      }
+    }
+  }
+}

test/Mono.Linker.Tests.Cases/UnreachableBlock/EndScopeOnMethoEnd.cs

@@ -0,0 +1,28 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.UnreachableBlock
+{
+	[SetupLinkerArgument ("--skip-unresolved", "true")]
+	[Define ("IL_ASSEMBLY_AVAILABLE")]
+	[SetupCompileBefore ("library.dll", new[] { "Dependencies/EndScopeOnMethod.il" })]
+	public class EndScopeOnMethoEnd
+	{
+		public static void Main ()
+		{
+#if IL_ASSEMBLY_AVAILABLE
+            // For now just have a method where the try/finally is the last thing in the method (no instruction after the
+			// end of finally - Roslyn doesn't seem to produce such method body.
+			Mono.Linker.Tests.Cases.UnreachableBlock.Dependencies.EndScopeOnMethod.TryFinally ();
+#endif
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/UnreachableBlock/ReplacedReturns.cs

@@ -241,7 +241,6 @@ static byte Test7 ()
 		}
 
 		[Kept]
-		[ExpectedExceptionHandlerSequence (new string[0])]
 		[ExpectedLocalsSequence (new string[0])]
 		[ExpectedInstructionSequence (new[] {
 			"call",