From 37a302b328368c81f35da3d8beb41d2e9cccdeb8 Mon Sep 17 00:00:00 2001 From: Alex Thornton Date: Sat, 6 Mar 2021 17:39:57 -0800 Subject: [PATCH 1/4] Correct coverage for `await using` (issue #914) This commit corrects coverage for the `await using` statement added in C# 8, including the variant without an explicit scope (i.e., without curly braces and a body), by eliminating two kinds of patterns that arise in the async state machine that weren't already being detected and eliminated. --- .../Extensions/InstructionExtensions.cs | 247 +++++++++ .../Symbols/CecilSymbolHelper.cs | 491 ++++++++++++------ .../Coverage/CoverageTests.AwaitUsing.cs | 52 ++ .../Samples/Instrumentation.AwaitUsing.cs | 48 ++ test/coverlet.core.tests/Samples/Samples.cs | 18 + .../Symbols/CecilSymbolHelperTests.cs | 34 +- 6 files changed, 722 insertions(+), 168 deletions(-) create mode 100644 src/coverlet.core/Extensions/InstructionExtensions.cs create mode 100644 test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs create mode 100644 test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs diff --git a/src/coverlet.core/Extensions/InstructionExtensions.cs b/src/coverlet.core/Extensions/InstructionExtensions.cs new file mode 100644 index 000000000..7c804dad5 --- /dev/null +++ b/src/coverlet.core/Extensions/InstructionExtensions.cs @@ -0,0 +1,247 @@ +using Mono.Cecil; +using Mono.Cecil.Cil; + + +namespace Coverlet.Core.Extensions +{ + // Extension methods that can smooth out small differences between similar CIL + // instructions, such as "ldarg.0" as opposed to "ldarg 0". + public static class InstructionExtensions + { + #region ldarg + + public static bool IsLdarg(this Instruction instruction, int argumentIndex) + { + return IsLdarg(instruction, out int actualArgumentIndex) && + argumentIndex == actualArgumentIndex; + } + + + public static bool IsLdarg(this Instruction instruction) + { + return IsLdarg(instruction, out int _); + } + + + public static bool IsLdarg(this Instruction instruction, out int argumentIndex) + { + if (instruction.OpCode == OpCodes.Ldarg || + instruction.OpCode == OpCodes.Ldarg_S) + { + argumentIndex = ((ParameterReference) instruction.Operand).Index; + return true; + } + else if (instruction.OpCode == OpCodes.Ldarg_0) + { + argumentIndex = 0; + return true; + } + else if (instruction.OpCode == OpCodes.Ldarg_1) + { + argumentIndex = 1; + return true; + } + else if (instruction.OpCode == OpCodes.Ldarg_2) + { + argumentIndex = 2; + return true; + } + else if (instruction.OpCode == OpCodes.Ldarg_3) + { + argumentIndex = 3; + return true; + } + else + { + argumentIndex = -1; + return false; + } + } + + #endregion + + + #region ldc.i4 + + public static bool IsLdc_I4(this Instruction instruction) + { + return IsLdc_I4(instruction, out int value); + } + + + public static bool IsLdc_I4(this Instruction instruction, int value) + { + return IsLdc_I4(instruction, out int actualValue) && + value == actualValue; + } + + + public static bool IsLdc_I4(this Instruction instruction, out int value) + { + if (instruction.OpCode == OpCodes.Ldc_I4 || + instruction.OpCode == OpCodes.Ldc_I4_S) + { + value = (int) instruction.Operand; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_0) + { + value = 0; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_1) + { + value = 1; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_2) + { + value = 2; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_3) + { + value = 3; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_4) + { + value = 4; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_5) + { + value = 5; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_6) + { + value = 6; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_7) + { + value = 7; + return true; + } + else if (instruction.OpCode == OpCodes.Ldc_I4_8) + { + value = 8; + return true; + } + else + { + value = 0; + return false; + } + } + + #endregion + + + #region ldloc + + public static bool IsLdloc(this Instruction instruction, int localVariableIndex) + { + return IsLdloc(instruction, out int actualLocalVariableIndex) && + localVariableIndex == actualLocalVariableIndex; + } + + + public static bool IsLdloc(this Instruction instruction) + { + return IsLdloc(instruction, out int _); + } + + + public static bool IsLdloc(this Instruction instruction, out int localVariableIndex) + { + if (instruction.OpCode == OpCodes.Ldloc || + instruction.OpCode == OpCodes.Ldloc_S) + { + localVariableIndex = ((VariableDefinition) instruction.Operand).Index; + return true; + } + else if (instruction.OpCode == OpCodes.Ldloc_0) + { + localVariableIndex = 0; + return true; + } + else if (instruction.OpCode == OpCodes.Ldloc_1) + { + localVariableIndex = 1; + return true; + } + else if (instruction.OpCode == OpCodes.Ldloc_2) + { + localVariableIndex = 2; + return true; + } + else if (instruction.OpCode == OpCodes.Ldloc_3) + { + localVariableIndex = 3; + return true; + } + else + { + localVariableIndex = -1; + return false; + } + } + + #endregion Ldloc + + + #region stloc + + public static bool IsStloc(this Instruction instruction, int localVariableIndex) + { + return IsStloc(instruction, out int actualLocalVariableIndex) && + localVariableIndex == actualLocalVariableIndex; + } + + + public static bool IsStloc(this Instruction instruction) + { + return IsStloc(instruction, out int _); + } + + + public static bool IsStloc(this Instruction instruction, out int localVariableIndex) + { + if (instruction.OpCode == OpCodes.Stloc || + instruction.OpCode == OpCodes.Stloc_S) + { + localVariableIndex = ((VariableDefinition) instruction.Operand).Index; + return true; + } + else if (instruction.OpCode == OpCodes.Stloc_0) + { + localVariableIndex = 0; + return true; + } + else if (instruction.OpCode == OpCodes.Stloc_1) + { + localVariableIndex = 1; + return true; + } + else if (instruction.OpCode == OpCodes.Stloc_2) + { + localVariableIndex = 2; + return true; + } + else if (instruction.OpCode == OpCodes.Stloc_3) + { + localVariableIndex = 3; + return true; + } + else + { + localVariableIndex = -1; + return false; + } + } + + #endregion + } +} diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 4f2f4720d..b8af5dabf 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -434,145 +434,297 @@ private static bool SkipGeneratedBranchesForAwaitForeach(List instr int currentIndex = instructions.BinarySearch(instruction, new InstructionByOffsetComparer()); - return SkipGeneratedBranchForAwaitForeach_CheckForAsyncEnumerator(instructions, instruction, currentIndex) || - SkipGeneratedBranchForAwaitForeach_CheckIfExceptionThrown(instructions, instruction, currentIndex) || - SkipGeneratedBranchForAwaitForeach_CheckThrownExceptionType(instructions, instruction, currentIndex); - } + return CheckForAsyncEnumerator(instructions, instruction, currentIndex) || + CheckIfExceptionThrown(instructions, instruction, currentIndex) || + CheckThrownExceptionType(instructions, instruction, currentIndex); - // The pattern for the "should we stay in the loop or not?", which we don't - // want to skip (so we have no method to try to find it), looks like this: - // - // IL_0111: ldloca.s 4 - // IL_0113: call instance !0 valuetype [System.Private.CoreLib]System.Runtime.CompilerServices.ValueTaskAwaiter`1::GetResult() - // IL_0118: brtrue IL_0047 - // - // In Debug mode, there are additional things that happen in between - // the "call" and branch, but it's the same idea either way: branch - // if GetResult() returned true. - - private static bool SkipGeneratedBranchForAwaitForeach_CheckForAsyncEnumerator(List instructions, Instruction instruction, int currentIndex) - { - // We're looking for the following pattern, which checks whether a - // compiler-generated field of type IAsyncEnumerator<> is null. + + // The pattern for the "should we stay in the loop or not?", which we don't + // want to skip (so we have no method to try to find it), looks like this: + // + // IL_0111: ldloca.s 4 + // IL_0113: call instance !0 valuetype [System.Private.CoreLib]System.Runtime.CompilerServices.ValueTaskAwaiter`1::GetResult() + // IL_0118: brtrue IL_0047 // - // IL_012b: ldarg.0 - // IL_012c: ldfld class [System.Private.CoreLib]System.Collections.Generic.IAsyncEnumerator`1 AwaitForeachStateMachine/'d__0'::'<>7__wrap1' - // IL_0131: brfalse.s IL_0196 + // In Debug mode, there are additional things that happen in between + // the "call" and branch, but it's the same idea either way: branch + // if GetResult() returned true. - if (instruction.OpCode != OpCodes.Brfalse && - instruction.OpCode != OpCodes.Brfalse_S) + + bool CheckForAsyncEnumerator(List instructions, Instruction instruction, int currentIndex) { + // We're looking for the following pattern, which checks whether a + // compiler-generated field of type IAsyncEnumerator<> is null. + // + // IL_012b: ldarg.0 + // IL_012c: ldfld class [System.Private.CoreLib]System.Collections.Generic.IAsyncEnumerator`1 AwaitForeachStateMachine/'d__0'::'<>7__wrap1' + // IL_0131: brfalse.s IL_0196 + + if (instruction.OpCode != OpCodes.Brfalse && + instruction.OpCode != OpCodes.Brfalse_S) + { + return false; + } + + if (currentIndex >= 2 && + instructions[currentIndex - 2].IsLdarg(0) && + instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && + instructions[currentIndex - 1].Operand is FieldDefinition field && + IsCompilerGenerated(field) && field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator")) + { + return true; + } + return false; } - if (currentIndex >= 2 && - (instructions[currentIndex - 2].OpCode == OpCodes.Ldarg || - instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0) && - instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && - instructions[currentIndex - 1].Operand is FieldDefinition field && - IsCompilerGenerated(field) && field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator")) + + bool CheckIfExceptionThrown(List instructions, Instruction instruction, int currentIndex) { - return true; - } + // Here, we want to find a pattern where we're checking whether a + // compiler-generated field of type Object is null. To narrow our + // search down and reduce the odds of false positives, we'll also + // expect a call to GetResult() to precede the loading of the field's + // value. The basic pattern looks like this: + // + // IL_018f: ldloca.s 2 + // IL_0191: call instance void [System.Private.CoreLib]System.Runtime.CompilerServices.ValueTaskAwaiter::GetResult() + // IL_0196: ldarg.0 + // IL_0197: ldfld object AwaitForeachStateMachine/'d__0'::'<>7__wrap2' + // IL_019c: stloc.s 6 + // IL_019e: ldloc.s 6 + // IL_01a0: brfalse.s IL_01b9 + // + // Variants are possible (e.g., a "dup" instruction instead of a + // "stloc.s" and "ldloc.s" pair), so we'll just look for the + // highlights. + + if (instruction.OpCode != OpCodes.Brfalse && + instruction.OpCode != OpCodes.Brfalse_S) + { + return false; + } - return false; - } + // We expect the field to be loaded no more than thre instructions before + // the branch, so that's how far we're willing to search for it. + int minFieldIndex = Math.Max(0, currentIndex - 3); - private static bool SkipGeneratedBranchForAwaitForeach_CheckIfExceptionThrown(List instructions, Instruction instruction, int currentIndex) - { - // Here, we want to find a pattern where we're checking whether a - // compiler-generated field of type Object is null. To narrow our - // search down and reduce the odds of false positives, we'll also - // expect a call to GetResult() to precede the loading of the field's - // value. The basic pattern looks like this: - // - // IL_018f: ldloca.s 2 - // IL_0191: call instance void [System.Private.CoreLib]System.Runtime.CompilerServices.ValueTaskAwaiter::GetResult() - // IL_0196: ldarg.0 - // IL_0197: ldfld object AwaitForeachStateMachine/'d__0'::'<>7__wrap2' - // IL_019c: stloc.s 6 - // IL_019e: ldloc.s 6 - // IL_01a0: brfalse.s IL_01b9 - // - // Variants are possible (e.g., a "dup" instruction instead of a - // "stloc.s" and "ldloc.s" pair), so we'll just look for the - // highlights. + for (int i = currentIndex - 1; i >= minFieldIndex; --i) + { + if (instructions[i].OpCode == OpCodes.Ldfld && + instructions[i].Operand is FieldDefinition field && + IsCompilerGenerated(field) && field.FieldType.FullName == "System.Object") + { + // We expect the call to GetResult() to be no more than three + // instructions before the loading of the field's value. + int minCallIndex = Math.Max(0, i - 3); + + for (int j = i - 1; j >= minCallIndex; --j) + { + if (instructions[j].OpCode == OpCodes.Call && + instructions[j].Operand is MethodReference callRef && + callRef.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices") && + callRef.DeclaringType.FullName.Contains("TaskAwait") && + callRef.Name == "GetResult") + { + return true; + } + } + } + } - if (instruction.OpCode != OpCodes.Brfalse && - instruction.OpCode != OpCodes.Brfalse_S) - { return false; } - // We expect the field to be loaded no more than thre instructions before - // the branch, so that's how far we're willing to search for it. - int minFieldIndex = Math.Max(0, currentIndex - 3); - for (int i = currentIndex - 1; i >= minFieldIndex; --i) + bool CheckThrownExceptionType(List instructions, Instruction instruction, int currentIndex) { - if (instructions[i].OpCode == OpCodes.Ldfld && - instructions[i].Operand is FieldDefinition field && - IsCompilerGenerated(field) && field.FieldType.FullName == "System.Object") + // In this case, we're looking for a branch generated by the compiler to + // check whether a previously-thrown exception has (at least) the type + // System.Exception, the pattern for which looks like this: + // + // IL_01db: ldloc.s 7 + // IL_01dd: isinst [System.Private.CoreLib]System.Exception + // IL_01e2: stloc.s 9 + // IL_01e4: ldloc.s 9 + // IL_01e6: brtrue.s IL_01eb + // + // Once again, variants are possible here, such as a "dup" instruction in + // place of the "stloc.s" and "ldloc.s" pair, and we'll reduce the odds of + // a false positive by requiring a "ldloc.s" instruction to precede the + // "isinst" instruction. + + if (instruction.OpCode != OpCodes.Brtrue && + instruction.OpCode != OpCodes.Brtrue_S) { - // We expect the call to GetResult() to be no more than three - // instructions before the loading of the field's value. - int minCallIndex = Math.Max(0, i - 3); + return false; + } - for (int j = i - 1; j >= minCallIndex; --j) + int minTypeCheckIndex = Math.Max(1, currentIndex - 3); + + for (int i = currentIndex - 1; i >= minTypeCheckIndex; --i) + { + if (instructions[i].OpCode == OpCodes.Isinst && + instructions[i].Operand is TypeReference typeRef && + typeRef.FullName == "System.Exception" && + instructions[i - 1].IsLdloc()) { - if (instructions[j].OpCode == OpCodes.Call && - instructions[j].Operand is MethodReference callRef && - callRef.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices") && - callRef.DeclaringType.FullName.Contains("TaskAwait") && - callRef.Name == "GetResult") - { - return true; - } + return true; } } - } - return false; + return false; + } } - private static bool SkipGeneratedBranchForAwaitForeach_CheckThrownExceptionType(List instructions, Instruction instruction, int currentIndex) + private static bool SkipGeneratedBranchesForAwaitUsing(List instructions, Instruction instruction) { - // In this case, we're looking for a branch generated by the compiler to - // check whether a previously-thrown exception has (at least) the type - // System.Exception, the pattern for which looks like this: - // - // IL_01db: ldloc.s 7 - // IL_01dd: isinst [System.Private.CoreLib]System.Exception - // IL_01e2: stloc.s 9 - // IL_01e4: ldloc.s 9 - // IL_01e6: brtrue.s IL_01eb - // - // Once again, variants are possible here, such as a "dup" instruction in - // place of the "stloc.s" and "ldloc.s" pair, and we'll reduce the odds of - // a false positive by requiring a "ldloc.s" instruction to precede the - // "isinst" instruction. + int currentIndex = instructions.BinarySearch(instruction, new InstructionByOffsetComparer()); + + return CheckForSkipDisposal(instructions, instruction, currentIndex) || + CheckForCleanup(instructions, instruction, currentIndex); - if (instruction.OpCode != OpCodes.Brtrue && - instruction.OpCode != OpCodes.Brtrue_S) + + bool CheckForSkipDisposal(List instructions, Instruction instruction, int currentIndex) { + // The async state machine generated for an "await using" contains a branch + // that checks whether the call to DisposeAsync() needs to be skipped. + // As it turns out, the pattern is a little different in Debug and Release + // configurations, but the idea is the same in either case: + // + // * Check whether the IAsyncDisposable that's being managed by the "await + // using" is null. If so, skip the call to DisposeAsync(). If not, + // make the call. + // + // To avoid other places where this pattern is used, we'll require it to be + // preceded by the tail end of a try/catch generated by the compiler. + // + // The primary difference between the Debug and Release versions of this + // pattern are where that IAsyncDisposable is stored. In Debug mode, it's + // in a field; in Release mode, it's in a local variable. So what we'll + // look for, generally, is a brfalse instruction that checks a value, + // followed shortly by reloading that same value and calling DisposeAsync() + // on it, preceded shortly by a store into a compiler-generated field and + // a "leave" instruction. + // + // Debug version: + // IL_0041: stfld object Coverlet.Core.Samples.Tests.AwaitUsing/'d__0'::'<>s__2' + // IL_0046: leave.s IL_0048 + // IL_0048: ldarg.0 + // IL_0049: ldfld class [System.Private.CoreLib]System.IO.MemoryStream Coverlet.Core.Samples.Tests.AwaitUsing/'d__0'::'5__1' + // IL_004e: brfalse.s IL_00b9 + // IL_0050: ldarg.0 + // IL_0051: ldfld class [System.Private.CoreLib]System.IO.MemoryStream Coverlet.Core.Samples.Tests.AwaitUsing/'d__0'::'5__1' + // IL_0056: callvirt instance valuetype [System.Private.CoreLib]System.Threading.Tasks.ValueTask [System.Private.CoreLib]System.IAsyncDisposable::DisposeAsync() + // + // Release version: + // IL_0032: stfld object Coverlet.Core.Samples.Tests.AwaitUsing/'d__0'::'<>7__wrap1' + // IL_0037: leave.s IL_0039 + // IL_0039: ldloc.1 + // IL_003a: brfalse.s IL_0098 + // IL_003c: ldloc.1 + // IL_003d: callvirt instance valuetype [System.Private.CoreLib]System.Threading.Tasks.ValueTask [System.Private.CoreLib]System.IAsyncDisposable::DisposeAsync() + + if (instruction.OpCode != OpCodes.Brfalse && + instruction.OpCode != OpCodes.Brfalse_S) + { + return false; + } + + bool isFollowedByDisposeAsync = false; + + if (instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && + instructions[currentIndex - 1].Operand is FieldDefinition field && + IsCompilerGenerated(field)) + { + int maxReloadFieldIndex = Math.Min(currentIndex + 2, instructions.Count - 2); + + for (int i = currentIndex + 1; i <= maxReloadFieldIndex; ++i) + { + if (instructions[i].OpCode == OpCodes.Ldfld && + instructions[i].Operand is FieldDefinition reloadedField && + field.Equals(reloadedField) && + instructions[i + 1].OpCode == OpCodes.Callvirt && + instructions[i + 1].Operand is MethodReference method && + method.DeclaringType.FullName == "System.IAsyncDisposable" && + method.Name == "DisposeAsync") + { + isFollowedByDisposeAsync = true; + break; + } + } + } + else if (instructions[currentIndex - 1].IsLdloc(out int localVariableIndex) && + instructions[currentIndex + 1].IsLdloc(out int reloadedLocalVariableIndex) && + localVariableIndex == reloadedLocalVariableIndex && + instructions[currentIndex + 2].OpCode == OpCodes.Callvirt && + instructions[currentIndex + 2].Operand is MethodReference method && + method.DeclaringType.FullName == "System.IAsyncDisposable" && + method.Name == "DisposeAsync") + { + isFollowedByDisposeAsync = true; + } + + if (isFollowedByDisposeAsync) + { + int minLeaveIndex = Math.Max(1, currentIndex - 4); + + for (int i = currentIndex - 1; i >= minLeaveIndex; --i) + { + if ((instructions[i].OpCode == OpCodes.Leave || + instructions[i].OpCode == OpCodes.Leave_S) && + instructions[i - 1].OpCode == OpCodes.Stfld && + instructions[i - 1].Operand is FieldDefinition storeField && + IsCompilerGenerated(storeField)) + { + return true; + } + } + } + return false; } - int minTypeCheckIndex = Math.Max(1, currentIndex - 3); - for (int i = currentIndex - 1; i >= minTypeCheckIndex; --i) + bool CheckForCleanup(List instructions, Instruction instruction, int currentIndex) { - if (instructions[i].OpCode == OpCodes.Isinst && - instructions[i].Operand is TypeReference typeRef && - typeRef.FullName == "System.Exception" && - (instructions[i - 1].OpCode == OpCodes.Ldloc || - instructions[i - 1].OpCode == OpCodes.Ldloc_S)) + if (instruction.OpCode != OpCodes.Beq && + instruction.OpCode != OpCodes.Beq_S && + instruction.OpCode != OpCodes.Bne_Un && + instruction.OpCode != OpCodes.Bne_Un_S) { - return true; + return false; } - } - return false; + if (currentIndex >= 1 && + instructions[currentIndex - 1].IsLdc_I4(1)) + { + int minLoadFieldIndex = Math.Max(1, currentIndex - 5); + + for (int i = currentIndex - 2; i >= minLoadFieldIndex; --i) + { + if (instructions[i].OpCode == OpCodes.Ldfld && + instructions[i].Operand is FieldDefinition loadedField && + IsCompilerGenerated(loadedField)) + { + int minRethrowIndex = Math.Max(0, i - 4); + + for (int j = i - 1; j >= minRethrowIndex; --j) + { + if (instructions[j].OpCode == OpCodes.Callvirt && + instructions[j].Operand is MethodReference method && + method.DeclaringType.FullName == "System.Runtime.ExceptionServices.ExceptionDispatchInfo" && + method.Name == "Throw") + { + return true; + } + } + } + } + } + + return false; + } } // https://docs.microsoft.com/en-us/archive/msdn-magazine/2019/november/csharp-iterating-with-async-enumerables-in-csharp-8 @@ -590,82 +742,86 @@ private static bool SkipGeneratedBranchesForAsyncIterator(List inst int currentIndex = instructions.BinarySearch(instruction, new InstructionByOffsetComparer()); - return SkipGeneratedBranchesForAsyncIterator_CheckForStateSwitch(instructions, instruction, currentIndex) || - SkipGeneratedBranchesForAsyncIterator_DisposeCheck(instructions, instruction, currentIndex); - } + return CheckForStateSwitch(instructions, instruction, currentIndex) || + DisposeCheck(instructions, instruction, currentIndex); - private static bool SkipGeneratedBranchesForAsyncIterator_CheckForStateSwitch(List instructions, Instruction instruction, int currentIndex) - { - // The pattern we're looking for here is this one: - // - // IL_0000: ldarg.0 - // IL_0001: ldfld int32 Test.AsyncEnumerableStateMachine/'d__0'::'<>1__state' - // IL_0006: stloc.0 - // .try - // { - // IL_0007: ldloc.0 - // IL_0008: ldc.i4.s -4 - // IL_000a: sub - // IL_000b: switch (IL_0026, IL_002b, IL_002f, IL_002f, IL_002d) - // - // The "switch" instruction is the branch we want to skip. To eliminate - // false positives, we'll search back for the "ldfld" of the compiler- - // generated "<>1__state" field, making sure it precedes it within five - // instructions. To be safe, we'll also require a "ldarg.0" instruction - // before the "ldfld". - if (instruction.OpCode != OpCodes.Switch) + bool CheckForStateSwitch(List instructions, Instruction instruction, int currentIndex) { + // The pattern we're looking for here is this one: + // + // IL_0000: ldarg.0 + // IL_0001: ldfld int32 Test.AsyncEnumerableStateMachine/'d__0'::'<>1__state' + // IL_0006: stloc.0 + // .try + // { + // IL_0007: ldloc.0 + // IL_0008: ldc.i4.s -4 + // IL_000a: sub + // IL_000b: switch (IL_0026, IL_002b, IL_002f, IL_002f, IL_002d) + // + // The "switch" instruction is the branch we want to skip. To eliminate + // false positives, we'll search back for the "ldfld" of the compiler- + // generated "<>1__state" field, making sure it precedes it within five + // instructions. To be safe, we'll also require a "ldarg.0" instruction + // before the "ldfld". + + if (instruction.OpCode != OpCodes.Switch) + { + return false; + } + + int minLoadStateFieldIndex = Math.Max(1, currentIndex - 5); + + for (int i = currentIndex - 1; i >= minLoadStateFieldIndex; --i) + { + if (instructions[i].OpCode == OpCodes.Ldfld && + instructions[i].Operand is FieldDefinition field && + IsCompilerGenerated(field) && field.FullName.EndsWith("__state") && + (instructions[i - 1].OpCode == OpCodes.Ldarg || + instructions[i - 1].OpCode == OpCodes.Ldarg_0)) + { + return true; + } + } + return false; } - int minLoadStateFieldIndex = Math.Max(1, currentIndex - 5); - for (int i = currentIndex - 1; i >= minLoadStateFieldIndex; --i) + bool DisposeCheck(List instructions, Instruction instruction, int currentIndex) { - if (instructions[i].OpCode == OpCodes.Ldfld && - instructions[i].Operand is FieldDefinition field && - IsCompilerGenerated(field) && field.FullName.EndsWith("__state") && - (instructions[i - 1].OpCode == OpCodes.Ldarg || - instructions[i - 1].OpCode == OpCodes.Ldarg_0)) + // Within the compiler-generated async iterator, there are at least a + // couple of places where we find this pattern, in which the async + // iterator is checking whether it's been disposed, so it'll know to + // stop iterating. + // + // IL_0024: ldarg.0 + // IL_0025: ldfld bool Test.AsyncEnumerableStateMachine/'d__0'::'<>w__disposeMode' + // IL_002a: brfalse.s IL_0031 + // + // We'll eliminate these wherever they appear. It's reasonable to just + // look for a "brfalse" or "brfalse.s" instruction, preceded immediately + // by "ldfld" of the compiler-generated "<>w__disposeMode" field. + + if (instruction.OpCode != OpCodes.Brfalse && + instruction.OpCode != OpCodes.Brfalse_S) { - return true; + return false; } - } - return false; - } - - private static bool SkipGeneratedBranchesForAsyncIterator_DisposeCheck(List instructions, Instruction instruction, int currentIndex) - { - // Within the compiler-generated async iterator, there are at least a - // couple of places where we find this pattern, in which the async - // iterator is checking whether it's been disposed, so it'll know to - // stop iterating. - // - // IL_0024: ldarg.0 - // IL_0025: ldfld bool Test.AsyncEnumerableStateMachine/'d__0'::'<>w__disposeMode' - // IL_002a: brfalse.s IL_0031 - // - // We'll eliminate these wherever they appear. It's reasonable to just - // look for a "brfalse" or "brfalse.s" instruction, preceded immediately - // by "ldfld" of the compiler-generated "<>w__disposeMode" field. + if (currentIndex >= 2 && + instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && + instructions[currentIndex - 1].Operand is FieldDefinition field && + IsCompilerGenerated(field) && field.FullName.EndsWith("__disposeMode") && + (instructions[currentIndex - 2].OpCode == OpCodes.Ldarg || + instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0)) + { + return true; + } - if (instruction.OpCode != OpCodes.Brfalse && - instruction.OpCode != OpCodes.Brfalse_S) - { return false; } - - if (currentIndex >= 1 && - instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && - instructions[currentIndex - 1].Operand is FieldDefinition field && - IsCompilerGenerated(field) && field.FullName.EndsWith("__disposeMode")) - { - return true; - } - - return false; } // https://github.com/dotnet/roslyn/blob/master/docs/compilers/CSharp/Expression%20Breakpoints.md @@ -735,7 +891,8 @@ public IReadOnlyList GetBranchPoints(MethodDefinition methodDefinit { if (SkipGeneratedBranchesForExceptionHandlers(methodDefinition, instruction, instructions) || SkipGeneratedBranchForExceptionRethrown(instructions, instruction) || - SkipGeneratedBranchesForAwaitForeach(instructions, instruction)) + SkipGeneratedBranchesForAwaitForeach(instructions, instruction) || + SkipGeneratedBranchesForAwaitUsing(instructions, instruction)) { continue; } diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs b/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs new file mode 100644 index 000000000..9b6ae0a60 --- /dev/null +++ b/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs @@ -0,0 +1,52 @@ +using System.IO; +using System.Threading.Tasks; + +using Coverlet.Core.Samples.Tests; +using Coverlet.Tests.Xunit.Extensions; +using Xunit; + +namespace Coverlet.Core.Tests +{ + public partial class CoverageTests + { + [Fact] + public void AwaitUsing() + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.RunInProcess(async (string[] pathSerialize) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + ((ValueTask)instance.HasAwaitUsing()).GetAwaiter().GetResult(); + ((Task)instance.Issue914_Repro()).GetAwaiter().GetResult(); + + return Task.CompletedTask; + }, persistPrepareResultToFile: pathSerialize[0]); + return 0; + }, new string[] { path }); + + TestInstrumentationHelper.GetCoverageResult(path) + .Document("Instrumentation.AwaitUsing.cs") + .AssertLinesCovered(BuildConfiguration.Debug, + // HasAwaitUsing() + (13, 1), (14, 1), (15, 1), (16, 1), (17, 1), + // Issue914_Repro() + (21, 1), (22, 1), (23, 1), (24, 1), + // Issue914_Repro_Example1() + (28, 1), (29, 1), (30, 1), + // Issue914_Repro_Example2() + (34, 1), (35, 1), (36, 1), (37, 1), + // MyTransaction.DisposeAsync() + (43, 2), (44, 2), (45, 2) + ) + .ExpectedTotalNumberOfBranches(BuildConfiguration.Debug, 0); + } + finally + { + File.Delete(path); + } + } + } +} diff --git a/test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs b/test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs new file mode 100644 index 000000000..9353a8c46 --- /dev/null +++ b/test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs @@ -0,0 +1,48 @@ +// Remember to use full name because adding new using directives change line numbers + +using System; +using System.IO; +using System.Text; +using System.Threading.Tasks; + +namespace Coverlet.Core.Samples.Tests +{ + public class AwaitUsing + { + async public ValueTask HasAwaitUsing() + { + await using (var ms = new MemoryStream(Encoding.ASCII.GetBytes("Boo"))) + { + } + } + + + async public Task Issue914_Repro() + { + await Issue914_Repro_Example1(); + await Issue914_Repro_Example2(); + } + + + async private Task Issue914_Repro_Example1() + { + await using var transaction = new MyTransaction(); + } + + + async private Task Issue914_Repro_Example2() + { + var transaction = new MyTransaction(); + await transaction.DisposeAsync(); + } + + + private class MyTransaction : IAsyncDisposable + { + public async ValueTask DisposeAsync() + { + await default(ValueTask); + } + } + } +} diff --git a/test/coverlet.core.tests/Samples/Samples.cs b/test/coverlet.core.tests/Samples/Samples.cs index 203555343..c34f9a669 100644 --- a/test/coverlet.core.tests/Samples/Samples.cs +++ b/test/coverlet.core.tests/Samples/Samples.cs @@ -242,6 +242,24 @@ async public IAsyncEnumerable CreateSequenceAsync() } } + public class AwaitUsingStateMachine + { + async public ValueTask HasAwaitUsing() + { + await using (var ms = new MemoryStream(System.Text.Encoding.ASCII.GetBytes("Boo"))) + { + } + } + } + + public class ScopedAwaitUsingStateMachine + { + async public ValueTask HasScopedAwaitUsing() + { + await using var ms = new MemoryStream(System.Text.Encoding.ASCII.GetBytes("Boo")); + } + } + [ExcludeFromCoverage] public class ClassExcludedByCoverletCodeCoverageAttr { diff --git a/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs b/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs index fbf477610..3107db0a3 100644 --- a/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs +++ b/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs @@ -360,7 +360,7 @@ public void GetBranchPoints_IgnoresMostBranchesIn_AwaitForeachStateMachine_WithB } [Fact] - public void GetBranchesPoints_IgnoresExtraBranchesIn_AsyncIteratorStateMachine() + public void GetBranchPoints_IgnoresExtraBranchesIn_AsyncIteratorStateMachine() { // arrange var nestedName = typeof(AsyncIteratorStateMachine).GetNestedTypes(BindingFlags.NonPublic).First().Name; @@ -379,6 +379,38 @@ public void GetBranchesPoints_IgnoresExtraBranchesIn_AsyncIteratorStateMachine() Assert.Equal(237, points[1].StartLine); } + [Fact] + public void GetBranchPoints_IgnoreBranchesIn_AwaitUsingStateMachine() + { + // arrange + var nestedName = typeof(AwaitUsingStateMachine).GetNestedTypes(BindingFlags.NonPublic).First().Name; + var type = _module.Types.FirstOrDefault(x => x.FullName == typeof(AwaitUsingStateMachine).FullName); + var nestedType = type.NestedTypes.FirstOrDefault(x => x.FullName.EndsWith(nestedName)); + var method = nestedType.Methods.First(x => x.FullName.EndsWith("::MoveNext()")); + + // act + var points = _cecilSymbolHelper.GetBranchPoints(method); + + // assert + Assert.Empty(points); + } + + [Fact] + public void GetBranchPoints_IgnoreBranchesIn_ScopedAwaitUsingStateMachine() + { + // arrange + var nestedName = typeof(ScopedAwaitUsingStateMachine).GetNestedTypes(BindingFlags.NonPublic).First().Name; + var type = _module.Types.FirstOrDefault(x => x.FullName == typeof(ScopedAwaitUsingStateMachine).FullName); + var nestedType = type.NestedTypes.FirstOrDefault(x => x.FullName.EndsWith(nestedName)); + var method = nestedType.Methods.First(x => x.FullName.EndsWith("::MoveNext()")); + + // act + var points = _cecilSymbolHelper.GetBranchPoints(method); + + // assert + Assert.Empty(points); + } + [Fact] public void GetBranchPoints_ExceptionFilter() { From 59d9277acb1879dbbac2fd7b6b37912341313e95 Mon Sep 17 00:00:00 2001 From: Alex Thornton Date: Sat, 6 Mar 2021 17:53:02 -0800 Subject: [PATCH 2/4] RunInProcess -> Run --- test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs b/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs index 9b6ae0a60..a5849090f 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs @@ -15,7 +15,7 @@ public void AwaitUsing() string path = Path.GetTempFileName(); try { - FunctionExecutor.RunInProcess(async (string[] pathSerialize) => + FunctionExecutor.Run(async (string[] pathSerialize) => { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { From 0f5be78c2df5fdc571d4bab30191c53657a40ca1 Mon Sep 17 00:00:00 2001 From: Alex Thornton Date: Sat, 6 Mar 2021 18:17:00 -0800 Subject: [PATCH 3/4] Added missing comment Added a missing comment describing one of the patterns we're searching for. --- .../Symbols/CecilSymbolHelper.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index b8af5dabf..c3324d3e3 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -688,6 +688,30 @@ instructions[i].Operand is FieldDefinition reloadedField && bool CheckForCleanup(List instructions, Instruction instruction, int currentIndex) { + // The pattern we're looking for here is this: + // + // IL_00c6: ldloc.s 5 + // IL_00c8: call class [System.Private.CoreLib]System.Runtime.ExceptionServices.ExceptionDispatchInfo [System.Private.CoreLib]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Capture(class [System.Private.CoreLib]System.Exception) + // IL_00cd: callvirt instance void [System.Private.CoreLib]System.Runtime.ExceptionServices.ExceptionDispatchInfo::Throw() + // IL_00d2: nop + // IL_00d3: ldarg.0 + // IL_00d4: ldfld int32 Coverlet.Core.Samples.Tests.AwaitUsing/'d__2'::'<>s__3' + // IL_00d9: stloc.s 6 + // IL_00db: ldloc.s 6 + // IL_00dd: ldc.i4.1 + // IL_00de: beq.s IL_00e2 + // IL_00e0: br.s IL_00e4 + // IL_00e2: leave.s IL_0115 + // + // It appears that this pattern is not generated in every "await using", + // but only in an "await using" without curly braces (i.e., that is + // scoped to the end of the method). It's also a slightly different + // pattern in Release vs. Debug (bne.un.s instead of beq.s followed by + // br.s). To be as safe as we can, we'll expect an ldc.i4 to precede + // the branch, then we want a load from a compiler-generated field within + // a few instructions before that, then we want an exception to be + // rethrown before that. + if (instruction.OpCode != OpCodes.Beq && instruction.OpCode != OpCodes.Beq_S && instruction.OpCode != OpCodes.Bne_Un && From 56d124de6da11d64ce89de14369d78afb0dc14be Mon Sep 17 00:00:00 2001 From: Alex Thornton Date: Sun, 7 Mar 2021 15:44:20 -0800 Subject: [PATCH 4/4] Adjustments from code review Two adjustments for things that arose during code review: (1) Now using static local functions instead of non-static ones. (2) Removed InstructionHelpers class and reverted to straightforward ways of checking for instruction types. --- .../Extensions/InstructionExtensions.cs | 247 ------------------ .../Symbols/CecilSymbolHelper.cs | 42 ++- 2 files changed, 29 insertions(+), 260 deletions(-) delete mode 100644 src/coverlet.core/Extensions/InstructionExtensions.cs diff --git a/src/coverlet.core/Extensions/InstructionExtensions.cs b/src/coverlet.core/Extensions/InstructionExtensions.cs deleted file mode 100644 index 7c804dad5..000000000 --- a/src/coverlet.core/Extensions/InstructionExtensions.cs +++ /dev/null @@ -1,247 +0,0 @@ -using Mono.Cecil; -using Mono.Cecil.Cil; - - -namespace Coverlet.Core.Extensions -{ - // Extension methods that can smooth out small differences between similar CIL - // instructions, such as "ldarg.0" as opposed to "ldarg 0". - public static class InstructionExtensions - { - #region ldarg - - public static bool IsLdarg(this Instruction instruction, int argumentIndex) - { - return IsLdarg(instruction, out int actualArgumentIndex) && - argumentIndex == actualArgumentIndex; - } - - - public static bool IsLdarg(this Instruction instruction) - { - return IsLdarg(instruction, out int _); - } - - - public static bool IsLdarg(this Instruction instruction, out int argumentIndex) - { - if (instruction.OpCode == OpCodes.Ldarg || - instruction.OpCode == OpCodes.Ldarg_S) - { - argumentIndex = ((ParameterReference) instruction.Operand).Index; - return true; - } - else if (instruction.OpCode == OpCodes.Ldarg_0) - { - argumentIndex = 0; - return true; - } - else if (instruction.OpCode == OpCodes.Ldarg_1) - { - argumentIndex = 1; - return true; - } - else if (instruction.OpCode == OpCodes.Ldarg_2) - { - argumentIndex = 2; - return true; - } - else if (instruction.OpCode == OpCodes.Ldarg_3) - { - argumentIndex = 3; - return true; - } - else - { - argumentIndex = -1; - return false; - } - } - - #endregion - - - #region ldc.i4 - - public static bool IsLdc_I4(this Instruction instruction) - { - return IsLdc_I4(instruction, out int value); - } - - - public static bool IsLdc_I4(this Instruction instruction, int value) - { - return IsLdc_I4(instruction, out int actualValue) && - value == actualValue; - } - - - public static bool IsLdc_I4(this Instruction instruction, out int value) - { - if (instruction.OpCode == OpCodes.Ldc_I4 || - instruction.OpCode == OpCodes.Ldc_I4_S) - { - value = (int) instruction.Operand; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_0) - { - value = 0; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_1) - { - value = 1; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_2) - { - value = 2; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_3) - { - value = 3; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_4) - { - value = 4; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_5) - { - value = 5; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_6) - { - value = 6; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_7) - { - value = 7; - return true; - } - else if (instruction.OpCode == OpCodes.Ldc_I4_8) - { - value = 8; - return true; - } - else - { - value = 0; - return false; - } - } - - #endregion - - - #region ldloc - - public static bool IsLdloc(this Instruction instruction, int localVariableIndex) - { - return IsLdloc(instruction, out int actualLocalVariableIndex) && - localVariableIndex == actualLocalVariableIndex; - } - - - public static bool IsLdloc(this Instruction instruction) - { - return IsLdloc(instruction, out int _); - } - - - public static bool IsLdloc(this Instruction instruction, out int localVariableIndex) - { - if (instruction.OpCode == OpCodes.Ldloc || - instruction.OpCode == OpCodes.Ldloc_S) - { - localVariableIndex = ((VariableDefinition) instruction.Operand).Index; - return true; - } - else if (instruction.OpCode == OpCodes.Ldloc_0) - { - localVariableIndex = 0; - return true; - } - else if (instruction.OpCode == OpCodes.Ldloc_1) - { - localVariableIndex = 1; - return true; - } - else if (instruction.OpCode == OpCodes.Ldloc_2) - { - localVariableIndex = 2; - return true; - } - else if (instruction.OpCode == OpCodes.Ldloc_3) - { - localVariableIndex = 3; - return true; - } - else - { - localVariableIndex = -1; - return false; - } - } - - #endregion Ldloc - - - #region stloc - - public static bool IsStloc(this Instruction instruction, int localVariableIndex) - { - return IsStloc(instruction, out int actualLocalVariableIndex) && - localVariableIndex == actualLocalVariableIndex; - } - - - public static bool IsStloc(this Instruction instruction) - { - return IsStloc(instruction, out int _); - } - - - public static bool IsStloc(this Instruction instruction, out int localVariableIndex) - { - if (instruction.OpCode == OpCodes.Stloc || - instruction.OpCode == OpCodes.Stloc_S) - { - localVariableIndex = ((VariableDefinition) instruction.Operand).Index; - return true; - } - else if (instruction.OpCode == OpCodes.Stloc_0) - { - localVariableIndex = 0; - return true; - } - else if (instruction.OpCode == OpCodes.Stloc_1) - { - localVariableIndex = 1; - return true; - } - else if (instruction.OpCode == OpCodes.Stloc_2) - { - localVariableIndex = 2; - return true; - } - else if (instruction.OpCode == OpCodes.Stloc_3) - { - localVariableIndex = 3; - return true; - } - else - { - localVariableIndex = -1; - return false; - } - } - - #endregion - } -} diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index c3324d3e3..66db95e5b 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -451,7 +451,7 @@ private static bool SkipGeneratedBranchesForAwaitForeach(List instr // if GetResult() returned true. - bool CheckForAsyncEnumerator(List instructions, Instruction instruction, int currentIndex) + static bool CheckForAsyncEnumerator(List instructions, Instruction instruction, int currentIndex) { // We're looking for the following pattern, which checks whether a // compiler-generated field of type IAsyncEnumerator<> is null. @@ -467,7 +467,8 @@ bool CheckForAsyncEnumerator(List instructions, Instruction instruc } if (currentIndex >= 2 && - instructions[currentIndex - 2].IsLdarg(0) && + (instructions[currentIndex - 2].OpCode == OpCodes.Ldarg || + instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0) && instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && instructions[currentIndex - 1].Operand is FieldDefinition field && IsCompilerGenerated(field) && field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator")) @@ -479,7 +480,7 @@ bool CheckForAsyncEnumerator(List instructions, Instruction instruc } - bool CheckIfExceptionThrown(List instructions, Instruction instruction, int currentIndex) + static bool CheckIfExceptionThrown(List instructions, Instruction instruction, int currentIndex) { // Here, we want to find a pattern where we're checking whether a // compiler-generated field of type Object is null. To narrow our @@ -537,7 +538,7 @@ instructions[j].Operand is MethodReference callRef && } - bool CheckThrownExceptionType(List instructions, Instruction instruction, int currentIndex) + static bool CheckThrownExceptionType(List instructions, Instruction instruction, int currentIndex) { // In this case, we're looking for a branch generated by the compiler to // check whether a previously-thrown exception has (at least) the type @@ -567,7 +568,12 @@ bool CheckThrownExceptionType(List instructions, Instruction instru if (instructions[i].OpCode == OpCodes.Isinst && instructions[i].Operand is TypeReference typeRef && typeRef.FullName == "System.Exception" && - instructions[i - 1].IsLdloc()) + (instructions[i - 1].OpCode == OpCodes.Ldloc || + instructions[i - 1].OpCode == OpCodes.Ldloc_S || + instructions[i - 1].OpCode == OpCodes.Ldloc_0 || + instructions[i - 1].OpCode == OpCodes.Ldloc_1 || + instructions[i - 1].OpCode == OpCodes.Ldloc_2 || + instructions[i - 1].OpCode == OpCodes.Ldloc_3)) { return true; } @@ -585,7 +591,7 @@ private static bool SkipGeneratedBranchesForAwaitUsing(List instruc CheckForCleanup(instructions, instruction, currentIndex); - bool CheckForSkipDisposal(List instructions, Instruction instruction, int currentIndex) + static bool CheckForSkipDisposal(List instructions, Instruction instruction, int currentIndex) { // The async state machine generated for an "await using" contains a branch // that checks whether the call to DisposeAsync() needs to be skipped. @@ -654,9 +660,18 @@ instructions[i].Operand is FieldDefinition reloadedField && } } } - else if (instructions[currentIndex - 1].IsLdloc(out int localVariableIndex) && - instructions[currentIndex + 1].IsLdloc(out int reloadedLocalVariableIndex) && - localVariableIndex == reloadedLocalVariableIndex && + else if ((instructions[currentIndex - 1].OpCode == OpCodes.Ldloc || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_S || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_0 || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_1 || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_2 || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_3) && + (instructions[currentIndex + 1].OpCode == OpCodes.Ldloc || + instructions[currentIndex + 1].OpCode == OpCodes.Ldloc_S || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_0 || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_1 || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_2 || + instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_3) && instructions[currentIndex + 2].OpCode == OpCodes.Callvirt && instructions[currentIndex + 2].Operand is MethodReference method && method.DeclaringType.FullName == "System.IAsyncDisposable" && @@ -686,7 +701,7 @@ instructions[i].Operand is FieldDefinition reloadedField && } - bool CheckForCleanup(List instructions, Instruction instruction, int currentIndex) + static bool CheckForCleanup(List instructions, Instruction instruction, int currentIndex) { // The pattern we're looking for here is this: // @@ -721,7 +736,8 @@ bool CheckForCleanup(List instructions, Instruction instruction, in } if (currentIndex >= 1 && - instructions[currentIndex - 1].IsLdc_I4(1)) + (instructions[currentIndex - 1].OpCode == OpCodes.Ldc_I4 || + instructions[currentIndex - 1].OpCode == OpCodes.Ldc_I4_1)) { int minLoadFieldIndex = Math.Max(1, currentIndex - 5); @@ -770,7 +786,7 @@ private static bool SkipGeneratedBranchesForAsyncIterator(List inst DisposeCheck(instructions, instruction, currentIndex); - bool CheckForStateSwitch(List instructions, Instruction instruction, int currentIndex) + static bool CheckForStateSwitch(List instructions, Instruction instruction, int currentIndex) { // The pattern we're looking for here is this one: // @@ -813,7 +829,7 @@ instructions[i].Operand is FieldDefinition field && } - bool DisposeCheck(List instructions, Instruction instruction, int currentIndex) + static bool DisposeCheck(List instructions, Instruction instruction, int currentIndex) { // Within the compiler-generated async iterator, there are at least a // couple of places where we find this pattern, in which the async