Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 0 additions & 43 deletions src/coverlet.core/Coverage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,53 +279,10 @@ private void CalculateCoverage()
}
}
}

// for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance)
// we'll remove all MoveNext() not covered branch
foreach (var document in result.Documents)
{
List<KeyValuePair<BranchKey, Branch>> branchesToRemove = new List<KeyValuePair<BranchKey, Branch>>();
foreach (var branch in document.Value.Branches)
{
//if one branch is covered we search the other one only if it's not covered
if (IsAsyncStateMachineMethod(branch.Value.Method) && branch.Value.Hits > 0)
{
foreach (var moveNextBranch in document.Value.Branches)
{
if (moveNextBranch.Value.Method == branch.Value.Method && moveNextBranch.Value != branch.Value && moveNextBranch.Value.Hits == 0)
{
branchesToRemove.Add(moveNextBranch);
}
}
}
}
foreach (var branchToRemove in branchesToRemove)
{
document.Value.Branches.Remove(branchToRemove.Key);
}
}

_instrumentationHelper.DeleteHitsFile(result.HitsFilePath);
}
}

private bool IsAsyncStateMachineMethod(string method)
{
if (!method.EndsWith("::MoveNext()"))
{
return false;
}

foreach (var instrumentationResult in _results)
{
if (instrumentationResult.AsyncMachineStateMethod.Contains(method))
{
return true;
}
}
return false;
}

private string GetSourceLinkUrl(Dictionary<string, string> sourceLinkDocuments, string document)
{
if (sourceLinkDocuments.TryGetValue(document, out string url))
Expand Down
51 changes: 0 additions & 51 deletions src/coverlet.core/CoverageResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,57 +108,6 @@ internal void Merge(Modules modules)
}
}
}

// for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance)
// we'll remove all MoveNext() not covered branch
List<BranchInfo> branchesToRemove = new List<BranchInfo>();
foreach (var module in this.Modules)
{
foreach (var document in module.Value)
{
foreach (var @class in document.Value)
{
foreach (var method in @class.Value)
{
foreach (var branch in method.Value.Branches)
{
//if one branch is covered we search the other one only if it's not covered
if (IsAsyncStateMachineMethod(method.Key) && branch.Hits > 0)
{
foreach (var moveNextBranch in method.Value.Branches)
{
if (moveNextBranch != branch && moveNextBranch.Hits == 0)
{
branchesToRemove.Add(moveNextBranch);
}
}
}
}
foreach (var branchToRemove in branchesToRemove)
{
method.Value.Branches.Remove(branchToRemove);
}
}
}
}
}
}

private bool IsAsyncStateMachineMethod(string method)
{
if (!method.EndsWith("::MoveNext()"))
{
return false;
}

foreach (var instrumentedResult in InstrumentedResults)
{
if (instrumentedResult.AsyncMachineStateMethod.Contains(method))
{
return true;
}
}
return false;
}

public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summary, double threshold, ThresholdTypeFlags thresholdTypes, ThresholdStatistic thresholdStat)
Expand Down
39 changes: 3 additions & 36 deletions src/coverlet.core/Instrumentation/Instrumenter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ internal class Instrumenter
private TypeDefinition _customTrackerTypeDef;
private MethodReference _customTrackerRegisterUnloadEventsMethod;
private MethodReference _customTrackerRecordHitMethod;
private List<string> _asyncMachineStateMethod;
private List<string> _excludedSourceFiles;

public Instrumenter(
Expand Down Expand Up @@ -121,8 +120,6 @@ public InstrumenterResult Instrument()

InstrumentModule();

_result.AsyncMachineStateMethod = _asyncMachineStateMethod == null ? Array.Empty<string>() : _asyncMachineStateMethod.ToArray();

if (_excludedSourceFiles != null)
{
foreach (string sourceFile in _excludedSourceFiles)
Expand Down Expand Up @@ -396,18 +393,18 @@ private void InstrumentIL(MethodDefinition method)
index += 2;
}

foreach (var _branchTarget in targetedBranchPoints)
foreach (var branchTarget in targetedBranchPoints)
{
/*
* Skip branches with no sequence point reference for now.
* In this case for an anonymous class the compiler will dynamically create an Equals 'utility' method.
* The CecilSymbolHelper will create branch points with a start line of -1 and no document, which
* I am currently not sure how to handle.
*/
if (_branchTarget.StartLine == -1 || _branchTarget.Document == null)
if (branchTarget.StartLine == -1 || branchTarget.Document == null)
continue;

var target = AddInstrumentationCode(method, processor, instruction, _branchTarget);
var target = AddInstrumentationCode(method, processor, instruction, branchTarget);
foreach (var _instruction in processor.Body.Instructions)
ReplaceInstructionTarget(_instruction, instruction, target);

Expand Down Expand Up @@ -467,43 +464,13 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
Ordinal = branchPoint.Ordinal
}
);

