Negative sequence point coverage when line executions exceed int.MaxValue

[Malivil]

Package: coverlet.msbuild
Version: 3.1.0
Format: opencover

In a project where a particular line of code gets called a lot, the sequence point coverage value for that line can become negative if the line is hit more than int.MaxValue times. The value in the XML is negative and shows in the generated report as uncovered.

I created a simple repro of this because I cannot show the actual code that caused the problem (it's from my job).

XML:

Report:

Repro code: https://github.com/Malivil/Coverlet1266Repro

[daveMueller]

Thanks for reporting and thanks for the repro. 🙏

[daveMueller]

Mhh, we could change the tracker to stop tracking hits when int.MaxValue is reached. Something like

public static void RecordHit(int hitLocationIndex)
{
    if(HitsArray[hitLocationIndex] < int.MaxValue)
        Interlocked.Increment(ref HitsArray[hitLocationIndex]);
}

I just tested it with the repro provided by @Malivil and it would work.

What do you think, @MarcoRossignoli @petli @tonerdo? Any other ideas for solutions?

[MarcoRossignoli]

I prefer to avoid a new branch on the tracker...we're injecting it to probe and we'll slow down more tests, maybe we can try to move to uint32 https://docs.microsoft.com/en-us/dotnet/api/system.threading.interlocked.increment?view=net-6.0#System_Threading_Interlocked_Increment_System_UInt32__ and let it overflow but always >= 0
We need to understand during report generation what's happen if we put a uint istead of int for instance for cobertura+report generator.

[petli]

An alternative is to translate negative numbers to int.MaxValue when collecting the results.

[MarcoRossignoli]

An alternative is to translate negative numbers to int.MaxValue when collecting the results.

Yep, if we're ok with int.MaxValue, @Malivil from your perspective does it make sense have "correct very huge value" there? I mean are you paying attention to that?

[petli]

I think that if you hit 31 bits you may be likely to hit 32 bits too, and then the only real solution would be to go to long, doubling the memory usage. (But maybe not slow down execution, on 64-bit machines?)

[Malivil]

An alternative is to translate negative numbers to int.MaxValue when collecting the results.

Yep, if we're ok with int.MaxValue, @Malivil from your perspective does it make sense have "correct very huge value" there? I mean are you paying attention to that?

We're not paying attention to the number, specifically. As long as it's positive and it makes sense with the surrounding lines we wouldn't even notice. Keeping it at int.MaxValue would work fine from our perspective.

[daveMueller]

An alternative is to translate negative numbers to int.MaxValue when collecting the results.

Then should we go with this solution? I could prepare a PR in the next days.

[MarcoRossignoli]

@daveMueller looks like we're all in agreement for the translation, feel free to tackle it if you want! Thanks a lot!