Skip to content

release/18.x: [clang][CoverageMapping] do not emit a gap region when either end doesn't have valid source locations (#89564) #90369

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

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Apr 28, 2024

Backport c1b6cca

Requested by: @whentojump

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Apr 28, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Apr 28, 2024

@ZequanWu What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from ZequanWu April 28, 2024 00:53
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Apr 28, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Apr 28, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: None (llvmbot)

Changes

Backport c1b6cca

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 --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<SourceRange> 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<AttributedStmt>(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 00000000000000..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,
+  };
+}

…sn't have valid source locations (llvm#89564)

Fixes llvm#86998

(cherry picked from commit c1b6cca)
@tstellar tstellar merged commit aea091b into llvm:release/18.x Apr 30, 2024
7 of 8 checks passed
@tstellar
Copy link
Collaborator

tstellar commented May 1, 2024

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.

@whentojump
Copy link
Member

whentojump commented May 1, 2024

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 <invalid sloc>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category release:note
Projects
Development

Successfully merging this pull request may close these issues.

4 participants