Skip to content

Commit 8b593d5

Browse files
vitek-karasvitek-karas
authored
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 parent a56b5bd commit 8b593d5

File tree

2 files changed

+57
-41
lines changed

2 files changed

+57
-41
lines changed

‎Mono.Cecil.Cil/MethodBody.cs‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,11 +384,16 @@ InstructionOffset ResolveInstructionOffset(InstructionOffset inputOffset, ref In
384384
// resolve by walking the instructions from start and don't cache the result.
385385
int size = 0;
386386
for (int i = 0; i < items.Length; i++) {
387+
// The array can be larger than the actual size, in which case its padded with nulls at the end
388+
// so when we reach null, treat it as an end of the IL.
389+
if (items [i] == null)
390+
return new InstructionOffset (i == 0 ? items [0] : items [i - 1]);
391+
387392
if (size == offset)
388393
return new InstructionOffset (items [i]);
389394

390395
if (size > offset)
391-
return new InstructionOffset (items [i - 1]);
396+
return new InstructionOffset (i == 0 ? items [0] : items [i - 1]);
392397

393398
size += items [i].GetSize ();
394399
}
@@ -407,15 +412,15 @@ InstructionOffset ResolveInstructionOffset(InstructionOffset inputOffset, ref In
407412
// Allow for trailing null values in the case of
408413
// instructions.Size < instructions.Capacity
409414
if (item == null)
410-
return new InstructionOffset (items [i - 1]);
415+
return new InstructionOffset (i == 0 ? items [0] : items [i - 1]);
411416

412417
cache.Instruction = item;
413418

414419
if (cache.Offset == offset)
415420
return new InstructionOffset (cache.Instruction);
416421

417-
if (cache.Offset > offset)
418-
return new InstructionOffset (items [i - 1]);
422+
if (cache.Offset > offset)
423+
return new InstructionOffset (i == 0 ? items [0] : items [i - 1]);
419424

420425
size += item.GetSize ();
421426
}

‎Test/Mono.Cecil.Tests/ILProcessorTests.cs‎

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -152,16 +152,18 @@ public void Clear ()
152152
AssertOpCodeSequence (new OpCode[] { }, method);
153153
}
154154