if (IsAsyncStateMachineBranch(method.DeclaringType, method))
{
if (_asyncMachineStateMethod == null)
{
_asyncMachineStateMethod = new List<string>();
}

if (!_asyncMachineStateMethod.Contains(method.FullName))
{
_asyncMachineStateMethod.Add(method.FullName);
}
}
}

_result.HitCandidates.Add(new HitCandidate(true, document.Index, branchPoint.StartLine, (int)branchPoint.Ordinal));

return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count - 1);
}

private bool IsAsyncStateMachineBranch(TypeDefinition typeDef, MethodDefinition method)
{
if (!method.FullName.EndsWith("::MoveNext()"))
{
return false;
}

foreach (InterfaceImplementation implementedInterface in typeDef.Interfaces)
{
if (implementedInterface.InterfaceType.FullName == "System.Runtime.CompilerServices.IAsyncStateMachine")
{
return true;
}
}
return false;
}

private Instruction AddInstrumentationInstructions(MethodDefinition method, ILProcessor processor, Instruction instruction, int hitEntryIndex)
{
if (_customTrackerRecordHitMethod == null)
Expand Down
2 changes: 0 additions & 2 deletions src/coverlet.core/Instrumentation/InstrumenterResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ public InstrumenterResult()
[DataMember]
public string Module;
[DataMember]
public string[] AsyncMachineStateMethod;
[DataMember]
public string HitsFilePath;
[DataMember]
public string ModulePath;
Expand Down
2 changes: 2 additions & 0 deletions src/coverlet.core/Symbols/BranchPoint.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
using System;
using System.Diagnostics;
using System.Text.RegularExpressions;

namespace Coverlet.Core.Symbols
{
/// <summary>
/// a branch point
/// </summary>
[DebuggerDisplay("StartLine = {StartLine}")]
public class BranchPoint
{
/// <summary>
Expand Down
57 changes: 54 additions & 3 deletions src/coverlet.core/Symbols/CecilSymbolHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;

using Coverlet.Core.Extensions;
Expand All @@ -18,7 +19,47 @@ namespace Coverlet.Core.Symbols
public static class CecilSymbolHelper
{
private const int StepOverLineCode = 0xFEEFEE;
private static readonly Regex IsMovenext = new Regex(@"\<[^\s>]+\>\w__\w(\w)?::MoveNext\(\)$", RegexOptions.Compiled | RegexOptions.ExplicitCapture);

private static bool IsMoveNextInsideAsyncStateMachine(MethodDefinition methodDefinition)
{
if (!methodDefinition.FullName.EndsWith("::MoveNext()"))
{
return false;
}

if (methodDefinition.DeclaringType.CustomAttributes.Count(ca => ca.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName) > 0)
{
foreach (InterfaceImplementation implementedInterface in methodDefinition.DeclaringType.Interfaces)
{
if (implementedInterface.InterfaceType.FullName == "System.Runtime.CompilerServices.IAsyncStateMachine")
{
return true;
}
}
}

return false;
}

private static bool IsMoveNextInsideEnumerator(MethodDefinition methodDefinition)
{
if (!methodDefinition.FullName.EndsWith("::MoveNext()"))
{
return false;
}
if (methodDefinition.DeclaringType.CustomAttributes.Count(ca => ca.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName) > 0)
{
foreach (InterfaceImplementation implementedInterface in methodDefinition.DeclaringType.Interfaces)
{
if (implementedInterface.InterfaceType.FullName == "System.Collections.IEnumerator")
{
return true;
}
}
}

return false;
}

public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
{
Expand All @@ -30,9 +71,10 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
var instructions = methodDefinition.Body.Instructions;

// if method is a generated MoveNext skip first branch (could be a switch or a branch)
var skipFirstBranch = IsMovenext.IsMatch(methodDefinition.FullName);
bool isAsyncStateMachineMoveNext = IsMoveNextInsideAsyncStateMachine(methodDefinition);
bool skipFirstBranch = isAsyncStateMachineMoveNext || IsMoveNextInsideEnumerator(methodDefinition);

foreach (var instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch))
foreach (Instruction instruction in instructions.Where(instruction => instruction.OpCode.FlowControl == FlowControl.Cond_Branch))
{
try
{
Expand All @@ -42,6 +84,15 @@ public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinitio
continue;
}

// Skip get_IsCompleted to avoid unuseful branch due to async/await state machine
if (isAsyncStateMachineMoveNext && instruction.Previous.Operand is MethodReference operand &&
operand.Name == "get_IsCompleted" &&
operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.TaskAwaiter") &&
operand.DeclaringType.Scope.Name == "System.Runtime")
{
continue;
}

if (BranchIsInGeneratedExceptionFilter(instruction, methodDefinition))
continue;

Expand Down
54 changes: 54 additions & 0 deletions test/coverlet.core.tests/CoverageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,59 @@ public void SelectionStatements_Switch()
File.Delete(path);
}
}

[Fact]
public void AsyncAwait()
{
string path = Path.GetTempFileName();
try
{
RemoteExecutor.Invoke(async pathSerialize =>
{
CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<AsyncAwait>(instance =>
{
instance.SyncExecution();

int res = ((Task<int>)instance.AsyncExecution(true)).ConfigureAwait(false).GetAwaiter().GetResult();
res = ((Task<int>)instance.AsyncExecution(1)).ConfigureAwait(false).GetAwaiter().GetResult();
res = ((Task<int>)instance.AsyncExecution(2)).ConfigureAwait(false).GetAwaiter().GetResult();
res = ((Task<int>)instance.AsyncExecution(3)).ConfigureAwait(false).GetAwaiter().GetResult();
res = ((Task<int>)instance.ContinuationCalled()).ConfigureAwait(false).GetAwaiter().GetResult();

return Task.CompletedTask;
}, pathSerialize);
return 0;
}, path).Dispose();

CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);
result.Document("Instrumentation.AsyncAwait.cs")
.AssertLinesCovered(BuildConfiguration.Debug,
// AsyncExecution(bool)
(10, 1), (11, 1), (12, 1), (14, 1), (16, 1), (17, 0), (18, 0), (19, 0), (21, 1), (22, 1),
// Async
(25, 9), (26, 9), (27, 9), (28, 9),
// SyncExecution
(31, 1), (32, 1), (33, 1),
// Sync
(36, 1), (37, 1), (38, 1),
// AsyncExecution(int)
(41, 3), (42, 3), (43, 3), (46, 1), (47, 1), (48, 1), (51, 1),
(52, 1), (53, 1), (56, 1), (57, 1), (58, 1), (59, 1),
(62, 0), (63, 0), (64, 0), (65, 0), (68, 0), (70, 3), (71, 3),
// ContinuationNotCalled
(74, 0), (75, 0), (76, 0), (77, 0), (78, 0),
// ContinuationCalled -> line 83 should be 1 hit some issue with Continuation state machine
(81, 1), (82, 1), (83, 2), (84, 1), (85, 1)
)
.AssertBranchesCovered(BuildConfiguration.Debug, (16, 0, 0), (16, 1, 1), (43, 0, 3), (43, 1, 1), (43, 2, 1), (43, 3, 1), (43, 4, 0))
// Real branch should be 2, we should try to remove compiler generated branch in method ContinuationNotCalled/ContinuationCalled
// for Continuation state machine
.ExpectedTotalNumberOfBranches(BuildConfiguration.Debug, 4);
}
finally
{
File.Delete(path);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public void CoveragePrepareResult_SerializationRoundTrip()
ir.Module = "Module";
ir.ModulePath = "ModulePath";
ir.SourceLink = "SourceLink";
ir.AsyncMachineStateMethod = new string[] { "A", "B" };

ir.HitCandidates.Add(new HitCandidate(true, 1, 2, 3));
ir.HitCandidates.Add(new HitCandidate(false, 4, 5, 6));
Expand Down Expand Up @@ -110,11 +109,6 @@ public void CoveragePrepareResult_SerializationRoundTrip()
Assert.Equal(cpr.Results[i].ModulePath, roundTrip.Results[i].ModulePath);
Assert.Equal(cpr.Results[i].SourceLink, roundTrip.Results[i].SourceLink);

for (int k = 0; k < cpr.Results[i].AsyncMachineStateMethod.Length; k++)
{
Assert.Equal(cpr.Results[i].AsyncMachineStateMethod[k], roundTrip.Results[i].AsyncMachineStateMethod[k]);
}

for (int k = 0; k < cpr.Results[i].HitCandidates.Count; k++)
{
Assert.Equal(cpr.Results[i].HitCandidates[k].start, roundTrip.Results[i].HitCandidates[k].start);
Expand Down
Loading