Auto update local scopes in debug info when manipulating instructions (#677)

* Auto update local scopes in debug info when manipulating instructions

Before this change Cecil would not update local scopes when manipulating mehod bodies. This can lead to corrupted debug info - which when written to a PDB results in corrupted PDB.

Cecil can store local scopes in two ways:
* Using IL offset numbers - this is how pretty much all of the symbol readers populate the OMs.
* Using references to instructions.

If the local scopes use offset values this change will update the local scopes for all insert and remove operations on the method body. The behaviors for insert is basically "insert after" in that the new instruction is added to the scopes of the previous instruction.

If the local scopes are using instructions directly the change only removes any references to instructions which are being removed from the method body (and replaces them with the instruction offset value).

To be able to tell the difference between these cases the instruction field has been made internal in the InstructionOffset structure.

* Add `IsResolved` to the `InstructionOffset` to make the code more readable.

Note: Still have to keep the `instruction` field visible to handle the case of removing a resolved instruction offset reliably (since most resolved instructions will have the numerical offset 0).

* Add internal property on `InstructionOffset` to be able to keep all its fields private.

* Improve comments in the code.

1 files changed, 98 insertions, 0 deletions

diff --git a/Test/Mono.Cecil.Tests/ILProcessorTests.cs b/Test/Mono.Cecil.Tests/ILProcessorTests.cs
index 1f22905..17b4fb0 100644
--- a/Test/Mono.Cecil.Tests/ILProcessorTests.cs
+++ b/Test/Mono.Cecil.Tests/ILProcessorTests.cs
@@ -67,6 +67,23 @@ namespace Mono.Cecil.Tests {
 		}
 
 		[Test]
+		public void InsertAfterWithLocalScopes ()
+		{
+			var method = CreateTestMethodWithLocalScopes ();
+			var il = method.GetILProcessor ();
+
+			il.InsertAfter (
+				0,
+				il.Create (OpCodes.Nop));
+
+			AssertOpCodeSequence (new [] { OpCodes.Ldloc_0, OpCodes.Nop, OpCodes.Ldloc_1, OpCodes.Ldloc_2 }, method);
+			var wholeBodyScope = VerifyWholeBodyScope (method);
+			AssertLocalScope (wholeBodyScope.Scopes [0], 0, 2);
+			AssertLocalScope (wholeBodyScope.Scopes [1], 2, 3);
+			AssertLocalScope (wholeBodyScope.Scopes [2], 3, null);
+		}
+
+		[Test]
 		public void ReplaceUsingIndex ()
 		{
 			var method = CreateTestMethod (OpCodes.Ldloc_0, OpCodes.Ldloc_2, OpCodes.Ldloc_3);
@@ -78,6 +95,24 @@ namespace Mono.Cecil.Tests {
 		}
 
 		[Test]
+		public void ReplaceWithLocalScopes ()
+		{
+			var method = CreateTestMethodWithLocalScopes ();
+			var il = method.GetILProcessor ();
+
+			// Replace with larger instruction
+			var instruction = il.Create (OpCodes.Ldstr, "test");
+			instruction.Offset = method.Instructions [1].Offset;
+			il.Replace (1, instruction);
+
+			AssertOpCodeSequence (new [] { OpCodes.Ldloc_0, OpCodes.Ldstr, OpCodes.Ldloc_2 }, method);
+			var wholeBodyScope = VerifyWholeBodyScope (method);
+			AssertLocalScope (wholeBodyScope.Scopes [0], 0, 1);
+			AssertLocalScope (wholeBodyScope.Scopes [1], 1, 6); // size of the new instruction is 5 bytes
+			AssertLocalScope (wholeBodyScope.Scopes [2], 6, null);
+		}
+
+		[Test]
 		public void Clear ()
 		{
 			var method = CreateTestMethod (OpCodes.Ldloc_0, OpCodes.Ldloc_2, OpCodes.Ldloc_3);
@@ -108,7 +143,70 @@ namespace Mono.Cecil.Tests {
 			foreach (var opcode in opcodes)
 				il.Emit (opcode);
 
+			var instructions = method.Body.Instructions;
+			int size = 0;
+			for (int i = 0; i < instructions.Count; i++) {
+				var instruction = instructions [i];
+				instruction.Offset = size;
+				size += instruction.GetSize ();
+			}
+
 			return method.Body;
 		}
+
+		static ScopeDebugInformation VerifyWholeBodyScope (MethodBody body)
+		{
+			var debug_info = body.Method.DebugInformation;
+			Assert.IsNotNull (debug_info);
+			AssertLocalScope (debug_info.Scope, 0, null);
+			return debug_info.Scope;
+		}
+
+		static void AssertLocalScope (ScopeDebugInformation scope, int startOffset, int? endOffset)
+		{
+			Assert.IsNotNull (scope);
+			Assert.AreEqual (startOffset, scope.Start.Offset);
+			if (endOffset.HasValue)
+				Assert.AreEqual (endOffset.Value, scope.End.Offset);
+			else
+				Assert.IsTrue (scope.End.IsEndOfMethod);
+		}
+
+		static MethodBody CreateTestMethodWithLocalScopes ()
+		{
+			var methodBody = CreateTestMethod (OpCodes.Ldloc_0, OpCodes.Ldloc_1, OpCodes.Ldloc_2);
+			var method = methodBody.Method;
+			var debug_info = method.DebugInformation;
+
+			var wholeBodyScope = new ScopeDebugInformation () {
+				Start = new InstructionOffset (0),
+				End = new InstructionOffset ()
+			};
+			int size = 0;
+			var instruction = methodBody.Instructions [0];
+			var innerScopeBegining = new ScopeDebugInformation () { 
+				Start = new InstructionOffset (size), 
+				End = new InstructionOffset (size + instruction.GetSize ())
+			};
+			size += instruction.GetSize ();
+			wholeBodyScope.Scopes.Add (innerScopeBegining);
+
+			instruction = methodBody.Instructions [1];
+			var innerScopeMiddle = new ScopeDebugInformation () {
+				Start = new InstructionOffset (size),
+				End = new InstructionOffset (size + instruction.GetSize ())
+			};
+			size += instruction.GetSize ();
+			wholeBodyScope.Scopes.Add (innerScopeMiddle);
+
+			var innerScopeEnd = new ScopeDebugInformation () {
+				Start = new InstructionOffset (size),
+				End = new InstructionOffset ()
+			};
+			wholeBodyScope.Scopes.Add (innerScopeEnd);
+
+			debug_info.Scope = wholeBodyScope;
+			return methodBody;
+		}
 	}
 }