From db6bb02db15a46521e5fac4e1273442ea243e352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20M=C3=BCller?= Date: Wed, 15 Dec 2021 00:06:08 +0100 Subject: [PATCH 1/2] transform neg value when integer overflows --- Documentation/Changelog.md | 3 +- src/coverlet.core/Coverage.cs | 7 ++++ .../Coverage/CoverageTests.IntegerOverflow.cs | 38 +++++++++++++++++++ .../Instrumentation.IntegerOverflow.cs | 22 +++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs create mode 100644 test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md index 03194109c..0042bfe00 100644 --- a/Documentation/Changelog.md +++ b/Documentation/Changelog.md @@ -6,7 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -### Fixed +### Fixed +-Fix negative coverage exceeding int.MaxValue [#1266](https://github.com/coverlet-coverage/coverlet/issues/1266) -Fix summary output format for culture de-DE [#1263](https://github.com/coverlet-coverage/coverlet/issues/1263) -Fix branch coverage issue for finally block with await [#1233](https://github.com/coverlet-coverage/coverlet/issues/1233) -Fix threshold doesn't work when coverage empty [#1205](https://github.com/coverlet-coverage/coverlet/issues/1205) diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 3a329dc56..9d1b7aa37 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -420,11 +420,15 @@ private void CalculateCoverage() var hitLocation = result.HitCandidates[i]; var document = documentsList[hitLocation.docIndex]; int hits = br.ReadInt32(); + hits = hits < 0 ? int.MaxValue : hits; if (hitLocation.isBranch) { var branch = document.Branches[new BranchKey(hitLocation.start, hitLocation.end)]; branch.Hits += hits; + + if (branch.Hits < 0) + branch.Hits = int.MaxValue; } else { @@ -437,6 +441,9 @@ private void CalculateCoverage() var line = document.Lines[j]; line.Hits += hits; + + if (line.Hits < 0) + line.Hits = int.MaxValue; } } } diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs b/test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs new file mode 100644 index 000000000..9624c6e97 --- /dev/null +++ b/test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs @@ -0,0 +1,38 @@ +using System.IO; +using System.Threading.Tasks; +using Coverlet.Core.Samples.Tests; +using Xunit; + +namespace Coverlet.Core.Tests +{ + public partial class CoverageTests + { + [Fact] + public void Overflow_Issue_1266() + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.Run(async (string[] pathSerialize) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + instance.Test(); + return Task.CompletedTask; + }, + persistPrepareResultToFile: pathSerialize[0]); + + return 0; + }, new string[] { path }); + + TestInstrumentationHelper.GetCoverageResult(path) + .Document("Instrumentation.IntegerOverflow.cs") + .AssertLinesCovered(BuildConfiguration.Debug, (16, int.MaxValue), (18, int.MaxValue)); + } + finally + { + File.Delete(path); + } + } + } +} diff --git a/test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs b/test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs new file mode 100644 index 000000000..9e80d5a1b --- /dev/null +++ b/test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs @@ -0,0 +1,22 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit.Sdk; + +namespace Coverlet.Core.Samples.Tests +{ + public class IntegerOverflow + { + const long max = (long)int.MaxValue + 1; + + public void Test() + { + for (long i = 0; i < max; i++) + { + _ = 1; + } + } + } +} From 49b0283cf9ef1e59a0e87645a6d226118a140c51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20M=C3=BCller?= Date: Sat, 18 Dec 2021 00:57:03 +0100 Subject: [PATCH 2/2] code review, changed test --- src/coverlet.core/Coverage.cs | 4 + .../Coverage/CoverageTests.IntegerOverflow.cs | 75 +++++++++++++------ .../Instrumentation.IntegerOverflow.cs | 22 ------ 3 files changed, 55 insertions(+), 46 deletions(-) delete mode 100644 test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 9d1b7aa37..30714c0dd 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -420,6 +420,10 @@ private void CalculateCoverage() var hitLocation = result.HitCandidates[i]; var document = documentsList[hitLocation.docIndex]; int hits = br.ReadInt32(); + + if (hits == 0) + continue; + hits = hits < 0 ? int.MaxValue : hits; if (hitLocation.isBranch) diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs b/test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs index 9624c6e97..5b9121f81 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTests.IntegerOverflow.cs @@ -1,6 +1,7 @@ using System.IO; -using System.Threading.Tasks; -using Coverlet.Core.Samples.Tests; +using Coverlet.Core.Abstractions; +using Coverlet.Core.Instrumentation; +using Moq; using Xunit; namespace Coverlet.Core.Tests @@ -8,31 +9,57 @@ namespace Coverlet.Core.Tests public partial class CoverageTests { [Fact] - public void Overflow_Issue_1266() + public void CoverageResult_NegativeLineCoverage_TranslatedToMaxValueOfInt32() { - string path = Path.GetTempFileName(); - try + InstrumenterResult instrumenterResult = new InstrumenterResult { - FunctionExecutor.Run(async (string[] pathSerialize) => - { - CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => - { - instance.Test(); - return Task.CompletedTask; - }, - persistPrepareResultToFile: pathSerialize[0]); - - return 0; - }, new string[] { path }); - - TestInstrumentationHelper.GetCoverageResult(path) - .Document("Instrumentation.IntegerOverflow.cs") - .AssertLinesCovered(BuildConfiguration.Debug, (16, int.MaxValue), (18, int.MaxValue)); - } - finally + HitsFilePath = "HitsFilePath", + SourceLink = "SourceLink", + ModulePath = "ModulePath" + }; + + instrumenterResult.HitCandidates.Add(new HitCandidate(false, 0, 1, 1)); + + var document = new Document + { + Index = 0, + Path = "Path0" + }; + + document.Lines.Add(1, new Line + { + Class = "Class0", + Hits = 0, + Method = "Method0", + Number = 1 + }); + + instrumenterResult.Documents.Add("document", document); + + CoveragePrepareResult coveragePrepareResult = new CoveragePrepareResult { - File.Delete(path); - } + UseSourceLink = true, + Results = new[] {instrumenterResult}, + Parameters = new CoverageParameters() + }; + + Stream memoryStream = new MemoryStream(); + BinaryWriter binaryWriter = new BinaryWriter(memoryStream); + binaryWriter.Write(1); + binaryWriter.Write(-1); + memoryStream.Position = 0; + + var fileSystemMock = new Mock(); + fileSystemMock.Setup(x => x.Exists(It.IsAny())).Returns(true); + fileSystemMock.Setup(x => x.NewFileStream(It.IsAny(), FileMode.Open, FileAccess.Read)) + .Returns(memoryStream); + + var coverage = new Coverage(coveragePrepareResult, new Mock().Object, new Mock().Object, + fileSystemMock.Object, new Mock().Object); + + var coverageResult = coverage.GetCoverageResult(); + coverageResult.Document("document").AssertLinesCovered(BuildConfiguration.Debug, (1, int.MaxValue)); + } } } diff --git a/test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs b/test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs deleted file mode 100644 index 9e80d5a1b..000000000 --- a/test/coverlet.core.tests/Samples/Instrumentation.IntegerOverflow.cs +++ /dev/null @@ -1,22 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using Xunit.Sdk; - -namespace Coverlet.Core.Samples.Tests -{ - public class IntegerOverflow - { - const long max = (long)int.MaxValue + 1; - - public void Test() - { - for (long i = 0; i < max; i++) - { - _ = 1; - } - } - } -}