155-
[TestCase (RoundtripType.None, false, false)]
156-
[TestCase (RoundtripType.Pdb, false, false)]
157-
[TestCase (RoundtripType.Pdb, true, false)]
158-
[TestCase (RoundtripType.Pdb, true, true)]
159-
[TestCase (RoundtripType.PortablePdb, false, false)]
160-
[TestCase (RoundtripType.PortablePdb, true, false)]
161-
[TestCase (RoundtripType.PortablePdb, true, true)]
162-
public void InsertAfterWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
155+
[TestCase (RoundtripType.None, false, false, false)]
156+
[TestCase (RoundtripType.Pdb, false, false, false)]
157+
[TestCase (RoundtripType.Pdb, true, false, false)]
158+
[TestCase (RoundtripType.Pdb, true, false, true)]
159+
[TestCase (RoundtripType.Pdb, true, true, false)]
160+
[TestCase (RoundtripType.PortablePdb, false, false, false)]
161+
[TestCase (RoundtripType.PortablePdb, true, false, false)]
162+
[TestCase (RoundtripType.PortablePdb, true, false, true)]
163+
[TestCase (RoundtripType.PortablePdb, true, true, false)]
164+
public void InsertAfterWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
163165
{
164-
var methodBody = CreateTestMethodWithLocalScopes ();
166+
var methodBody = CreateTestMethodWithLocalScopes (padIL);
165167
methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
166168

167169
var il = methodBody.GetILProcessor ();
@@ -176,16 +178,18 @@ public void InsertAfterWithSymbolRoundtrip (RoundtripType roundtripType, bool fo
176178
methodBody.Method.Module.Dispose ();
177179
}
178180

179-
[TestCase (RoundtripType.None, false, false)]
180-
[TestCase (RoundtripType.Pdb, false, false)]
181-
[TestCase (RoundtripType.Pdb, true, false)]
182-
[TestCase (RoundtripType.Pdb, true, true)]
183-
[TestCase (RoundtripType.PortablePdb, false, false)]
184-
[TestCase (RoundtripType.PortablePdb, true, false)]
185-
[TestCase (RoundtripType.PortablePdb, true, true)]
186-
public void RemoveWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
181+
[TestCase (RoundtripType.None, false, false, false)]
182+
[TestCase (RoundtripType.Pdb, false, false, false)]
183+
[TestCase (RoundtripType.Pdb, true, false, false)]
184+
[TestCase (RoundtripType.Pdb, true, false, true)]
185+
[TestCase (RoundtripType.Pdb, true, true, false)]
186+
[TestCase (RoundtripType.PortablePdb, false, false, false)]
187+
[TestCase (RoundtripType.PortablePdb, true, false, false)]
188+
[TestCase (RoundtripType.PortablePdb, true, false, true)]
189+
[TestCase (RoundtripType.PortablePdb, true, true, false)]
190+
public void RemoveWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
187191
{
188-
var methodBody = CreateTestMethodWithLocalScopes ();
192+
var methodBody = CreateTestMethodWithLocalScopes (padIL);
189193
methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
190194

191195
var il = methodBody.GetILProcessor ();
@@ -200,16 +204,18 @@ public void RemoveWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUn
200204
methodBody.Method.Module.Dispose ();
201205
}
202206

203-
[TestCase (RoundtripType.None, false, false)]
204-
[TestCase (RoundtripType.Pdb, false, false)]
205-
[TestCase (RoundtripType.Pdb, true, false)]
206-
[TestCase (RoundtripType.Pdb, true, true)]
207-
[TestCase (RoundtripType.PortablePdb, false, false)]
208-
[TestCase (RoundtripType.PortablePdb, true, false)]
209-
[TestCase (RoundtripType.PortablePdb, true, true)]
210-
public void ReplaceWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
207+
[TestCase (RoundtripType.None, false, false, false)]
208+
[TestCase (RoundtripType.Pdb, false, false, false)]
209+
[TestCase (RoundtripType.Pdb, true, false, false)]
210+
[TestCase (RoundtripType.Pdb, true, false, true)]
211+
[TestCase (RoundtripType.Pdb, true, true, false)]
212+
[TestCase (RoundtripType.PortablePdb, false, false, false)]
213+
[TestCase (RoundtripType.PortablePdb, true, false, false)]
214+
[TestCase (RoundtripType.PortablePdb, true, false, true)]
215+
[TestCase (RoundtripType.PortablePdb, true, true, false)]
216+
public void ReplaceWithSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
211217
{
212-
var methodBody = CreateTestMethodWithLocalScopes ();
218+
var methodBody = CreateTestMethodWithLocalScopes (padIL);
213219
methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
214220

215221
var il = methodBody.GetILProcessor ();
@@ -224,16 +230,18 @@ public void ReplaceWithSymbolRoundtrip (RoundtripType roundtripType, bool forceU
224230
methodBody.Method.Module.Dispose ();
225231
}
226232

227-
[TestCase (RoundtripType.None, false, false)]
228-
[TestCase (RoundtripType.Pdb, false, false)]
229-
[TestCase (RoundtripType.Pdb, true, false)]
230-
[TestCase (RoundtripType.Pdb, true, true)]
231-
[TestCase (RoundtripType.PortablePdb, false, false)]
232-
[TestCase (RoundtripType.PortablePdb, true, false)]
233-
[TestCase (RoundtripType.PortablePdb, true, true)]
234-
public void EditBodyWithScopesAndSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes)
233+
[TestCase (RoundtripType.None, false, false, false)]
234+
[TestCase (RoundtripType.Pdb, false, false, false)]
235+
[TestCase (RoundtripType.Pdb, true, false, false)]
236+
[TestCase (RoundtripType.Pdb, true, false, true)]
237+
[TestCase (RoundtripType.Pdb, true, true, false)]
238+
[TestCase (RoundtripType.PortablePdb, false, false, false)]
239+
[TestCase (RoundtripType.PortablePdb, true, false, false)]
240+
[TestCase (RoundtripType.PortablePdb, true, false, true)]
241+
[TestCase (RoundtripType.PortablePdb, true, true, false)]
242+
public void EditBodyWithScopesAndSymbolRoundtrip (RoundtripType roundtripType, bool forceUnresolved, bool reverseScopes, bool padIL)
235243
{
236-
var methodBody = CreateTestMethodWithLocalScopes ();
244+
var methodBody = CreateTestMethodWithLocalScopes (padIL);
237245
methodBody = RoundtripMethodBody (methodBody, roundtripType, forceUnresolved, reverseScopes);
238246

239247
var il = methodBody.GetILProcessor ();
@@ -320,13 +328,16 @@ static void AssertLocalScope (MethodBody methodBody, ScopeDebugInformation scope
320328
Assert.IsTrue (scope.End.IsEndOfMethod);
321329
}
322330

323-
static MethodBody CreateTestMethodWithLocalScopes ()
331+
static MethodBody CreateTestMethodWithLocalScopes (bool padILWithNulls)
324332
{
325333
var module = ModuleDefinition.CreateModule ("TestILProcessor", ModuleKind.Dll);
326334
var type = new TypeDefinition ("NS", "TestType", TypeAttributes.Public | TypeAttributes.Abstract | TypeAttributes.Sealed, module.ImportReference (typeof (object)));
327335
module.Types.Add (type);
328336

329337
var methodBody = CreateTestMethod (OpCodes.Nop, OpCodes.Ldloc_0, OpCodes.Nop, OpCodes.Ldloc_1, OpCodes.Nop, OpCodes.Ldloc_2, OpCodes.Nop);
338+
if (padILWithNulls)
339+
methodBody.Instructions.Capacity += 10;
340+
330341
var method = methodBody.Method;
331342
method.ReturnType = module.ImportReference (typeof (void));
332343
type.Methods.Add (method);

0 commit comments

Comments
 (0)