Harden debug scope update logic (#824)

* Harden debug scope update logic

Based on bug reports like #816 it seems there are still cases where the IL and scope offsets are out of sync in weird ways. This change modifies the logic to have no potential to cause the `IndexOutOfRangeException`. While I was not able to determine what combination could cause this, it's better this way.

The corner case comes when there's potential problem with the first/second instruction in the method body. The change in this case will potentially make the debug scopes slightly wrong by not pointing to the previous instruction (as there's none). Without having a real repro it's hard to tell what would be a better solution, this way it won't crash and the scopes still make sense.

* Fix typo

1 files changed, 48 insertions, 37 deletions

diff --git a/Test/Mono.Cecil.Tests/ILProcessorTests.cs b/Test/Mono.Cecil.Tests/ILProcessorTests.cs
index c585403..c1dc13c 100644
--- a/Test/Mono.Cecil.Tests/ILProcessorTests.cs
+++ b/Test/Mono.Cecil.Tests/ILProcessorTests.cs
@@ -152,16 +152,18 @@ namespace Mono.Cecil.Tests {
 			AssertOpCodeSequence (new OpCode[] { }, method);
 		}
 
-		[TestCase (RoundtripType.None, false, false)]
-		[TestCase (RoundtripType.Pdb, false, false)]
-		[TestCase (RoundtripType.Pdb, true, false)]
-		[TestCase (RoundtripType.Pdb, true, true)]
-		[TestCase (RoundtripType.PortablePdb, false, false)]
-		[TestCase (RoundtripType.PortablePdb, true, false)]
-		[TestCase (RoundtripType.PortablePdb, true, true)]
-		public void InsertAfterWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
+		[TestCase (RoundtripType.None, false, false, false)]
+		[TestCase (RoundtripType.Pdb, false, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, true)]
+		[TestCase (RoundtripType.Pdb, true, true, false)]
+		[TestCase (RoundtripType.PortablePdb, false, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, true)]
+		[TestCase (RoundtripType.PortablePdb, true, true, false)]
+		public void InsertAfterWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
 		{
-			var methodBody = CreateTestMethodWithLocalScopes ();
+			var methodBody = CreateTestMethodWithLocalScopes (padIL);
 			methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
 
 			var il = methodBody.GetILProcessor ();
@@ -176,16 +178,18 @@ namespace Mono.Cecil.Tests {
 			methodBody.Method.Module.Dispose ();
 		}
 
-		[TestCase (RoundtripType.None, false, false)]
-		[TestCase (RoundtripType.Pdb, false, false)]
-		[TestCase (RoundtripType.Pdb, true, false)]
-		[TestCase (RoundtripType.Pdb, true, true)]
-		[TestCase (RoundtripType.PortablePdb, false, false)]
-		[TestCase (RoundtripType.PortablePdb, true, false)]
-		[TestCase (RoundtripType.PortablePdb, true, true)]
-		public void RemoveWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
+		[TestCase (RoundtripType.None, false, false, false)]
+		[TestCase (RoundtripType.Pdb, false, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, true)]
+		[TestCase (RoundtripType.Pdb, true, true, false)]
+		[TestCase (RoundtripType.PortablePdb, false, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, true)]
+		[TestCase (RoundtripType.PortablePdb, true, true, false)]
+		public void RemoveWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
 		{
-			var methodBody = CreateTestMethodWithLocalScopes ();
+			var methodBody = CreateTestMethodWithLocalScopes (padIL);
 			methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
 
 			var il = methodBody.GetILProcessor ();
@@ -200,16 +204,18 @@ namespace Mono.Cecil.Tests {
 			methodBody.Method.Module.Dispose ();
 		}
 
-		[TestCase (RoundtripType.None, false, false)]
-		[TestCase (RoundtripType.Pdb, false, false)]
-		[TestCase (RoundtripType.Pdb, true, false)]
-		[TestCase (RoundtripType.Pdb, true, true)]
-		[TestCase (RoundtripType.PortablePdb, false, false)]
-		[TestCase (RoundtripType.PortablePdb, true, false)]
-		[TestCase (RoundtripType.PortablePdb, true, true)]
-		public void ReplaceWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
+		[TestCase (RoundtripType.None, false, false, false)]
+		[TestCase (RoundtripType.Pdb, false, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, true)]
+		[TestCase (RoundtripType.Pdb, true, true, false)]
+		[TestCase (RoundtripType.PortablePdb, false, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, true)]
+		[TestCase (RoundtripType.PortablePdb, true, true, false)]
+		public void ReplaceWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
 		{
-			var methodBody = CreateTestMethodWithLocalScopes ();
+			var methodBody = CreateTestMethodWithLocalScopes (padIL);
 			methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
 
 			var il = methodBody.GetILProcessor ();
@@ -224,16 +230,18 @@ namespace Mono.Cecil.Tests {
 			methodBody.Method.Module.Dispose ();
 		}
 
-		[TestCase (RoundtripType.None, false, false)]
-		[TestCase (RoundtripType.Pdb, false, false)]
-		[TestCase (RoundtripType.Pdb, true, false)]
-		[TestCase (RoundtripType.Pdb, true, true)]
-		[TestCase (RoundtripType.PortablePdb, false, false)]
-		[TestCase (RoundtripType.PortablePdb, true, false)]
-		[TestCase (RoundtripType.PortablePdb, true, true)]
-		public void EditBodyWithScopesAndSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
+		[TestCase (RoundtripType.None, false, false, false)]
+		[TestCase (RoundtripType.Pdb, false, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, false)]
+		[TestCase (RoundtripType.Pdb, true, false, true)]
+		[TestCase (RoundtripType.Pdb, true, true, false)]
+		[TestCase (RoundtripType.PortablePdb, false, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, false)]
+		[TestCase (RoundtripType.PortablePdb, true, false, true)]
+		[TestCase (RoundtripType.PortablePdb, true, true, false)]
+		public void EditBodyWithScopesAndSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
 		{
-			var methodBody = CreateTestMethodWithLocalScopes ();
+			var methodBody = CreateTestMethodWithLocalScopes (padIL);
 			methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
 
 			var il = methodBody.GetILProcessor ();
@@ -320,13 +328,16 @@ namespace Mono.Cecil.Tests {
 				Assert.IsTrue (scope.End.IsEndOfMethod);
 		}
 
-		static MethodBody CreateTestMethodWithLocalScopes ()
+		static MethodBody CreateTestMethodWithLocalScopes (bool padILWithNulls)
 		{
 			var module = ModuleDefinition.CreateModule ("TestILProcessor", ModuleKind.Dll);
 			var type = new TypeDefinition ("NS", "TestType", TypeAttributes.Public | TypeAttributes.Abstract | TypeAttributes.Sealed, module.ImportReference (typeof (object)));
 			module.Types.Add (type);
 
 			var methodBody = CreateTestMethod (OpCodes.Nop, OpCodes.Ldloc_0, OpCodes.Nop, OpCodes.Ldloc_1, OpCodes.Nop, OpCodes.Ldloc_2, OpCodes.Nop);
+			if (padILWithNulls)
+				methodBody.Instructions.Capacity += 10;
+
 			var method = methodBody.Method;
 			method.ReturnType = module.ImportReference (typeof (void));
 			type.Methods.Add (method);