[coro] Fix crash due to DILabel in LineTableOnly mode

[vogelsgesang]

Since the recent commit de3c841, the CoroSplit pass adds DILabels to help find the location of a suspension point from its suspension point id. Those labels are added in both DebugEmissionKind::FullDebug and DebugEmissionKind::LineTableOnly mode. The idea was that this information is necessary to reconstruct async stack traces and should hence also be available for LineTableOnly.

Unfortunately, it turns out that the DWARF backend does not expect to find any DILabel debug metadata if the emission kind is set to LineTableOnly. The Dwarf backend simply runs into an assertion in that case.

This commit fixes the issue by only adding DILabel for FullDebug.

[llvmbot]

@llvm/pr-subscribers-coroutines

@llvm/pr-subscribers-llvm-transforms

Author: Adrian Vogelsgesang (vogelsgesang)

Changes

Since the recent commit de3c841, the CoroSplit pass adds DILabels to help find the location of a suspension point from its suspension point id. Those labels are added in both DebugEmissionKind::FullDebug and DebugEmissionKind::LineTableOnly mode. The idea was that this information is necessary to reconstruct async stack traces and should hence also be available for LineTableOnly.

Unfortunately, it turns out that the DWARF backend does not expect to find any DILabel debug metadata if the emission kind is set to LineTableOnly. The Dwarf backend simply runs into an assertion in that case.

This commit fixes the issue by only adding DILabel for FullDebug.

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+3-5)

diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 077dbe697a87e..6e5a017238c20 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -1485,11 +1485,9 @@ struct SwitchCoroutineSplitter {
     // without debug info. So we also don't generate debug info for the
     // suspension points.
     bool AddDebugLabels =
-        (DIS && DIS->getUnit() &&
-         (DIS->getUnit()->getEmissionKind() ==
-              DICompileUnit::DebugEmissionKind::FullDebug ||
-          DIS->getUnit()->getEmissionKind() ==
-              DICompileUnit::DebugEmissionKind::LineTablesOnly));
+        DIS && DIS->getUnit() &&
+        (DIS->getUnit()->getEmissionKind() ==
+              DICompileUnit::DebugEmissionKind::FullDebug);
 
     // resume.entry:
     //  %index.addr = getelementptr inbounds %f.Frame, %f.Frame* %FramePtr, i32

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[bgra8]

I just verified that the crash we were seeing is fixed with this change.

Thank you!

[alexfh]

Please add a regression test.

[vogelsgesang]

Please add a regression test.

Added a test case. Let me know if you are happy with that.
If you are happy with the test case, feel free to hit "Squash and Merge" on my behalf. I would not be planning any other changes and this PR would be ready for merging. (Assuming that CI comes back green, of course. But it passed on my local machine, at least)

[alexfh]

Please add a regression test.

Added a test case. Let me know if you are happy with that. If you are happy with the test case, feel free to hit "Squash and Merge" on my behalf. I would not be planning any other changes and this PR would be ready for merging. (Assuming that CI comes back green, of course. But it passed on my local machine, at least)

Thanks! This looks good to me. I'd probably add a test from #141937 (comment) or a similar one that makes clang without this fix crash, but that can be done separately from this fix, which I would prefer to submit now to unblock our testing.