Skip to content

Commit 528956b

Browse files
matteoerigozzimatteoerigozzi
authored
Hide compiler generated branches for try/catch blocks inside async state machine (#716)
Hide compiler generated branches for try/catch blocks inside async state machine
1 parent ec180b2 commit 528956b

File tree

4 files changed

+715
-3
lines changed

4 files changed

+715
-3
lines changed

‎src/coverlet.core/Instrumentation/Instrumenter.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,13 @@ private void InstrumentIL(MethodDefinition method)
447447
var sequencePoint = method.DebugInformation.GetSequencePoint(instruction);
448448
var targetedBranchPoints = branchPoints.Where(p => p.EndOffset == instruction.Offset);
449449

450+
// Check if the instruction is coverable
451+
if (CecilSymbolHelper.SkipNotCoverableInstruction(method, instruction))
452+
{
453+
index++;
454+
continue;
455+
}
456+
450457
if (sequencePoint != null && !sequencePoint.IsHidden)
451458
{
452459
var target = AddInstrumentationCode(method, processor, instruction, sequencePoint);

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

Lines changed: 277 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// team in OpenCover.Framework.Symbols.CecilSymbolManager
44
//
55
using System;
6+
using System.Collections.Concurrent;
67
using System.Collections.Generic;
78
using System.Linq;
89
using System.Runtime.CompilerServices;
@@ -18,6 +19,13 @@ namespace Coverlet.Core.Symbols
1819
internal static class CecilSymbolHelper
1920
{
2021
private const int StepOverLineCode = 0xFEEFEE;
22+
private static ConcurrentDictionary<string, int[]> CompilerGeneratedBranchesToExclude = null;
23+
24+
static CecilSymbolHelper()
25+
{
26+
// Create single instance, we cannot collide because we use full method name as key
27+
CompilerGeneratedBranchesToExclude = new ConcurrentDictionary<string, int[]>();
28+
}
2129

2230
// In case of nested compiler generated classes, only the root one presents the CompilerGenerated attribute.
2331
// So let's search up to the outermost declaring type to find the attribute
@@ -227,6 +235,170 @@ Lambda cached field pattern
227235
return false;
228236
}
229237

238+
private static bool SkipGeneratedBranchForExceptionRethrown(List<Instruction> instructions, Instruction instruction)
239+
{
240+
/*
241+
In case of exception re-thrown inside the catch block,
242+
the compiler generates a branch to check if the exception reference is null.
243+
244+
A sample of generated code:
245+
246+
IL_00b4: isinst [System.Runtime]System.Exception
247+
IL_00b9: stloc.s 6
248+
// if (ex == null)
249+
IL_00bb: ldloc.s 6
250+
// (no C# code)
251+
IL_00bd: brtrue.s IL_00c6
252+
253+
So we can go back to previous instructions and skip this branch if recognize that type of code block
254+
*/
255+
int branchIndex = instructions.BinarySearch(instruction, new InstructionByOffsetComparer());
256+
return branchIndex >= 3 && // avoid out of range exception (need almost 3 instruction before the branch)
257+
instructions[branchIndex - 3].OpCode == OpCodes.Isinst &&
258+
instructions[branchIndex - 3].Operand is TypeReference tr && tr.FullName == "System.Exception" &&
259+
instructions[branchIndex - 2].OpCode == OpCodes.Stloc &&
260+
instructions[branchIndex - 1].OpCode == OpCodes.Ldloc &&
261+
// check for throw opcode after branch
262+
instructions.Count - branchIndex >= 3 &&
263+
instructions[branchIndex + 1].OpCode == OpCodes.Ldarg &&
264+
instructions[branchIndex + 2].OpCode == OpCodes.Ldfld &&
265+
instructions[branchIndex + 3].OpCode == OpCodes.Throw;
266+
}
267+
268+
private static bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDefinition, Instruction instruction, List<Instruction> bodyInstructions)
269+
{
270+
if (!CompilerGeneratedBranchesToExclude.ContainsKey(methodDefinition.FullName))
271+
{
272+
/*
273+
This method is used to parse compiler generated code inside async state machine and find branches generated for exception catch blocks
274+
Typical generated code for catch block is:
275+
276+
catch ...
277+
{
278+
// (no C# code)
279+
IL_0028: stloc.2
280+
// object obj2 = <>s__1 = obj;
281+
IL_0029: ldarg.0
282+
// (no C# code)
283+
IL_002a: ldloc.2
284+
IL_002b: stfld object ...::'<>s__1'
285+
// <>s__2 = 1;
286+
IL_0030: ldarg.0
287+
IL_0031: ldc.i4.1
288+
IL_0032: stfld int32 ...::'<>s__2' <- store 1 into <>s__2
289+
// (no C# code)
290+
IL_0037: leave.s IL_0039
291+
} // end handle
292+
293+
// int num2 = <>s__2;
294+
IL_0039: ldarg.0
295+
IL_003a: ldfld int32 ...::'<>s__2' <- load <>s__2 value and check if 1
296+
IL_003f: stloc.3
297+
// if (num2 == 1)
298+
IL_0040: ldloc.3
299+
IL_0041: ldc.i4.1
300+
IL_0042: beq.s IL_0049 <- BRANCH : if <>s__2 value is 1 go to exception handler code
301+
302+
IL_0044: br IL_00d6
303+
304+
IL_0049: nop <- start exception handler code
305+
306+
In case of multiple catch blocks as
307+
try
308+
{
309+
}
310+
catch (ExceptionType1)
311+
{
312+
}
313+
catch (ExceptionType2)
314+
{
315+
}
316+
317+
generated IL contains multiple branches:
318+
catch ...(type1)
319+
{
320+
...
321+
}
322+
catch ...(type2)
323+
{
324+
...
325+
}
326+
// int num2 = <>s__2;
327+
IL_0039: ldarg.0
328+
IL_003a: ldfld int32 ...::'<>s__2' <- load <>s__2 value and check if 1
329+
IL_003f: stloc.3
330+
// if (num2 == 1)
331+
IL_0040: ldloc.3
332+
IL_0041: ldc.i4.1
333+
IL_0042: beq.s IL_0049 <- BRANCH 1 (type 1)
334+
335+
IL_0044: br IL_00d6
336+
337+
// if (num2 == 2)
338+
IL_0067: ldloc.s 4
339+
IL_0069: ldc.i4.2
340+
IL_006a: beq IL_0104 <- BRANCH 2 (type 2)
341+
342+
// (no C# code)
343+
IL_006f: br IL_0191
344+
*/
345+
List<int> detectedBranches = new List<int>();
346+
Collection<ExceptionHandler> handlers = methodDefinition.Body.ExceptionHandlers;
347+
348+
int numberOfCatchBlocks = 1;
349+
foreach (var handler in handlers)
350+
{
351+
if (handlers.Any(h => h.HandlerStart == handler.HandlerEnd))
352+
{
353+
// In case of multiple consecutive catch block
354+
numberOfCatchBlocks++;
355+
continue;
356+
}
357+
358+
int currentIndex = bodyInstructions.BinarySearch(handler.HandlerEnd, new InstructionByOffsetComparer());
359+
360+
/* Detect flag load
361+
// int num2 = <>s__2;
362+
IL_0058: ldarg.0
363+
IL_0059: ldfld int32 ...::'<>s__2'
364+
IL_005e: stloc.s 4
365+
*/
366+
if (bodyInstructions.Count - currentIndex > 3 && // check boundary
367+
bodyInstructions[currentIndex].OpCode == OpCodes.Ldarg &&
368+
bodyInstructions[currentIndex + 1].OpCode == OpCodes.Ldfld && bodyInstructions[currentIndex + 1].Operand is FieldReference fr && fr.Name.StartsWith("<>s__") &&
369+
bodyInstructions[currentIndex + 2].OpCode == OpCodes.Stloc)
370+
{
371+
currentIndex += 3;
372+
for (int i = 0; i < numberOfCatchBlocks; i++)
373+
{
374+
/*
375+
// if (num2 == 1)
376+
IL_0060: ldloc.s 4
377+
IL_0062: ldc.i4.1
378+
IL_0063: beq.s IL_0074
379+
380+
// (no C# code)
381+
IL_0065: br.s IL_0067
382+
*/
383+
if (bodyInstructions.Count - currentIndex > 4 && // check boundary
384+
bodyInstructions[currentIndex].OpCode == OpCodes.Ldloc &&
385+
bodyInstructions[currentIndex + 1].OpCode == OpCodes.Ldc_I4 &&
386+
bodyInstructions[currentIndex + 2].OpCode == OpCodes.Beq &&
387+
bodyInstructions[currentIndex + 3].OpCode == OpCodes.Br)
388+
{
389+
detectedBranches.Add(bodyInstructions[currentIndex + 2].Offset);
390+
}
391+
currentIndex += 4;
392+
}
393+
}
394+
}
395+
396+
CompilerGeneratedBranchesToExclude.TryAdd(methodDefinition.FullName, detectedBranches.ToArray());
397+
}
398+
399+
return CompilerGeneratedBranchesToExclude[methodDefinition.FullName].Contains(instruction.Offset);
400+
}
401+
230402
public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
231403
{
232404
var list = new List<BranchPoint>();
@@ -236,7 +408,7 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
236408
}
237409

238410
uint ordinal = 0;
239-
var instructions = methodDefinition.Body.Instructions;
411+
var instructions = methodDefinition.Body.Instructions.ToList();
240412

241413
bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition);
242414
bool isMoveNextInsideAsyncStateMachineProlog = isAsyncStateMachineMoveNext && IsMoveNextInsideAsyncStateMachineProlog(methodDefinition);
@@ -265,6 +437,14 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
265437
continue;
266438
}
267439

