diff --git a/Mono.Cecil.Cil/MethodBody.cs b/Mono.Cecil.Cil/MethodBody.cs index 0b5692d8d..d970a0195 100644 --- a/Mono.Cecil.Cil/MethodBody.cs +++ b/Mono.Cecil.Cil/MethodBody.cs @@ -258,9 +258,7 @@ protected override void OnInsert (Instruction item, int index) item.next = current; } - var scope = GetLocalScope (); - if (scope != null) - UpdateLocalScope (scope, startOffset, item.GetSize (), instructionRemoved: null); + UpdateLocalScopes (null, null); } protected override void OnSet (Instruction item, int index) @@ -273,11 +271,7 @@ protected override void OnSet (Instruction item, int index) current.previous = null; current.next = null; - var scope = GetLocalScope (); - if (scope != null) { - var sizeOfCurrent = current.GetSize (); - UpdateLocalScope (scope, current.Offset + sizeOfCurrent, item.GetSize () - sizeOfCurrent, current); - } + UpdateLocalScopes (item, current); } protected override void OnRemove (Instruction item, int index) @@ -291,12 +285,7 @@ protected override void OnRemove (Instruction item, int index) next.previous = item.previous; RemoveSequencePoint (item); - - var scope = GetLocalScope (); - if (scope != null) { - var size = item.GetSize (); - UpdateLocalScope (scope, item.Offset + size, -size, item); - } + UpdateLocalScopes (item, next ?? previous); item.previous = null; item.next = null; @@ -317,36 +306,113 @@ void RemoveSequencePoint (Instruction instruction) } } - ScopeDebugInformation GetLocalScope () + void UpdateLocalScopes (Instruction removedInstruction, Instruction existingInstruction) { var debug_info = method.debug_info; if (debug_info == null) - return null; + return; - return debug_info.Scope; + // Local scopes store start/end pair of "instruction offsets". Instruction offset can be either resolved, in which case it + // has a reference to Instruction, or unresolved in which case it stores numerical offset (instruction offset in the body). + // Typically local scopes loaded from PE/PDB files will be resolved, but it's not a requirement. + // Each instruction has its own offset, which is populated on load, but never updated (this would be pretty expensive to do). + // Instructions created during the editting will typically have offset 0 (so incorrect). + // Local scopes created during editing will also likely be resolved (so no numerical offsets). + // So while local scopes which are unresolved are relatively rare if they appear, manipulating them based + // on the offsets allone is pretty hard (since we can't rely on correct offsets of instructions). + // On the other hand resolved local scopes are easy to maintain, since they point to instructions and thus inserting + // instructions is basically a no-op and removing instructions is as easy as changing the pointer. + // For this reason the algorithm here is: + // - First make sure that all instruction offsets are resolved - if not - resolve them + // - First time this will be relatively expensinve as it will walk the entire method body to convert offsets to instruction pointers + // Almost all local scopes are stored in the "right" order (sequentially per start offsets), so the code uses a simple one-item + // cache instruction<->offset to avoid walking instructions multiple times (that would only happen for scopes which are out of order). + // - Subsequent calls should be cheap as it will only walk all local scopes without doing anything + // - If there was an edit on local scope which makes some of them unresolved, the cost is proportional + // - Then update as necessary by manipulaitng instruction references alone + + InstructionOffsetCache cache = new InstructionOffsetCache () { + Offset = 0, + Index = 0, + Instruction = items [0] + }; + + UpdateLocalScope (debug_info.Scope, removedInstruction, existingInstruction, ref cache); } - static void UpdateLocalScope (ScopeDebugInformation scope, int startFromOffset, int offset, Instruction instructionRemoved) + void UpdateLocalScope (ScopeDebugInformation scope, Instruction removedInstruction, Instruction existingInstruction, ref InstructionOffsetCache cache) { - // For both start and enf offsets on the scope: - // * If the offset is resolved (points to instruction by reference) only update it if the instruction it points to is being removed. - // For non-removed instructions it remains correct regardless of any updates to the instructions. - // * If the offset is not resolved (stores the instruction offset number itself) - // update the number accordingly to keep it pointing to the correct instruction (by offset). - - if ((!scope.Start.IsResolved && scope.Start.Offset >= startFromOffset) || - (instructionRemoved != null && scope.Start.ResolvedInstruction == instructionRemoved)) - scope.Start = new InstructionOffset (scope.Start.Offset + offset); - - // For end offset only update it if it's not the special sentinel value "EndOfMethod"; that should remain as-is. - if (!scope.End.IsEndOfMethod && - ((!scope.End.IsResolved && scope.End.Offset >= startFromOffset) || - (instructionRemoved != null && scope.End.ResolvedInstruction == instructionRemoved))) - scope.End = new InstructionOffset (scope.End.Offset + offset); + if (scope == null) + return; + + if (!scope.Start.IsResolved) + scope.Start = ResolveInstructionOffset (scope.Start, ref cache); + + if (!scope.Start.IsEndOfMethod && scope.Start.ResolvedInstruction == removedInstruction) + scope.Start = new InstructionOffset (existingInstruction); if (scope.HasScopes) { foreach (var subScope in scope.Scopes) - UpdateLocalScope (subScope, startFromOffset, offset, instructionRemoved); + UpdateLocalScope (subScope, removedInstruction, existingInstruction, ref cache); + } + + if (!scope.End.IsResolved) + scope.End = ResolveInstructionOffset (scope.End, ref cache); + + if (!scope.End.IsEndOfMethod && scope.End.ResolvedInstruction == removedInstruction) + scope.End = new InstructionOffset (existingInstruction); + } + + struct InstructionOffsetCache { + public int Offset; + public int Index; + public Instruction Instruction; + } + + InstructionOffset ResolveInstructionOffset(InstructionOffset inputOffset, ref InstructionOffsetCache cache) + { + if (inputOffset.IsResolved) + return inputOffset; + + int offset = inputOffset.Offset; + + if (cache.Offset == offset) + return new InstructionOffset (cache.Instruction); + + if (cache.Offset > offset) { + // This should be rare - we're resolving offset pointing to a place before the current cache position + // resolve by walking the instructions from start and don't cache the result. + int size = 0; + for (int i = 0; i < items.Length; i++) { + if (size == offset) + return new InstructionOffset (items [i]); + + if (size > offset) + return new InstructionOffset (items [i - 1]); + + size += items [i].GetSize (); + } + + // Offset is larger than the size of the body - so it points after the end + return new InstructionOffset (); + } else { + // The offset points after the current cache position - so continue counting and update the cache + int size = cache.Offset; + for (int i = cache.Index; i < items.Length; i++) { + cache.Index = i; + cache.Offset = size; + cache.Instruction = items [i]; + + if (cache.Offset == offset) + return new InstructionOffset (cache.Instruction); + + if (cache.Offset > offset) + return new InstructionOffset (items [i - 1]); + + size += items [i].GetSize (); + } + + return new InstructionOffset (); } } } diff --git a/Mono.Cecil.Cil/Symbols.cs b/Mono.Cecil.Cil/Symbols.cs index 38b47a42e..4cf435dc2 100644 --- a/Mono.Cecil.Cil/Symbols.cs +++ b/Mono.Cecil.Cil/Symbols.cs @@ -207,7 +207,7 @@ public bool IsEndOfMethod { get { return instruction == null && !offset.HasValue; } } - internal bool IsResolved => instruction != null; + internal bool IsResolved => instruction != null || !offset.HasValue; internal Instruction ResolvedInstruction => instruction; diff --git a/Test/Mono.Cecil.Tests/ILProcessorTests.cs b/Test/Mono.Cecil.Tests/ILProcessorTests.cs index 17b4fb05f..88366152d 100644 --- a/Test/Mono.Cecil.Tests/ILProcessorTests.cs +++ b/Test/Mono.Cecil.Tests/ILProcessorTests.cs @@ -1,9 +1,11 @@ using System; +using System.Collections.Generic; +using System.IO; using System.Linq; using Mono.Cecil; using Mono.Cecil.Cil; - +using Mono.Cecil.Pdb; using NUnit.Framework; namespace Mono.Cecil.Tests { @@ -67,60 +69,143 @@ public void InsertAfterUsingIndex () } [Test] - public void InsertAfterWithLocalScopes () + public void ReplaceUsingIndex () { - var method = CreateTestMethodWithLocalScopes (); + var method = CreateTestMethod (OpCodes.Ldloc_0, OpCodes.Ldloc_2, OpCodes.Ldloc_3); var il = method.GetILProcessor (); - il.InsertAfter ( - 0, - il.Create (OpCodes.Nop)); + il.Replace (1, 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); + AssertOpCodeSequence (new [] { OpCodes.Ldloc_0, OpCodes.Nop, OpCodes.Ldloc_3 }, method); } [Test] - public void ReplaceUsingIndex () + public void Clear () { var method = CreateTestMethod (OpCodes.Ldloc_0, OpCodes.Ldloc_2, OpCodes.Ldloc_3); var il = method.GetILProcessor (); - il.Replace (1, il.Create (OpCodes.Nop)); + il.Clear (); - AssertOpCodeSequence (new [] { OpCodes.Ldloc_0, OpCodes.Nop, OpCodes.Ldloc_3 }, method); + AssertOpCodeSequence (new OpCode[] { }, method); } - [Test] - public void ReplaceWithLocalScopes () + [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) { - var method = CreateTestMethodWithLocalScopes (); - var il = method.GetILProcessor (); + var methodBody = CreateTestMethodWithLocalScopes (); + methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes); + + var il = methodBody.GetILProcessor (); + il.InsertAfter (1, il.Create (OpCodes.Ldstr, "Test")); + + methodBody = RoundtripMethodBody (methodBody, roundtripType, false, reverseScopes); + var wholeBodyScope = VerifyWholeBodyScope (methodBody); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [0], 1, 3); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1], 4, null); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1].Scopes [0], 5, 6); - // Replace with larger instruction - var instruction = il.Create (OpCodes.Ldstr, "test"); - instruction.Offset = method.Instructions [1].Offset; - il.Replace (1, instruction); + 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) + { + var methodBody = CreateTestMethodWithLocalScopes (); + methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes); + + var il = methodBody.GetILProcessor (); + il.RemoveAt (1); + + methodBody = RoundtripMethodBody (methodBody, roundtripType, false, reverseScopes); + var wholeBodyScope = VerifyWholeBodyScope (methodBody); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [0], 1, 1); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1], 2, null); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1].Scopes [0], 3, 4); - 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); + 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) + { + var methodBody = CreateTestMethodWithLocalScopes (); + methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes); + + var il = methodBody.GetILProcessor (); + il.Replace (1, il.Create (OpCodes.Ldstr, "Test")); + + methodBody = RoundtripMethodBody (methodBody, roundtripType, false, reverseScopes); + var wholeBodyScope = VerifyWholeBodyScope (methodBody); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [0], 1, 2); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1], 3, null); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1].Scopes [0], 4, 5); + + 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) + { + var methodBody = CreateTestMethodWithLocalScopes (); + methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes); + + var il = methodBody.GetILProcessor (); + il.Replace (4, il.Create (OpCodes.Ldstr, "Test")); + il.InsertAfter (5, il.Create (OpCodes.Ldloc_3)); + var tempVar3 = new VariableDefinition (methodBody.Method.Module.ImportReference (typeof (string))); + methodBody.Variables.Add (tempVar3); + methodBody.Method.DebugInformation.Scope.Scopes [reverseScopes ? 0 : 1].Scopes.Insert (reverseScopes ? 0 : 1, + new ScopeDebugInformation (methodBody.Instructions [5], methodBody.Instructions [6]) { + Variables = { new VariableDebugInformation (tempVar3, "tempVar3") } + }); + + methodBody = RoundtripMethodBody (methodBody, roundtripType, false, reverseScopes); + var wholeBodyScope = VerifyWholeBodyScope (methodBody); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [0], 1, 2); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1], 3, null); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1].Scopes [0], 4, 5); + AssertLocalScope (methodBody, wholeBodyScope.Scopes [1].Scopes [1], 5, 6); + + methodBody.Method.Module.Dispose (); } [Test] - public void Clear () + public void EditWithDebugInfoButNoLocalScopes() { - var method = CreateTestMethod (OpCodes.Ldloc_0, OpCodes.Ldloc_2, OpCodes.Ldloc_3); - var il = method.GetILProcessor (); + var methodBody = CreateTestMethod (OpCodes.Ret); + methodBody.Method.DebugInformation = new MethodDebugInformation (methodBody.Method); - il.Clear (); + var il = methodBody.GetILProcessor (); + il.Replace (methodBody.Instructions [0], il.Create (OpCodes.Nop)); - AssertOpCodeSequence (new OpCode[] { }, method); + Assert.AreEqual (1, methodBody.Instructions.Count); + Assert.AreEqual (Code.Nop, methodBody.Instructions [0].OpCode.Code); + Assert.Null (methodBody.Method.DebugInformation.Scope); } static void AssertOpCodeSequence (OpCode [] expected, MethodBody body) @@ -136,6 +221,7 @@ static MethodBody CreateTestMethod (params OpCode [] opcodes) { var method = new MethodDefinition { Name = "function", + Attributes = MethodAttributes.Public | MethodAttributes.Static }; var il = method.Body.GetILProcessor (); @@ -158,55 +244,163 @@ static ScopeDebugInformation VerifyWholeBodyScope (MethodBody body) { var debug_info = body.Method.DebugInformation; Assert.IsNotNull (debug_info); - AssertLocalScope (debug_info.Scope, 0, null); + AssertLocalScope (body, debug_info.Scope, 0, null); return debug_info.Scope; } - static void AssertLocalScope (ScopeDebugInformation scope, int startOffset, int? endOffset) + static void AssertLocalScope (MethodBody methodBody, ScopeDebugInformation scope, int startIndex, int? endIndex) { Assert.IsNotNull (scope); - Assert.AreEqual (startOffset, scope.Start.Offset); - if (endOffset.HasValue) - Assert.AreEqual (endOffset.Value, scope.End.Offset); + Assert.AreEqual (methodBody.Instructions [startIndex], scope.Start.ResolvedInstruction); + if (endIndex.HasValue) + Assert.AreEqual (methodBody.Instructions [endIndex.Value], scope.End.ResolvedInstruction); else Assert.IsTrue (scope.End.IsEndOfMethod); } static MethodBody CreateTestMethodWithLocalScopes () { - var methodBody = CreateTestMethod (OpCodes.Ldloc_0, OpCodes.Ldloc_1, OpCodes.Ldloc_2); + 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); var method = methodBody.Method; - var debug_info = method.DebugInformation; + method.ReturnType = module.ImportReference (typeof (void)); + type.Methods.Add (method); - 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); + methodBody.InitLocals = true; + var tempVar1 = new VariableDefinition (module.ImportReference (typeof (string))); + methodBody.Variables.Add (tempVar1); + var tempVar2 = new VariableDefinition (module.ImportReference (typeof (string))); + methodBody.Variables.Add (tempVar2); - 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 debug_info = method.DebugInformation; - var innerScopeEnd = new ScopeDebugInformation () { - Start = new InstructionOffset (size), - End = new InstructionOffset () + // Must add a sequence point, otherwise the native PDB writer will not actually write the method's debug info + // into the PDB. + foreach (var instr in methodBody.Instructions) { + var sequence_point = new SequencePoint (instr, new Document (@"C:\test.cs")) { + StartLine = 0, + StartColumn = 0, + EndLine = 0, + EndColumn = 4, + }; + debug_info.SequencePoints.Add (sequence_point); + } + + // The method looks like this: + // | Scope | Scope.Scopes[0] | Scope.Scopes[1] | Scope.Scopes[1].Scopes[0] + // #0 Nop | > | | | + // #1 Ldloc_0 | . | > | | + // #2 Nop | . | < | | + // #3 Ldloc_1 | . | | > | + // #4 Nop | . | | . | > + // #5 Ldloc_2 | . | | . | < + // #6 Nop | . | | . | + // | < | | < | + + var instructions = methodBody.Instructions; + debug_info.Scope = new ScopeDebugInformation (instructions[0], null) { + Scopes = { + new ScopeDebugInformation (instructions[1], instructions[2]) { + Variables = { new VariableDebugInformation(tempVar1, "tempVar1") } + }, + new ScopeDebugInformation (instructions[3], null) { + Scopes = { + new ScopeDebugInformation (instructions[4], instructions[5]) { + Variables = { new VariableDebugInformation(tempVar2, "tempVar2") } + } + } + } + } }; - wholeBodyScope.Scopes.Add (innerScopeEnd); - debug_info.Scope = wholeBodyScope; return methodBody; } + + public enum RoundtripType { + None, + Pdb, + PortablePdb + } + + static MethodBody RoundtripMethodBody(MethodBody methodBody, RoundtripType roundtripType, bool forceUnresolvedScopes = false, bool reverseScopeOrder = false) + { + var newModule = RoundtripModule (methodBody.Method.Module, roundtripType); + var newMethodBody = newModule.GetType ("NS.TestType").GetMethod ("function").Body; + + if (forceUnresolvedScopes) + UnresolveScopes (newMethodBody.Method.DebugInformation.Scope); + + if (reverseScopeOrder) + ReverseScopeOrder (newMethodBody.Method.DebugInformation.Scope); + + return newMethodBody; + } + + static void UnresolveScopes(ScopeDebugInformation scope) + { + scope.Start = new InstructionOffset (scope.Start.Offset); + if (!scope.End.IsEndOfMethod) + scope.End = new InstructionOffset (scope.End.Offset); + + foreach (var subScope in scope.Scopes) + UnresolveScopes (subScope); + } + + static void ReverseScopeOrder(ScopeDebugInformation scope) + { + List subScopes = scope.Scopes.ToList (); + subScopes.Reverse (); + scope.Scopes.Clear (); + foreach (var subScope in subScopes) + scope.Scopes.Add (subScope); + + foreach (var subScope in scope.Scopes) + ReverseScopeOrder (subScope); + } + + static ModuleDefinition RoundtripModule(ModuleDefinition module, RoundtripType roundtripType) + { + if (roundtripType == RoundtripType.None) + return module; + + var file = Path.Combine (Path.GetTempPath (), "TestILProcessor.dll"); + if (File.Exists (file)) + File.Delete (file); + + ISymbolWriterProvider symbolWriterProvider; + switch (roundtripType) { + case RoundtripType.Pdb when Platform.HasNativePdbSupport: + symbolWriterProvider = new PdbWriterProvider (); + break; + case RoundtripType.PortablePdb: + default: + symbolWriterProvider = new PortablePdbWriterProvider (); + break; + } + + module.Write (file, new WriterParameters { + SymbolWriterProvider = symbolWriterProvider, + }); + module.Dispose (); + + ISymbolReaderProvider symbolReaderProvider; + switch (roundtripType) { + case RoundtripType.Pdb when Platform.HasNativePdbSupport: + symbolReaderProvider = new PdbReaderProvider (); + break; + case RoundtripType.PortablePdb: + default: + symbolReaderProvider = new PortablePdbReaderProvider (); + break; + } + + return ModuleDefinition.ReadModule (file, new ReaderParameters { + SymbolReaderProvider = symbolReaderProvider, + InMemory = true + }); + } } }