Hide compiler generated branches for try/catch blocks inside async state machine

src/coverlet.core/Instrumentation/Instrumenter.cs

@@ -447,6 +447,42 @@ private void InstrumentIL(MethodDefinition method)
                 var sequencePoint = method.DebugInformation.GetSequencePoint(instruction);
                 var targetedBranchPoints = branchPoints.Where(p => p.EndOffset == instruction.Offset);
 
+                /*
+                    Skip instrumentation after exception re-throw insiede catch block (only for async state machine MoveNext())
+                    es:
+                    try 
+                    {
+                        ...
+                    }
+                    catch
+                    {
+                        await ...
+                        throw; 
+                    } // need to skip instrumentation here
+
+                    We can detect this type of code block by searching for method ExceptionDispatchInfo.Throw() inside the compiled IL
+                    ...
+                    // ExceptionDispatchInfo.Capture(ex).Throw();
+		            IL_00c6: ldloc.s 6
+		            IL_00c8: call class [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Capture(class [System.Runtime]System.Exception)
+		            IL_00cd: callvirt instance void [System.Runtime]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw()
+		            // (no C# code)
+		            IL_00d2: nop
+		            IL_00d3: nop
+                    ...
+                */
+                Instruction previous;
+                if (
+                        CecilSymbolHelper.IsMoveNextInsideAsyncStateMachine(method) &&
+                        (previous = CecilSymbolHelper.GetPreviousNoNopInstruction(instruction)) != null &&
+                        previous != null &&
+                        previous.OpCode == OpCodes.Callvirt &&
+                        previous.Operand is MethodReference mr && mr.FullName == "System.Void System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw()"
+                    )
+                {
+                    continue;
+                }
+
                 if (sequencePoint != null && !sequencePoint.IsHidden)
                 {
                     var target = AddInstrumentationCode(method, processor, instruction, sequencePoint);

src/coverlet.core/Symbols/CecilSymbolHelper.cs

@@ -36,7 +36,7 @@ private static bool IsCompilerGenerated(MethodDefinition methodDefinition)
             return false;
         }
 
