Reland [Coverage] Fix region termination for GNU statement expressions #132222
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-pgo Author: Justin Cady (justincady) ChangesRelands #130976 with adjustments to test requirements. Calls to noreturn functions result in region termination for In this scenario an extra gap region is introduced within VisitStmt, This change adjusts the mapping such that terminate statements Fixes #124296 Full diff: https://github.com/llvm/llvm-project/pull/132222.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index f09157771d2b5..73811d15979d5 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1505,6 +1505,14 @@ struct CounterCoverageMappingBuilder
handleFileExit(getEnd(S));
}
+ void VisitStmtExpr(const StmtExpr *E) {
+ Visit(E->getSubStmt());
+ // Any region termination (such as a noreturn CallExpr) within the statement
+ // expression has been handled by visiting the sub-statement. The visitor
+ // cannot be at a terminate statement leaving the statement expression.
+ HasTerminateStmt = false;
+ }
+
void VisitDecl(const Decl *D) {
Stmt *Body = D->getBody();
diff --git a/clang/test/CoverageMapping/terminate-statements.cpp b/clang/test/CoverageMapping/terminate-statements.cpp
index 0067185fee8e6..3f8e43f0fbcb6 100644
--- a/clang/test/CoverageMapping/terminate-statements.cpp
+++ b/clang/test/CoverageMapping/terminate-statements.cpp
@@ -346,6 +346,12 @@ int elsecondnoret(void) {
return 0;
}
+// CHECK-LABEL: _Z18statementexprnoretb
+int statementexprnoret(bool crash) {
+ int rc = ({ if (crash) abort(); 0; }); // CHECK: File 0, 351:35 -> 352:12 = (#0 - #1)
+ return rc; // CHECK-NOT: Gap
+}
+
int main() {
foo(0);
foo(1);
@@ -368,5 +374,6 @@ int main() {
ornoret();
abstractcondnoret();
elsecondnoret();
+ statementexprnoret(false);
return 0;
}
diff --git a/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp b/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
new file mode 100644
index 0000000000000..19300411d54a2
--- /dev/null
+++ b/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: lld-available
+// XFAIL: powerpc64-target-arch
+
+// RUN: %clangxx_profgen -std=gnu++17 -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
+
+#include <stdio.h>
+
+// clang-format off
+__attribute__ ((__noreturn__))
+void foo(void) { while (1); } // CHECK: [[@LINE]]| 0|void foo(void)
+_Noreturn void bar(void) { while (1); } // CHECK: [[@LINE]]| 0|_Noreturn void bar(void)
+ // CHECK: [[@LINE]]| |
+int main(int argc, char **argv) { // CHECK: [[@LINE]]| 1|int main(
+ int rc = ({ if (argc > 3) foo(); 0; }); // CHECK: [[@LINE]]| 1| int rc =
+ printf("coverage after foo is present\n"); // CHECK: [[@LINE]]| 1| printf(
+ // CHECK: [[@LINE]]| |
+ int rc2 = ({ if (argc > 3) bar(); 0; }); // CHECK: [[@LINE]]| 1| int rc2 =
+ printf("coverage after bar is present\n"); // CHECK: [[@LINE]]| 1| printf(
+ return rc + rc2; // CHECK: [[@LINE]]| 1| return rc
+} // CHECK: [[@LINE]]| 1|}
+// clang-format on
|
Member
|
@llvm/pr-subscribers-clang Author: Justin Cady (justincady) ChangesRelands #130976 with adjustments to test requirements. Calls to noreturn functions result in region termination for In this scenario an extra gap region is introduced within VisitStmt, This change adjusts the mapping such that terminate statements Fixes #124296 Full diff: https://github.com/llvm/llvm-project/pull/132222.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index f09157771d2b5..73811d15979d5 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1505,6 +1505,14 @@ struct CounterCoverageMappingBuilder
handleFileExit(getEnd(S));
}
+ void VisitStmtExpr(const StmtExpr *E) {
+ Visit(E->getSubStmt());
+ // Any region termination (such as a noreturn CallExpr) within the statement
+ // expression has been handled by visiting the sub-statement. The visitor
+ // cannot be at a terminate statement leaving the statement expression.
+ HasTerminateStmt = false;
+ }
+
void VisitDecl(const Decl *D) {
Stmt *Body = D->getBody();
diff --git a/clang/test/CoverageMapping/terminate-statements.cpp b/clang/test/CoverageMapping/terminate-statements.cpp
index 0067185fee8e6..3f8e43f0fbcb6 100644
--- a/clang/test/CoverageMapping/terminate-statements.cpp
+++ b/clang/test/CoverageMapping/terminate-statements.cpp
@@ -346,6 +346,12 @@ int elsecondnoret(void) {
return 0;
}
+// CHECK-LABEL: _Z18statementexprnoretb
+int statementexprnoret(bool crash) {
+ int rc = ({ if (crash) abort(); 0; }); // CHECK: File 0, 351:35 -> 352:12 = (#0 - #1)
+ return rc; // CHECK-NOT: Gap
+}
+
int main() {
foo(0);
foo(1);
@@ -368,5 +374,6 @@ int main() {
ornoret();
abstractcondnoret();
elsecondnoret();
+ statementexprnoret(false);
return 0;
}
diff --git a/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp b/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
new file mode 100644
index 0000000000000..19300411d54a2
--- /dev/null
+++ b/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: lld-available
+// XFAIL: powerpc64-target-arch
+
+// RUN: %clangxx_profgen -std=gnu++17 -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
+
+#include <stdio.h>
+
+// clang-format off
+__attribute__ ((__noreturn__))
+void foo(void) { while (1); } // CHECK: [[@LINE]]| 0|void foo(void)
+_Noreturn void bar(void) { while (1); } // CHECK: [[@LINE]]| 0|_Noreturn void bar(void)
+ // CHECK: [[@LINE]]| |
+int main(int argc, char **argv) { // CHECK: [[@LINE]]| 1|int main(
+ int rc = ({ if (argc > 3) foo(); 0; }); // CHECK: [[@LINE]]| 1| int rc =
+ printf("coverage after foo is present\n"); // CHECK: [[@LINE]]| 1| printf(
+ // CHECK: [[@LINE]]| |
+ int rc2 = ({ if (argc > 3) bar(); 0; }); // CHECK: [[@LINE]]| 1| int rc2 =
+ printf("coverage after bar is present\n"); // CHECK: [[@LINE]]| 1| printf(
+ return rc + rc2; // CHECK: [[@LINE]]| 1| return rc
+} // CHECK: [[@LINE]]| 1|}
+// clang-format on
|
Contributor
Author
|
This is the buildbot failure from the initial commit: https://lab.llvm.org/buildbot/#/builders/133/builds/13131 I updated the failing test with the same restrictions as other tests using |
MaskRay
approved these changes
Mar 21, 2025
Relands llvm#130976 with adjustments to test requirements. Calls to __noreturn__ functions result in region termination for coverage mapping. But this creates incorrect coverage results when __noreturn__ functions (or other constructs that result in region termination) occur within [GNU statement expressions][1]. In this scenario an extra gap region is introduced within VisitStmt, such that if the following line does not introduce a new region it is unconditionally counted as uncovered. This change adjusts the mapping such that terminate statements within statement expressions do not propagate that termination state after the statement expression is processed. [1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html Fixes llvm#124296
justincady
force-pushed
the
stmt-expr-cov
branch
from
e282d30 to
fa65258
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relands #130976 with adjustments to test requirements.
Calls to noreturn functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
noreturn functions (or other constructs that result in region
termination) occur within GNU statement expressions.
In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.
This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.
Fixes #124296