440+
if (isAsyncStateMachineMoveNext)
441+
{
442+
if (SkipGeneratedBranchesForExceptionHandlers(methodDefinition, instruction, instructions) ||
443+
SkipGeneratedBranchForExceptionRethrown(instructions, instruction))
444+
{
445+
continue;
446+
}
447+
}
268448
if (SkipBranchGeneratedExceptionFilter(instruction, methodDefinition))
269449
{
270450
continue;
@@ -303,7 +483,7 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
303483

304484
private static bool BuildPointsForConditionalBranch(List<BranchPoint> list, Instruction instruction,
305485
int branchingInstructionLine, string document, int branchOffset, int pathCounter,
306-
Collection<Instruction> instructions, ref uint ordinal, MethodDefinition methodDefinition)
486+
List<Instruction> instructions, ref uint ordinal, MethodDefinition methodDefinition)
307487
{
308488
// Add Default branch (Path=0)
309489

@@ -351,7 +531,7 @@ private static bool BuildPointsForConditionalBranch(List<BranchPoint> list, Inst
351531
}
352532

353533
private static uint BuildPointsForBranch(List<BranchPoint> list, Instruction then, int branchingInstructionLine, string document,
354-
int branchOffset, uint ordinal, int pathCounter, BranchPoint path0, Collection<Instruction> instructions, MethodDefinition methodDefinition)
534+
int branchOffset, uint ordinal, int pathCounter, BranchPoint path0, List<Instruction> instructions, MethodDefinition methodDefinition)
355535
{
356536
var pathOffsetList1 = GetBranchPath(@then);
357537

@@ -431,6 +611,100 @@ private static uint BuildPointsForSwitchCases(List<BranchPoint> list, BranchPoin
431611
return ordinal;
432612
}
433613

614+
/*
615+
Need to skip instrumentation after exception re-throw inside catch block (only for async state machine MoveNext())
616+
es:
617+
try
618+
{
619+
...
620+
}
621+
catch
622+
{
623+
await ...
624+
throw;
625+
} // need to skip instrumentation here
626+
627+
We can detect this type of code block by searching for method ExceptionDispatchInfo.Throw() inside the compiled IL
628+
...
629+
// ExceptionDispatchInfo.Capture(ex).Throw();
630+
IL_00c6: ldloc.s 6
631+
IL_00c8: call class [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Capture(class [System.Runtime]System.Exception)
632+
IL_00cd: callvirt instance void [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw()
633+
// NOT COVERABLE
634+
IL_00d2: nop
635+
IL_00d3: nop
636+
...
637+
638+
In case of nested code blocks inside catch we need to detect also goto calls
639+
...
640+
// ExceptionDispatchInfo.Capture(ex).Throw();
641+
IL_00d3: ldloc.s 7
642+
IL_00d5: call class [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Capture(class [System.Runtime]System.Exception)
643+
IL_00da: callvirt instance void [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw()
644+
// NOT COVERABLE
645+
IL_00df: nop
646+
IL_00e0: nop
647+
IL_00e1: br.s IL_00ea
648+
...
649+
// NOT COVERABLE
650+
IL_00ea: nop
651+
IL_00eb: br.s IL_00ed
652+
...
653+
*/
654+
internal static bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction)
655+
{
656+
if (!IsMoveNextInsideAsyncStateMachine(methodDefinition))
657+
{
658+
return false;
659+
}
660+
661+
if (instruction.OpCode != OpCodes.Nop)
662+
{
663+
return false;
664+
}
665+
666+
// detect if current instruction is not coverable
667+
Instruction prev = GetPreviousNoNopInstruction(instruction);
668+
if (prev != null &&
669+
prev.OpCode == OpCodes.Callvirt &&
670+
prev.Operand is MethodReference mr && mr.FullName == "System.Void System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw()")
671+
{
672+
return true;
673+
}
674+
675+
// find the caller of current instruction and detect if not coverable
676+
prev = instruction.Previous;
677+
while (prev != null)
678+
{
679+
if (prev.Operand is Instruction i && (i.Offset == instruction.Offset || i.Offset == prev.Next.Offset)) // caller
680+
{
681+
prev = GetPreviousNoNopInstruction(prev);
682+
break;
683+
}
684+
prev = prev.Previous;
685+
}
686+
687+
return prev != null &&
688+
prev.OpCode == OpCodes.Callvirt &&
689+
prev.Operand is MethodReference mr1 && mr1.FullName == "System.Void System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw()";
690+
691+
// local helper
692+
static Instruction GetPreviousNoNopInstruction(Instruction i)
693+
{
694+
Instruction instruction = i.Previous;
695+
while (instruction != null)
696+
{
697+
if (instruction.OpCode != OpCodes.Nop)
698+
{
699+
return instruction;
700+
}
701+
instruction = instruction.Previous;
702+
}
703+
704+
return null;
705+
}
706+
}
707+
434708
private static bool SkipBranchGeneratedExceptionFilter(Instruction branchInstruction, MethodDefinition methodDefinition)
435709
{
436710
if (!methodDefinition.Body.HasExceptionHandlers)

0 commit comments

Comments
 (0)