-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[Coverage] Fix region termination for GNU statement expressions #130976
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-pgo Author: Justin Cady (justincady) ChangesCalls 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/130976.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..d03e35630b317 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-NOT: Gap,File 0, [[@LINE]]:41 -> [[@LINE+1]]:3 = 0
+ return rc;
+}
+
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..5894275b941a2
--- /dev/null
+++ b/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
@@ -0,0 +1,19 @@
+// 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>
+
+__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|}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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 #124296
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/13131 Here is the relevant piece of the build log for the reference |
…t expressions" (#132095) Reverts llvm/llvm-project#130976 Breaks clang-cmake-x86_64-avx512-linux bot.
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
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
#132222) 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][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 #124296
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