Missing exclusion for delegate cache branch in release builds

[ulrichb]

In 98bd0f01e2c0d236092e1fb932d50f7121cdab55 (feature request #302) support for ignoring delegate cache branches has been added.

The detection seems to miss the situation, when using a static call in a lambda in release (optimized) builds. Tested in VS 2015/Roslyn 1.2.

See the following example.

[TestFixture]
public class ActionLambda
{
    private void M(string p) { }

    [Test]
    public void InstanceCallInsideAction()
    {
        Action a = () => M("");

        a();
    }

    [Test]
    public void StaticCallInsideAction()
    {
        // Here the detection misses the delegate cache branch in an optimized build:
        Action a = () => StaticClass.M("");

        a();
    }
}

public static class StaticClass
{
    public static void M(string p) { }
}

Debug compilation:

Release compilation:

[sawilde]

What does the IL look like for release/debug builds?

[ulrichb]

A side-by-side diff of the StaticCallInsideAction() method (left=Debug, right=Release):

[sawilde]

Interesting - we can investigate if we can exclude without excluding legitimate cases probably through some code analysis similar to what we have done before

@ddur - what do you think?

[ulrichb]

At the moment, delegate cache branches are ignored by the pattern of the generated IL (see CecilSymbolManager.

Delegate cache fields always start with <>9 (see MakeLambdaCacheFieldName(), LambdaCacheField = '9', and MakeMethodScopedSynthesizedName() in the Roslyn code).

So maybe an alternative to detect the to be ignored brtrue.s branch instruction would be to search for the pattern ldsfld F; dup; brtrue.s LABEL where F is a field reference to a field starting with <>9 (which would never be user code).

[ddur]

Difference is because dll in release build has no sequence-points, therefore source code is not available. Without access to source code, removal of excess branches has limited capabilities.

[ulrichb]

@ddur Isn't the detection of the delegate cache braches based on the IL-instruction pattern (and not on symbols)? Without symbols (removed .pdb), I get no coverage result at all.

[ddur]

@ulrichb Then I do not know. You can ignore my previous post.

[sawilde]

@ulrichb - opencover uses the PDB files to determine the sequence points and it uses the IL to determine the branch points. On some occasions to determine whether to cover or not to cover certain branches, we look at files and we may also look at metadata but the more analysis we do at runtime this will slow the process down so we have to make trade-offs between speed and usefulness.

ALso the PDB also allows us to identify which sequence point relates to which file and line - we (well I) decided no PDB, no coverage required.