-        private static bool IsMoveNextInsideAsyncStateMachine(MethodDefinition methodDefinition)
+        internal static bool IsMoveNextInsideAsyncStateMachine(MethodDefinition methodDefinition)
         {
             if (methodDefinition.FullName.EndsWith("::MoveNext()") && IsCompilerGenerated(methodDefinition))
             {
@@ -202,6 +202,89 @@ so we know that current branch are checking that field and we're not interested
                         continue;
                     }
 
+                    /*
+                        Handle try/catch blocks inside async state machine
+                     */
+                    if (isAsyncStateMachineMoveNext)
+                    {
+                        /*
+                            Typical generated code for catch block (inside async state machine MoveNext() method) 
+                            is this:
+
+                            catch ...
+		                    {
+			                    // (no C# code)
+			                    IL_0028: stloc.2
+			                    // object obj2 = <>s__1 = obj;
+			                    IL_0029: ldarg.0
+			                    // (no C# code)
+			                    IL_002a: ldloc.2
+			                    IL_002b: stfld object ...::'<>s__1'
+			                    // <>s__2 = 1;
+			                    IL_0030: ldarg.0
+			                    IL_0031: ldc.i4.1
+			                    IL_0032: stfld int32 ...::'<>s__2'      <- store 1 into <>s__2
+			                    // (no C# code)
+			                    IL_0037: leave.s IL_0039
+		                    } // end handle
+
+                            // int num2 = <>s__2;
+                            IL_0039: ldarg.0
+		                    IL_003a: ldfld int32 ...::'<>s__2'          <- load <>s__2 value and check if 1
+		                    IL_003f: stloc.3
+                            // if (num2 == 1)
+		                    IL_0040: ldloc.3
+		                    IL_0041: ldc.i4.1
+		                    IL_0042: beq.s IL_0049                      <- if <>s__2 value is 1 go to exception handler code
+
+                            IL_0044: br IL_00d6
+
+                            IL_0049: nop                                <- start exception handler code
+                            ...
+
+
+                            So starting from branch instruction 'beq.s', we can go back to starting block instruction 
+                            which is always 5 step before and then check if this istruction is the end of an exception handler block
+                        */
+
+                        int branchIndex = instructions.IndexOf(instruction);
+                        if (
+                                branchIndex >= 5 &&
+                                instructions[branchIndex - 5].OpCode == OpCodes.Ldarg &&
+                                methodDefinition.Body.ExceptionHandlers.Any(h => h.HandlerEnd == instructions[branchIndex - 5])
+                            )
+                        {
+                            continue;
+                        }
+
+                        /* 
+                            In case of exception re-thrown inside the catch block, 
+                            the compiler generates a branch to check if the exception reference is null.
+
+                            A sample of generated code:
+
+                            IL_00b4: isinst [System.Runtime]System.Exception
+                            IL_00b9: stloc.s 6
+                            // if (ex == null)
+                            IL_00bb: ldloc.s 6
+                            // (no C# code)
+                            IL_00bd: brtrue.s IL_00c6
+
+                            So we can go back to previous instructions and skip this branch if recognize that type of code block
+                        */
+
+                        if (
+                                branchIndex >= 3 && // avoid out of range exception (need almost 3 instruction before the branch)
+                                instructions[branchIndex - 3].OpCode == OpCodes.Isinst &&
+                                instructions[branchIndex - 3].Operand is TypeReference tr && tr.FullName == "System.Exception" &&
+                                instructions[branchIndex - 2].OpCode == OpCodes.Stloc &&
+                                instructions[branchIndex - 1].OpCode == OpCodes.Ldloc
+                           )
+                        {
+                            continue;
+                        }
+                    }
+
                     if (BranchIsInGeneratedExceptionFilter(instruction, methodDefinition))
                         continue;
 
@@ -279,6 +362,13 @@ private static bool BuildPointsForConditionalBranch(List<BranchPoint> list, Inst
             return true;
         }
 
+        internal static Instruction GetPreviousNoNopInstruction(Instruction current)
+        {
+            Instruction instruction = current.Previous;
+            for (; instruction != null && instruction.OpCode == OpCodes.Nop; instruction = instruction.Previous) { }
+            return instruction;
+        }
+
         private static uint BuildPointsForBranch(List<BranchPoint> list, Instruction then, int branchingInstructionLine, string document,
             int branchOffset, uint ordinal, int pathCounter, BranchPoint path0, Collection<Instruction> instructions, MethodDefinition methodDefinition)
         {

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

@@ -0,0 +1,45 @@
+using System.IO;
+using System.Threading.Tasks;
+
+using Coverlet.Core.Samples.Tests;
+using Coverlet.Tests.RemoteExecutor;
+using Xunit;
+
+namespace Coverlet.Core.Tests
+{
+    public partial class CoverageTests
+    {
+        [Fact]
+        public void CatchBlock_Issue465()
+        {
+            string path = Path.GetTempFileName();
+            try
+            {
+                RemoteExecutor.Invoke(async pathSerialize =>
+                {
+                    CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<CatchBlock>(instance =>
+                    {
+                        instance.Test();
+                        ((Task)instance.TestAsync()).ConfigureAwait(false).GetAwaiter().GetResult();
+
+                        return Task.CompletedTask;
+                    }, persistPrepareResultToFile: pathSerialize);
+                    return 0;
+                }, path).Dispose();
+
+                CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);
+
+                result.Document("Instrumentation.CatchBlock.cs")
+                    .AssertLinesCoveredAllBut(BuildConfiguration.Debug,
+                        41, 
+                        51)
+                    .ExpectedTotalNumberOfBranches(BuildConfiguration.Debug, 0) 
+                    ;
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
+    }
+}

test/coverlet.core.tests/Samples/Instrumentation.CatchBlock.cs

@@ -0,0 +1,55 @@
+// Remember to use full name because adding new using directives change line numbers
+
+using System.Threading.Tasks;
+
+namespace Coverlet.Core.Samples.Tests
+{
+    public class CatchBlock
+    {
+        public int Parse(string str)
+        {
+            try
+            {
+                return int.Parse(str);
+            }
+            catch
+            {
+                throw;
+            }
+        }
+
+        public async Task<int> ParseAsync(string str)
+        {
+            try
+            {
+                return int.Parse(str);
+            }
+            catch
+            {
+                await Task.Delay(0);
+
+                throw;
+            }
+        }
+
+        public void Test()
+        {
+            Parse(nameof(Test).Length.ToString());
+            try
+            {
+                Parse(nameof(Test));
+            }
+            catch { }
+        }
+
+        public async Task TestAsync()
+        {
+            await ParseAsync(nameof(Test).Length.ToString());
+            try
+            {
+                await ParseAsync(nameof(Test));
+            }
+            catch { }
+        }
+    }
+}