Skip to content

Conversation

whentojump
Copy link
Member

Fixes #86998

Two compiler explorer examples: 1, 2

Cause:

  1. When visiting AST and generating mapping regions, a region terminator (like break and goto) is likely followed by a stmt with <invalid sloc> (like ImplicitValueInitExpr).

  2. Because a terminator is seen, the below branch will be executed when visiting the 2nd stmt:

    if (LastStmt && HasTerminateStmt && !isa<AttributedStmt>(Child)) {
    auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));

  3. However, the 2nd stmt doesn't have a valid source location and will fail some assertions in findGapAreaBetween().

@whentojump whentojump added clang:codegen IR generation bugs: mangling, exceptions, etc. coverage crash Prefer [crash-on-valid] or [crash-on-invalid] labels Apr 22, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Apr 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 22, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Wentao Zhang (whentojump)

Changes

Fixes #86998

Two compiler explorer examples: 1, 2

Cause:

  1. When visiting AST and generating mapping regions, a region terminator (like break and goto) is likely followed by a stmt with &lt;invalid sloc&gt; (like ImplicitValueInitExpr).

  2. Because a terminator is seen, the below branch will be executed when visiting the 2nd stmt:

    if (LastStmt && HasTerminateStmt && !isa<AttributedStmt>(Child)) {
    auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));

  3. However, the 2nd stmt doesn't have a valid source location and will fail some assertions in findGapAreaBetween().


Full diff: https://github.com/llvm/llvm-project/pull/89564.diff

1 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+3-1)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 64c39c5de351c7..45296ff9cfb5e3 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1370,7 +1370,9 @@ struct CounterCoverageMappingBuilder
         // 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)) {
+        if (LastStmt && HasTerminateStmt && !isa<AttributedStmt>(Child) &&
+            !isa<ImplicitValueInitExpr>(Child) &&
+            !isa<ImplicitValueInitExpr>(LastStmt)) {
           auto Gap = findGapAreaBetween(getEnd(LastStmt), getStart(Child));
           if (Gap)
             fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(),

@whentojump whentojump requested a review from ZequanWu April 22, 2024 02:59
@ZequanWu
Copy link
Contributor

ZequanWu commented Apr 22, 2024

There could be some other statements with invalid source locations (we have seen that before). I suggest validating both source locations inside findGapAreaBetween and returning std::nullopt if either of them is invalid.

Also need a test case. You can add the crashed example as a test case under clang/test/CoverageMapping.

@whentojump whentojump marked this pull request as draft April 22, 2024 15:56
@whentojump whentojump marked this pull request as ready for review April 22, 2024 16:39
@whentojump
Copy link
Member Author

Thanks for your suggestion! @ZequanWu

Would you please take another look?

@whentojump
Copy link
Member Author

Fwiw, I uncovered #86998 in the first place when I was compiling this file in Linux kernel: https://elixir.bootlin.com/linux/v6.8.1/source/fs/coredump.c#L545. I can confirm on my local side, with this patch, fs/coredump.o can be compiled and instrumented successfully.

I am going to squash merge this PR.

@whentojump
Copy link
Member Author

/cherry-pick c1b6cca

@whentojump whentojump added this to the LLVM 18.X Release milestone Apr 28, 2024
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Apr 28, 2024
…sn't have valid source locations (llvm#89564)

Fixes llvm#86998

(cherry picked from commit c1b6cca)
@llvmbot
Copy link
Member

llvmbot commented Apr 28, 2024

/pull-request #90369

tstellar pushed a commit to llvmbot/llvm-project that referenced this pull request Apr 30, 2024
…sn't have valid source locations (llvm#89564)

Fixes llvm#86998

(cherry picked from commit c1b6cca)
@pointhex pointhex mentioned this pull request May 7, 2024
Tedlion pushed a commit to Tedlion/llvm-project that referenced this pull request Jun 15, 2025
…sn't have valid source locations (llvm#89564)

Fixes llvm#86998

(cherry picked from commit c1b6cca)
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 coverage crash Prefer [crash-on-valid] or [crash-on-invalid]
Projects
Development

Successfully merging this pull request may close these issues.

[clang][CoverageMapping] "Assertion AfterLoc.isValid() failed" during compiling switch within statement expressions
3 participants