From feb9f8d888802152e7c557592f68e66a29787ac9 Mon Sep 17 00:00:00 2001 From: Justin Cady Date: Thu, 12 Jun 2025 10:16:59 -0400 Subject: [PATCH] [llvm-cov] Improve line count for wrapped segments Adjust line coverage calculation to report the count of the wrapped segment only if there are no regions starting on that line. This resolves the nested macro scenario reported in #140893. Because the inner macro creates a new region, the counter for the outer macro is not considered. As it stands the diff passes all existing tests, but the modifications required indicate a broad reporting impact. I suspect when most users absorbed this change their coverage numbers would decrease, though I believe those numbers would be more accurate. This is a significant change in reporting. I am not certain it is the right approach, and thus I am posting it as a draft for consideration. More testing is likely required to iron out other inconsistencies that may be accidentally introduced. It's possible there are other, more desirable approaches to resolve #140893 (and other similar issues noted in that bug). Fixes #140893 --- .../test/profile/Linux/coverage-exception.cpp | 12 ++--- .../profile/Linux/coverage-nested-macro.c | 49 +++++++++++++++++++ .../ProfileData/Coverage/CoverageMapping.cpp | 10 ++-- .../tools/llvm-cov/Inputs/instrprof-comdat.h | 2 +- .../test/tools/llvm-cov/branch-c-general.test | 12 ++--- .../tools/llvm-cov/branch-noShowBranch.test | 2 +- .../tools/llvm-cov/ignore-filename-regex.test | 4 +- .../ProfileData/CoverageMappingTest.cpp | 2 +- 8 files changed, 72 insertions(+), 21 deletions(-) create mode 100755 compiler-rt/test/profile/Linux/coverage-nested-macro.c diff --git a/compiler-rt/test/profile/Linux/coverage-exception.cpp b/compiler-rt/test/profile/Linux/coverage-exception.cpp index 3f0c907764269..c21fd61848908 100755 --- a/compiler-rt/test/profile/Linux/coverage-exception.cpp +++ b/compiler-rt/test/profile/Linux/coverage-exception.cpp @@ -43,7 +43,7 @@ static int test_no_exception() { // CHECK: [[@LINE]]| 1|int test_no_exception() try { // CHECK: [[@LINE]]| 1| try { do_throw(false); // CHECK: [[@LINE]]| 1| do_throw( - } catch (...) { // CHECK: [[@LINE]]| 1| } catch ( + } catch (...) { // CHECK: [[@LINE]]| 0| } catch ( abort(); // CHECK: [[@LINE]]| 0| abort( } // CHECK: [[@LINE]]| 0| } printf("%s\n", __func__); // CHECK: [[@LINE]]| 1| printf( @@ -78,7 +78,7 @@ static int test_exception_macro_nested() { // CHECK: [[@LINE]]| 1|int test_exception_macro_nested() try { // CHECK: [[@LINE]]| 1| try { TRY_AND_CATCH_ALL(do_throw(true)); // CHECK: [[@LINE]]| 1| TRY_AND_CATCH_ALL( - } catch (...) { // CHECK: [[@LINE]]| 1| } catch ( + } catch (...) { // CHECK: [[@LINE]]| 0| } catch ( abort(); // CHECK: [[@LINE]]| 0| abort( } // CHECK: [[@LINE]]| 0| } printf("%s\n", __func__); // CHECK: [[@LINE]]| 1| printf( @@ -104,10 +104,10 @@ int test_conditional(int i) { // CHECK: [[@LINE]]| 1|int test_conditi try { // CHECK: [[@LINE]]| 1| try { if (i % 2 == 0) { // CHECK: [[@LINE]]| 1| if ( printf("%s\n", __func__); // CHECK: [[@LINE]]| 1| printf( - } else { // CHECK: [[@LINE]]| 1| } else { + } else { // CHECK: [[@LINE]]| 0| } else { do_throw(true); // CHECK: [[@LINE]]| 0| do_throw( } // CHECK: [[@LINE]]| 0| } - } catch (...) { // CHECK: [[@LINE]]| 1| } catch ( + } catch (...) { // CHECK: [[@LINE]]| 0| } catch ( abort(); // CHECK: [[@LINE]]| 0| abort( } // CHECK: [[@LINE]]| 0| } return 0; // CHECK: [[@LINE]]| 1| return @@ -117,11 +117,11 @@ static int test_multiple_catch() { // CHECK: [[@LINE]]| 1|int test_multiple_catch() try { // CHECK: [[@LINE]]| 1| try { do_throw(true); // CHECK: [[@LINE]]| 1| do_throw( - } catch (double) { // CHECK: [[@LINE]]| 1| } catch (double) + } catch (double) { // CHECK: [[@LINE]]| 0| } catch (double) abort(); // CHECK: [[@LINE]]| 0| abort( } catch (bool) { // CHECK: [[@LINE]]| 1| } catch (bool) printf("bool\n"); // CHECK: [[@LINE]]| 1| printf( - } catch (float) { // CHECK: [[@LINE]]| 1| } catch (float) + } catch (float) { // CHECK: [[@LINE]]| 0| } catch (float) abort(); // CHECK: [[@LINE]]| 0| abort( } catch (...) { // CHECK: [[@LINE]]| 0| } catch ( abort(); // CHECK: [[@LINE]]| 0| abort( diff --git a/compiler-rt/test/profile/Linux/coverage-nested-macro.c b/compiler-rt/test/profile/Linux/coverage-nested-macro.c new file mode 100755 index 0000000000000..e4d38a92faaa3 --- /dev/null +++ b/compiler-rt/test/profile/Linux/coverage-nested-macro.c @@ -0,0 +1,49 @@ +// REQUIRES: lld-available +// XFAIL: powerpc64-target-arch + +// RUN: %clang_profgen -fuse-ld=lld -fcoverage-mapping -o %t %s +// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t +// RUN: llvm-profdata merge -o %t.profdata %t.profraw +// RUN: llvm-cov show %t -instr-profile=%t.profdata 2>&1 | FileCheck %s --match-full-lines + +// CHECK: 32| 0|#define MY_ID(x) ((x) ? 1 : 2) +// CHECK: 33| | +// CHECK: 34| |#define MY_LOG(fmt, ...) \ +// CHECK: 35| 1| { \ +// CHECK: 36| 1| if (enabled) { \ +// CHECK: 37| 0| printf(fmt, ## __VA_ARGS__); \ +// CHECK: 38| 0| } \ +// CHECK: 39| 1| } +// CHECK: 40| | +// CHECK: 41| 1|int main(int argc, char *argv[]) { +// CHECK: 42| 1| enabled = argc > 2; +// CHECK: 43| 1| MY_LOG("%d, %s, %d\n", +// CHECK: 44| 0| MY_ID(argc > 3), +// CHECK: 45| 0| "a", +// CHECK: 46| 0| 1); +// CHECK: 47| 1| return 0; +// CHECK: 48| 1|} + +#include + +static int enabled = 0; + +// clang-format off +#define MY_ID(x) ((x) ? 1 : 2) + +#define MY_LOG(fmt, ...) \ + { \ + if (enabled) { \ + printf(fmt, ## __VA_ARGS__); \ + } \ + } + +int main(int argc, char *argv[]) { + enabled = argc > 2; + MY_LOG("%d, %s, %d\n", + MY_ID(argc > 3), + "a", + 1); + return 0; +} +// clang-format on diff --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp index dd74eb054a34c..b5ed6e15c058f 100644 --- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp +++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp @@ -1567,11 +1567,13 @@ LineCoverageStats::LineCoverageStats( } // Pick the max count from the non-gap, region entry segments and the - // wrapped count. - if (WrappedSegment) - ExecutionCount = WrappedSegment->Count; - if (!MinRegionCount) + // wrapped count. But, only consider the wrapped count if no regions start + // on this line. + if (!MinRegionCount) { + if (WrappedSegment) + ExecutionCount = WrappedSegment->Count; return; + } for (const auto *LS : LineSegments) if (isStartOfRegion(LS)) ExecutionCount = std::max(ExecutionCount, LS->Count); diff --git a/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h b/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h index 07941f9bb497a..d224fd0d00ea0 100644 --- a/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h +++ b/llvm/test/tools/llvm-cov/Inputs/instrprof-comdat.h @@ -12,7 +12,7 @@ template T FOO::DoIt(T ti) { // HEADER: [[@LINE]]| 2|template for (T I = 0; I < ti; I++) { // HEADER: [[@LINE]]| 22| for (T t += I; // HEADER: [[@LINE]]| 20| t += I; if (I > ti / 2) // HEADER: [[@LINE]]| 20| if (I > ti - t -= 1; // HEADER: [[@LINE]]| 20| t -= 1; + t -= 1; // HEADER: [[@LINE]]| 8| t -= 1; } // HEADER: [[@LINE]]| 20| } // HEADER: [[@LINE]]| 2| return t; // HEADER: [[@LINE]]| 2| return t; diff --git a/llvm/test/tools/llvm-cov/branch-c-general.test b/llvm/test/tools/llvm-cov/branch-c-general.test index 3c163bf6de45c..85c541f4158ab 100644 --- a/llvm/test/tools/llvm-cov/branch-c-general.test +++ b/llvm/test/tools/llvm-cov/branch-c-general.test @@ -118,18 +118,18 @@ // REPORT-NEXT: --- // REPORT-NEXT: simple_loops 8 0 100.00% 9 0 100.00% 6 0 100.00% // REPORT-NEXT: conditionals 24 0 100.00% 15 0 100.00% 16 2 87.50% -// REPORT-NEXT: early_exits 20 4 80.00% 25 2 92.00% 16 6 62.50% -// REPORT-NEXT: jumps 39 12 69.23% 48 2 95.83% 26 9 65.38% -// REPORT-NEXT: switches 28 5 82.14% 38 4 89.47% 28 7 75.00% +// REPORT-NEXT: early_exits 20 4 80.00% 25 3 88.00% 16 6 62.50% +// REPORT-NEXT: jumps 39 12 69.23% 48 4 91.67% 26 9 65.38% +// REPORT-NEXT: switches 28 5 82.14% 38 5 86.84% 28 7 75.00% // REPORT-NEXT: big_switch 25 1 96.00% 32 0 100.00% 30 6 80.00% // REPORT-NEXT: boolean_operators 16 0 100.00% 13 0 100.00% 22 2 90.91% // REPORT-NEXT: boolop_loops 19 0 100.00% 14 0 100.00% 16 2 87.50% -// REPORT-NEXT: conditional_operator 4 2 50.00% 8 0 100.00% 4 2 50.00% +// REPORT-NEXT: conditional_operator 4 2 50.00% 8 1 87.50% 4 2 50.00% // REPORT-NEXT: do_fallthrough 9 0 100.00% 12 0 100.00% 6 0 100.00% // REPORT-NEXT: main 1 0 100.00% 16 0 100.00% 0 0 0.00% // REPORT-NEXT: c-general.c:static_func 4 0 100.00% 4 0 100.00% 2 0 100.00% // REPORT-NEXT: --- -// REPORT-NEXT: TOTAL 197 24 87.82% 234 8 96.58% 172 36 79.07% +// REPORT-NEXT: TOTAL 197 24 87.82% 234 13 94.44% 172 36 79.07% // Test file-level report. // RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata @@ -157,7 +157,7 @@ // HTML-INDEX: // HTML-INDEX: 100.00% (12/12) // HTML-INDEX: -// HTML-INDEX: 96.58% (226/234) +// HTML-INDEX: 94.44% (221/234) // HTML-INDEX: // HTML-INDEX: 87.82% (173/197) // HTML-INDEX: diff --git a/llvm/test/tools/llvm-cov/branch-noShowBranch.test b/llvm/test/tools/llvm-cov/branch-noShowBranch.test index 9f3cfd55f029b..133af743f50b6 100644 --- a/llvm/test/tools/llvm-cov/branch-noShowBranch.test +++ b/llvm/test/tools/llvm-cov/branch-noShowBranch.test @@ -20,5 +20,5 @@ // REPORT-NOT: do_fallthrough 9 0 100.00% 12 0 100.00% 6 0 100.00% // REPORT-NOT: main 1 0 100.00% 16 0 100.00% 0 0 0.00% // REPORT-NOT: c-general.c:static_func 4 0 100.00% 4 0 100.00% 2 0 100.00% -// REPORT: TOTAL 197 24 87.82% 234 8 96.58% +// REPORT: TOTAL 197 24 87.82% 234 13 94.44% // REPORT-NOT: TOTAL 197 24 87.82% 234 8 96.58% 172 36 79.07% diff --git a/llvm/test/tools/llvm-cov/ignore-filename-regex.test b/llvm/test/tools/llvm-cov/ignore-filename-regex.test index aea9e4646776d..efc4cda4abc04 100644 --- a/llvm/test/tools/llvm-cov/ignore-filename-regex.test +++ b/llvm/test/tools/llvm-cov/ignore-filename-regex.test @@ -22,7 +22,7 @@ REPORT_IGNORE_DIR-NOT: {{.*}}extra{{[/\\]}}dec.h{{.*}} REPORT_IGNORE_DIR-NOT: {{.*}}extra{{[/\\]}}inc.h{{.*}} REPORT_IGNORE_DIR: {{.*}}abs.h{{.*}} REPORT_IGNORE_DIR: {{.*}}main.cc{{.*}} -REPORT_IGNORE_DIR: {{^}}TOTAL 5{{.*}}100.00%{{$}} +REPORT_IGNORE_DIR: {{^}}TOTAL 5{{.*}}90.00%{{$}} # Ignore all files from "extra" directory even when SOURCES specified. RUN: llvm-cov report -instr-profile %S/Inputs/sources_specified/main.profdata \ @@ -35,7 +35,7 @@ REPORT_IGNORE_DIR_WITH_SOURCES-NOT: {{.*}}extra{{[/\\]}}dec.h{{.*}} REPORT_IGNORE_DIR_WITH_SOURCES-NOT: {{.*}}extra{{[/\\]}}inc.h{{.*}} REPORT_IGNORE_DIR_WITH_SOURCES-NOT: {{.*}}main.cc{{.*}} REPORT_IGNORE_DIR_WITH_SOURCES: {{.*}}abs.h{{.*}} -REPORT_IGNORE_DIR_WITH_SOURCES: {{^}}TOTAL 4{{.*}}100.00%{{$}} +REPORT_IGNORE_DIR_WITH_SOURCES: {{^}}TOTAL 4{{.*}}80.00%{{$}} ######################## # Test "show" command. diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp index 46f881ecddb5f..70196da9ff307 100644 --- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp +++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp @@ -754,7 +754,7 @@ TEST_P(CoverageMappingTest, test_line_coverage_iterator) { CoverageData Data = LoadedCoverage->getCoverageForFile("file1"); unsigned Line = 0; - const unsigned LineCounts[] = {20, 20, 20, 0, 30, 10, 10, 10, 10, 0, 0, 0, 0}; + const unsigned LineCounts[] = {20, 20, 20, 0, 10, 10, 10, 10, 10, 0, 0, 0, 0}; const bool MappedLines[] = {1, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 0, 0}; ASSERT_EQ(std::size(LineCounts), std::size(MappedLines));