Skip to content

Commit 4e1a055

Browse files
daveMuellerdaveMueller
authored
Bugfix partially covered throw statement (#1144)
Bugfix partially covered throw statement
1 parent 1f9e04b commit 4e1a055

File tree

2 files changed

+146
-119
lines changed

2 files changed

+146
-119
lines changed

‎src/coverlet.core/Symbols/CecilSymbolHelper.cs

+144-118
Original file line numberDiff line numberDiff line change
@@ -254,107 +254,150 @@ private static bool SkipGeneratedBranchForExceptionRethrown(List<Instruction> in
254254
In case of exception re-thrown inside the catch block,
255255
the compiler generates a branch to check if the exception reference is null.
256256
257-
A sample of generated code:
258-
257+
// Debug version:
259258
IL_00b4: isinst [System.Runtime]System.Exception
260259
IL_00b9: stloc.s 6
261-
// if (ex == null)
262260
IL_00bb: ldloc.s 6
263-
// (no C# code)
264261
IL_00bd: brtrue.s IL_00c6
265262
266-
So we can go back to previous instructions and skip this branch if recognize that type of code block
263+
// Release version:
264+
IL_008A: isinst[System.Runtime]System.Exception
265+
IL_008F: dup
266+
IL_0090: brtrue.s IL_0099
267+
268+
So we can go back to previous instructions and skip this branch if recognize that type of code block.
269+
There are differences between debug and release configuration so we just look for the highlights.
267270
*/
271+
268272
int branchIndex = instructions.BinarySearch(instruction, new InstructionByOffsetComparer());
269-
return branchIndex >= 3 && // avoid out of range exception (need almost 3 instruction before the branch)
270-
instructions[branchIndex - 3].OpCode == OpCodes.Isinst &&
271-
instructions[branchIndex - 3].Operand is TypeReference tr && tr.FullName == "System.Exception" &&
272-
instructions[branchIndex - 2].OpCode == OpCodes.Stloc &&
273-
instructions[branchIndex - 1].OpCode == OpCodes.Ldloc &&
274-
// check for throw opcode after branch
275-
instructions.Count - branchIndex >= 3 &&
276-
instructions[branchIndex + 1].OpCode == OpCodes.Ldarg &&
277-
instructions[branchIndex + 2].OpCode == OpCodes.Ldfld &&
278-
instructions[branchIndex + 3].OpCode == OpCodes.Throw;
273+
274+
return new[] {2,3}.Any(x => branchIndex >= x &&
275+
instructions[branchIndex - x].OpCode == OpCodes.Isinst &&
276+
instructions[branchIndex - x].Operand is TypeReference tr &&
277+
tr.FullName == "System.Exception")
278+
&&
279+
// check for throw opcode after branch
280+
instructions.Count - branchIndex >= 3 &&
281+
instructions[branchIndex + 1].OpCode == OpCodes.Ldarg &&
282+
instructions[branchIndex + 2].OpCode == OpCodes.Ldfld &&
283+
instructions[branchIndex + 3].OpCode == OpCodes.Throw;
279284
}
280285

281286
private bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDefinition, Instruction instruction, List<Instruction> bodyInstructions)
282287
{
288+
/*
289+
This method is used to parse compiler generated code inside async state machine and find branches generated for exception catch blocks.
290+
There are differences between debug and release configuration. Also variations of the shown examples exist e.g. multiple catch blocks,
291+
nested catch blocks... Therefore we just look for the highlights.
292+
Typical generated code for catch block is:
293+
294+
// Debug version:
295+
catch ...
296+
{
297+
// (no C# code)
298+
IL_0028: stloc.2
299+
// object obj2 = <>s__1 = obj;
300+
IL_0029: ldarg.0
301+
// (no C# code)
302+
IL_002a: ldloc.2
303+
IL_002b: stfld object ...::'<>s__1'
304+
// <>s__2 = 1;
305+
IL_0030: ldarg.0
306+
IL_0031: ldc.i4.1
307+
IL_0032: stfld int32 ...::'<>s__2' <- store 1 into <>s__2
308+
// (no C# code)
309+
IL_0037: leave.s IL_0039
310+
} // end handle
311+
312+
// int num2 = <>s__2;
313+
IL_0039: ldarg.0
314+
IL_003a: ldfld int32 ...::'<>s__2' <- load <>s__2 value and check if 1
315+
IL_003f: stloc.3
316+
// if (num2 == 1)
317+
IL_0040: ldloc.3
318+
IL_0041: ldc.i4.1
319+
IL_0042: beq.s IL_0049 <- BRANCH : if <>s__2 value is 1 go to exception handler code
320+
IL_0044: br IL_00d6
321+
IL_0049: nop <- start exception handler code
322+
323+
// Release version:
324+
catch ...
325+
{
326+
IL_001D: stloc.3
327+
IL_001E: ldarg.0
328+
IL_001F: ldloc.3
329+
IL_0020: stfld object Coverlet.Core.Samples.Tests.CatchBlock / '<ParseAsync>d__0'::'<>7__wrap1'
330+
IL_0025: ldc.i4.1
331+
IL_0026: stloc.2
332+
IL_0027: leave.s IL_0029
333+
} // end handler
334+
335+
IL_0029: ldloc.2
336+
IL_002A: ldc.i4.1
337+
IL_002B: bne.un.s IL_00AC
338+
339+
340+
In case of multiple catch blocks as
341+
try
342+
{
343+
}
344+
catch (ExceptionType1)
345+
{
346+
}
347+
catch (ExceptionType2)
348+
{
349+
}
350+
351+
generated IL contains multiple branches:
352+
353+
// Debug version:
354+
catch ...(type1)
355+
{
356+
...
357+
}
358+
catch ...(type2)
359+
{
360+
...
361+
}
362+
// int num2 = <>s__2;
363+
IL_0039: ldarg.0
364+
IL_003a: ldfld int32 ...::'<>s__2' <- load <>s__2 value and check if 1
365+
IL_003f: stloc.3
366+
// if (num2 == 1)
367+
IL_0040: ldloc.3
368+
IL_0041: ldc.i4.1
369+
IL_0042: beq.s IL_0049 <- BRANCH 1 (type 1)
370+
IL_0044: br IL_00d6
371+
372+
// if (num2 == 2)
373+
IL_0067: ldloc.s 4
374+
IL_0069: ldc.i4.2
375+
IL_006a: beq IL_0104 <- BRANCH 2 (type 2)
376+
377+
// (no C# code)
378+
IL_006f: br IL_0191
379+
380+
// Release version:
381+
catch ...(type1)
382+
{
383+
...
384+
}
385+
catch ...(type2)
386+
{
387+
...
388+
}
389+
IL_003E: ldloc.2
390+
IL_003F: ldc.i4.1
391+
IL_0040: beq.s IL_004E
392+
393+
IL_0042: ldloc.2/
394+
IL_0043: ldc.i4.2
395+
IL_0044: beq IL_00CD
396+
397+
IL_0049: br IL_0147
398+
*/
283399
if (!_compilerGeneratedBranchesToExclude.ContainsKey(methodDefinition.FullName))
284400
{
285-
/*
286-
This method is used to parse compiler generated code inside async state machine and find branches generated for exception catch blocks
287-
Typical generated code for catch block is:
288-
289-
catch ...
290-
{
291-
// (no C# code)
292-
IL_0028: stloc.2
293-
// object obj2 = <>s__1 = obj;
294-
IL_0029: ldarg.0
295-
// (no C# code)
296-
IL_002a: ldloc.2
297-
IL_002b: stfld object ...::'<>s__1'
298-
// <>s__2 = 1;
299-
IL_0030: ldarg.0
300-
IL_0031: ldc.i4.1
301-
IL_0032: stfld int32 ...::'<>s__2' <- store 1 into <>s__2
302-
// (no C# code)
303-
IL_0037: leave.s IL_0039
304-
} // end handle
305-
306-
// int num2 = <>s__2;
307-
IL_0039: ldarg.0
308-
IL_003a: ldfld int32 ...::'<>s__2' <- load <>s__2 value and check if 1
309-
IL_003f: stloc.3
310-
// if (num2 == 1)
311-
IL_0040: ldloc.3
312-
IL_0041: ldc.i4.1
313-
IL_0042: beq.s IL_0049 <- BRANCH : if <>s__2 value is 1 go to exception handler code
314-
315-
IL_0044: br IL_00d6
316-
317-
IL_0049: nop <- start exception handler code
318-
319-
In case of multiple catch blocks as
320-
try
321-
{
322-
}
323-
catch (ExceptionType1)
324-
{
325-
}
326-
catch (ExceptionType2)
327-
{
328-
}
329-
330-
generated IL contains multiple branches:
331-
catch ...(type1)
332-
{
333-
...
334-
}
335-
catch ...(type2)
336-
{
337-
...
338-
}
339-
// int num2 = <>s__2;
340-
IL_0039: ldarg.0
341-
IL_003a: ldfld int32 ...::'<>s__2' <- load <>s__2 value and check if 1
342-
IL_003f: stloc.3
343-
// if (num2 == 1)
344-
IL_0040: ldloc.3
345-
IL_0041: ldc.i4.1
346-
IL_0042: beq.s IL_0049 <- BRANCH 1 (type 1)
347-
348-
IL_0044: br IL_00d6
349-
350-
// if (num2 == 2)
351-
IL_0067: ldloc.s 4
352-
IL_0069: ldc.i4.2
353-
IL_006a: beq IL_0104 <- BRANCH 2 (type 2)
354-
355-
// (no C# code)
356-
IL_006f: br IL_0191
357-
*/
358401
List<int> detectedBranches = new List<int>();
359402
Collection<ExceptionHandler> handlers = methodDefinition.Body.ExceptionHandlers;
360403

@@ -370,39 +413,22 @@ private bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDe
370413

371414
int currentIndex = bodyInstructions.BinarySearch(handler.HandlerEnd, new InstructionByOffsetComparer());
372415

373-
/* Detect flag load
374-
// int num2 = <>s__2;
375-
IL_0058: ldarg.0
376-
IL_0059: ldfld int32 ...::'<>s__2'
377-
IL_005e: stloc.s 4
378-
*/
379-
if (bodyInstructions.Count - currentIndex > 3 && // check boundary
380-
bodyInstructions[currentIndex].OpCode == OpCodes.Ldarg &&
381-
bodyInstructions[currentIndex + 1].OpCode == OpCodes.Ldfld && bodyInstructions[currentIndex + 1].Operand is FieldReference fr && fr.Name.StartsWith("<>s__") &&
382-
bodyInstructions[currentIndex + 2].OpCode == OpCodes.Stloc)
416+
for (int i = 0; i < numberOfCatchBlocks; i++)
383417
{
384-
currentIndex += 3;
385-
for (int i = 0; i < numberOfCatchBlocks; i++)
418+
// We expect the first branch after the end of the handler to be no more than five instructions away.
419+
int firstBranchIndex = currentIndex + Enumerable.Range(1, 5).FirstOrDefault(x =>
420+
currentIndex + x < bodyInstructions.Count &&
421+
bodyInstructions[currentIndex + x].OpCode == OpCodes.Beq ||
422+
bodyInstructions[currentIndex + x].OpCode == OpCodes.Bne_Un);
423+
424+
if (bodyInstructions.Count - firstBranchIndex > 2 &&
425+
bodyInstructions[firstBranchIndex - 1].OpCode == OpCodes.Ldc_I4 &&
426+
bodyInstructions[firstBranchIndex - 2].OpCode == OpCodes.Ldloc)
386427
{
387-
/*
388-
// if (num2 == 1)
389-
IL_0060: ldloc.s 4
390-
IL_0062: ldc.i4.1
391-
IL_0063: beq.s IL_0074
392-
393-
// (no C# code)
394-
IL_0065: br.s IL_0067
395-
*/
396-
if (bodyInstructions.Count - currentIndex > 4 && // check boundary
397-
bodyInstructions[currentIndex].OpCode == OpCodes.Ldloc &&
398-
bodyInstructions[currentIndex + 1].OpCode == OpCodes.Ldc_I4 &&
399-
bodyInstructions[currentIndex + 2].OpCode == OpCodes.Beq &&
400-
bodyInstructions[currentIndex + 3].OpCode == OpCodes.Br)
401-
{
402-
detectedBranches.Add(bodyInstructions[currentIndex + 2].Offset);
403-
}
404-
currentIndex += 4;
428+
detectedBranches.Add(bodyInstructions[firstBranchIndex].Offset);
405429
}
430+
431+
currentIndex = firstBranchIndex;
406432
}
407433
}
408434

‎test/coverlet.core.tests/Coverage/CoverageTests.CatchBlock.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ public void CatchBlock_Issue465()
6767
var res = TestInstrumentationHelper.GetCoverageResult(path);
6868
res.Document("Instrumentation.CatchBlock.cs")
6969
.AssertLinesCoveredAllBut(BuildConfiguration.Debug, 45, 59, 113, 127, 137, 138, 139, 153, 154, 155, 156, 175, 189, 199, 200, 201, 222, 223, 224, 225, 252, 266, 335, 349)
70-
.ExpectedTotalNumberOfBranches(BuildConfiguration.Debug, 6);
70+
.ExpectedTotalNumberOfBranches(BuildConfiguration.Debug, 6)
71+
.ExpectedTotalNumberOfBranches(BuildConfiguration.Release, 6);
7172
}
7273
finally
7374
{

0 commit comments

Comments
 (0)