[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
whentojump wrote: > Hi @whentojump (or anyone else). If you would like to add a note about this > fix in the release notes (completely optional). Please reply to this comment > with a one or two sentence description of the fix. When you are done, please > add the release:note label to this PR. Fixes a Clang assertion failure caused by emitting gap coverage mapping regions between statements with . https://github.com/llvm/llvm-project/pull/90369 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
tstellar wrote: Hi @whentojump (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. https://github.com/llvm/llvm-project/pull/90369 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
https://github.com/tstellar closed https://github.com/llvm/llvm-project/pull/90369 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
https://github.com/tstellar updated
https://github.com/llvm/llvm-project/pull/90369
>From aea091b70edaf5b53bdd37f5ee6351c1642b07cc Mon Sep 17 00:00:00 2001
From: Wentao Zhang <35722712+whentoj...@users.noreply.github.com>
Date: Mon, 22 Apr 2024 12:37:38 -0500
Subject: [PATCH] [clang][CoverageMapping] do not emit a gap region when either
end doesn't have valid source locations (#89564)
Fixes #86998
(cherry picked from commit c1b6cca1214e7a9c14a30b81585dd8b81baeaa77)
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 11 --
.../CoverageMapping/statement-expression.c| 36 +++
2 files changed, 44 insertions(+), 3 deletions(-)
create mode 100644 clang/test/CoverageMapping/statement-expression.c
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..ae4e6d4c88c02d 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1207,6 +1207,12 @@ struct CounterCoverageMappingBuilder
/// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
std::optional findGapAreaBetween(SourceLocation AfterLoc,
SourceLocation BeforeLoc) {
+// Some statements (like AttributedStmt and ImplicitValueInitExpr) don't
+// have valid source locations. Do not emit a gap region if this is the
case
+// in either AfterLoc end or BeforeLoc end.
+if (AfterLoc.isInvalid() || BeforeLoc.isInvalid())
+ return std::nullopt;
+
// If AfterLoc is in function-like macro, use the right parenthesis
// location.
if (AfterLoc.isMacroID()) {
@@ -1370,9 +1376,8 @@ struct CounterCoverageMappingBuilder
for (const Stmt *Child : S->children())
if (Child) {
// If last statement contains terminate statements, add a gap area
-// between the two statements. Skipping attributed statements, because
-// they don't have valid start location.
-if (LastStmt && HasTerminateStmt && !isa(Child)) {
+// between the two statements.
+if (LastStmt && HasTerminateStmt) {
auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),
diff --git a/clang/test/CoverageMapping/statement-expression.c
b/clang/test/CoverageMapping/statement-expression.c
new file mode 100644
index 00..5f9ab5838af342
--- /dev/null
+++ b/clang/test/CoverageMapping/statement-expression.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-emit-llvm-only -main-file-name statement-expression.c %s
+
+// No crash for the following examples, where GNU Statement Expression
extension
+// could introduce region terminators (break, goto etc) before implicit
+// initializers in a struct or an array.
+// See https://github.com/llvm/llvm-project/pull/89564
+
+struct Foo {
+ int field1;
+ int field2;
+};
+
+void f1(void) {
+ struct Foo foo = {
+.field1 = ({
+ switch (0) {
+ case 0:
+break; // A region terminator
+ }
+ 0;
+}),
+// ImplicitValueInitExpr introduced here for .field2
+ };
+}
+
+void f2(void) {
+ int arr[3] = {
+[0] = ({
+goto L0; // A region terminator
+L0:
+ 0;
+}),
+// ImplicitValueInitExpr introduced here for subscript [1]
+[2] = 0,
+ };
+}
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/90369 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: None (llvmbot)
Changes
Backport c1b6cca1214e7a9c14a30b81585dd8b81baeaa77
Requested by: @whentojump
---
Full diff: https://github.com/llvm/llvm-project/pull/90369.diff
2 Files Affected:
- (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+8-3)
- (added) clang/test/CoverageMapping/statement-expression.c (+36)
``diff
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..ae4e6d4c88c02d 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1207,6 +1207,12 @@ struct CounterCoverageMappingBuilder
/// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
std::optional findGapAreaBetween(SourceLocation AfterLoc,
SourceLocation BeforeLoc) {
+// Some statements (like AttributedStmt and ImplicitValueInitExpr) don't
+// have valid source locations. Do not emit a gap region if this is the
case
+// in either AfterLoc end or BeforeLoc end.
+if (AfterLoc.isInvalid() || BeforeLoc.isInvalid())
+ return std::nullopt;
+
// If AfterLoc is in function-like macro, use the right parenthesis
// location.
if (AfterLoc.isMacroID()) {
@@ -1370,9 +1376,8 @@ struct CounterCoverageMappingBuilder
for (const Stmt *Child : S->children())
if (Child) {
// If last statement contains terminate statements, add a gap area
-// between the two statements. Skipping attributed statements, because
-// they don't have valid start location.
-if (LastStmt && HasTerminateStmt && !isa(Child)) {
+// between the two statements.
+if (LastStmt && HasTerminateStmt) {
auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),
diff --git a/clang/test/CoverageMapping/statement-expression.c
b/clang/test/CoverageMapping/statement-expression.c
new file mode 100644
index 00..5f9ab5838af342
--- /dev/null
+++ b/clang/test/CoverageMapping/statement-expression.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-emit-llvm-only -main-file-name statement-expression.c %s
+
+// No crash for the following examples, where GNU Statement Expression
extension
+// could introduce region terminators (break, goto etc) before implicit
+// initializers in a struct or an array.
+// See https://github.com/llvm/llvm-project/pull/89564
+
+struct Foo {
+ int field1;
+ int field2;
+};
+
+void f1(void) {
+ struct Foo foo = {
+.field1 = ({
+ switch (0) {
+ case 0:
+break; // A region terminator
+ }
+ 0;
+}),
+// ImplicitValueInitExpr introduced here for .field2
+ };
+}
+
+void f2(void) {
+ int arr[3] = {
+[0] = ({
+goto L0; // A region terminator
+L0:
+ 0;
+}),
+// ImplicitValueInitExpr introduced here for subscript [1]
+[2] = 0,
+ };
+}
``
https://github.com/llvm/llvm-project/pull/90369
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
llvmbot wrote: @ZequanWu What do you think about merging this PR to the release branch? https://github.com/llvm/llvm-project/pull/90369 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
https://github.com/llvmbot milestoned https://github.com/llvm/llvm-project/pull/90369 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) (PR #90369)
https://github.com/llvmbot created
https://github.com/llvm/llvm-project/pull/90369
Backport c1b6cca1214e7a9c14a30b81585dd8b81baeaa77
Requested by: @whentojump
>From 71dd393acd7948b8290e65d082c509fd2df4c02f Mon Sep 17 00:00:00 2001
From: Wentao Zhang <35722712+whentoj...@users.noreply.github.com>
Date: Mon, 22 Apr 2024 12:37:38 -0500
Subject: [PATCH] [clang][CoverageMapping] do not emit a gap region when either
end doesn't have valid source locations (#89564)
Fixes #86998
(cherry picked from commit c1b6cca1214e7a9c14a30b81585dd8b81baeaa77)
---
clang/lib/CodeGen/CoverageMappingGen.cpp | 11 --
.../CoverageMapping/statement-expression.c| 36 +++
2 files changed, 44 insertions(+), 3 deletions(-)
create mode 100644 clang/test/CoverageMapping/statement-expression.c
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0c43317642bca4..ae4e6d4c88c02d 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1207,6 +1207,12 @@ struct CounterCoverageMappingBuilder
/// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
std::optional findGapAreaBetween(SourceLocation AfterLoc,
SourceLocation BeforeLoc) {
+// Some statements (like AttributedStmt and ImplicitValueInitExpr) don't
+// have valid source locations. Do not emit a gap region if this is the
case
+// in either AfterLoc end or BeforeLoc end.
+if (AfterLoc.isInvalid() || BeforeLoc.isInvalid())
+ return std::nullopt;
+
// If AfterLoc is in function-like macro, use the right parenthesis
// location.
if (AfterLoc.isMacroID()) {
@@ -1370,9 +1376,8 @@ struct CounterCoverageMappingBuilder
for (const Stmt *Child : S->children())
if (Child) {
// If last statement contains terminate statements, add a gap area
-// between the two statements. Skipping attributed statements, because
-// they don't have valid start location.
-if (LastStmt && HasTerminateStmt && !isa(Child)) {
+// between the two statements.
+if (LastStmt && HasTerminateStmt) {
auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
if (Gap)
fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),
diff --git a/clang/test/CoverageMapping/statement-expression.c
b/clang/test/CoverageMapping/statement-expression.c
new file mode 100644
index 00..5f9ab5838af342
--- /dev/null
+++ b/clang/test/CoverageMapping/statement-expression.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping
-emit-llvm-only -main-file-name statement-expression.c %s
+
+// No crash for the following examples, where GNU Statement Expression
extension
+// could introduce region terminators (break, goto etc) before implicit
+// initializers in a struct or an array.
+// See https://github.com/llvm/llvm-project/pull/89564
+
+struct Foo {
+ int field1;
+ int field2;
+};
+
+void f1(void) {
+ struct Foo foo = {
+.field1 = ({
+ switch (0) {
+ case 0:
+break; // A region terminator
+ }
+ 0;
+}),
+// ImplicitValueInitExpr introduced here for .field2
+ };
+}
+
+void f2(void) {
+ int arr[3] = {
+[0] = ({
+goto L0; // A region terminator
+L0:
+ 0;
+}),
+// ImplicitValueInitExpr introduced here for subscript [1]
+[2] = 0,
+ };
+}
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
