[Coroutines] Fix debug info scoping for nested structs in coroutine frames #147622
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Grigory Pastukhov (grigorypas) ChangesWhen generating debug info for coroutine frames, nested struct types were incorrectly inheriting the top-level function scope instead of having their parent struct as scope. This caused assertion failures in DebugInfoMetadata.h during member list replacement for complex nested struct hierarchies. Fix by passing the parent DIStruct as scope when recursively calling solveDIType for nested struct fields, ensuring proper debug info scoping hierarchy. Add regression test that validates proper nested struct scoping hierarchy and cc: @MatzeB, @yuxuanchen1997 Full diff: https://github.com/llvm/llvm-project/pull/147622.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 7224a56cd7b8a..fe30c6dc6abe4 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -639,10 +639,10 @@ static DIType *solveDIType(DIBuilder &Builder, Type *Ty,
SmallVector<Metadata *, 16> Elements;
for (unsigned I = 0; I < StructTy->getNumElements(); I++) {
DIType *DITy = solveDIType(Builder, StructTy->getElementType(I), Layout,
- Scope, LineNum, DITypeCache);
+ DIStruct, LineNum, DITypeCache);
assert(DITy);
Elements.push_back(Builder.createMemberType(
- Scope, DITy->getName(), Scope->getFile(), LineNum,
+ DIStruct, DITy->getName(), DIStruct->getFile(), LineNum,
DITy->getSizeInBits(), DITy->getAlignInBits(),
Layout.getStructLayout(StructTy)->getElementOffsetInBits(I),
llvm::DINode::FlagArtificial, DITy));
diff --git a/llvm/test/Transforms/Coroutines/coro-split-dbg-nested-struct.ll b/llvm/test/Transforms/Coroutines/coro-split-dbg-nested-struct.ll
new file mode 100644
index 0000000000000..7aa1a0fb2c921
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-split-dbg-nested-struct.ll
@@ -0,0 +1,50 @@
+; RUN: opt < %s -passes='cgscc(coro-split)' -S | FileCheck %s
+
+; Test that nested structs in coroutine frames have correct debug info scoping.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Minimal nested struct types that trigger the scoping issue
+%"struct.Inner" = type { i32, ptr }
+%"struct.Outer" = type { %"struct.Inner", i64 }
+%"class.Promise" = type { %"struct.Outer" }
+
+define void @test_coro_function() presplitcoroutine !dbg !10 {
+entry:
+ %__promise = alloca %"class.Promise", align 8
+ %0 = call token @llvm.coro.id(i32 0, ptr %__promise, ptr null, ptr null)
+ %1 = call ptr @llvm.coro.begin(token %0, ptr null)
+ %2 = call token @llvm.coro.save(ptr null)
+ ret void
+}
+
+; The test passes if the debug info is generated without crashing
+; CHECK: define void @test_coro_function()
+
+; Check that frame debug info is generated
+; CHECK: ![[FRAME_TYPE:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "{{.*}}.coro_frame_ty"
+
+; Key validation: Check that nested structs have the correct scope hierarchy
+; 1. Promise should be scoped to the frame
+; CHECK: ![[PROMISE:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "class_Promise", scope: ![[FRAME_TYPE]]
+
+; 2. Outer should be scoped to Promise (not the frame!)
+; CHECK: ![[OUTER:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "struct_Outer", scope: ![[PROMISE]]
+
+; 3. Inner should be scoped to Outer (proper nesting)
+; CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "struct_Inner", scope: ![[OUTER]]
+
+declare token @llvm.coro.id(i32, ptr readnone, ptr readonly, ptr)
+declare ptr @llvm.coro.begin(token, ptr writeonly)
+declare token @llvm.coro.save(ptr)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "test.cpp", directory: ".")
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = distinct !DISubprogram(name: "test_coro_function", scope: !1, file: !1, line: 1, type: !11, spFlags: DISPFlagDefinition, unit: !0)
+!11 = !DISubroutineType(types: !12)
+!12 = !{null}
|
|
@llvm/pr-subscribers-coroutines Author: Grigory Pastukhov (grigorypas) ChangesWhen generating debug info for coroutine frames, nested struct types were incorrectly inheriting the top-level function scope instead of having their parent struct as scope. This caused assertion failures in DebugInfoMetadata.h during member list replacement for complex nested struct hierarchies. Fix by passing the parent DIStruct as scope when recursively calling solveDIType for nested struct fields, ensuring proper debug info scoping hierarchy. Add regression test that validates proper nested struct scoping hierarchy and cc: @MatzeB, @yuxuanchen1997 Full diff: https://github.com/llvm/llvm-project/pull/147622.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 7224a56cd7b8a..fe30c6dc6abe4 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -639,10 +639,10 @@ static DIType *solveDIType(DIBuilder &Builder, Type *Ty,
SmallVector<Metadata *, 16> Elements;
for (unsigned I = 0; I < StructTy->getNumElements(); I++) {
DIType *DITy = solveDIType(Builder, StructTy->getElementType(I), Layout,
- Scope, LineNum, DITypeCache);
+ DIStruct, LineNum, DITypeCache);
assert(DITy);
Elements.push_back(Builder.createMemberType(
- Scope, DITy->getName(), Scope->getFile(), LineNum,
+ DIStruct, DITy->getName(), DIStruct->getFile(), LineNum,
DITy->getSizeInBits(), DITy->getAlignInBits(),
Layout.getStructLayout(StructTy)->getElementOffsetInBits(I),
llvm::DINode::FlagArtificial, DITy));
diff --git a/llvm/test/Transforms/Coroutines/coro-split-dbg-nested-struct.ll b/llvm/test/Transforms/Coroutines/coro-split-dbg-nested-struct.ll
new file mode 100644
index 0000000000000..7aa1a0fb2c921
--- /dev/null
+++ b/llvm/test/Transforms/Coroutines/coro-split-dbg-nested-struct.ll
@@ -0,0 +1,50 @@
+; RUN: opt < %s -passes='cgscc(coro-split)' -S | FileCheck %s
+
+; Test that nested structs in coroutine frames have correct debug info scoping.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Minimal nested struct types that trigger the scoping issue
+%"struct.Inner" = type { i32, ptr }
+%"struct.Outer" = type { %"struct.Inner", i64 }
+%"class.Promise" = type { %"struct.Outer" }
+
+define void @test_coro_function() presplitcoroutine !dbg !10 {
+entry:
+ %__promise = alloca %"class.Promise", align 8
+ %0 = call token @llvm.coro.id(i32 0, ptr %__promise, ptr null, ptr null)
+ %1 = call ptr @llvm.coro.begin(token %0, ptr null)
+ %2 = call token @llvm.coro.save(ptr null)
+ ret void
+}
+
+; The test passes if the debug info is generated without crashing
+; CHECK: define void @test_coro_function()
+
+; Check that frame debug info is generated
+; CHECK: ![[FRAME_TYPE:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "{{.*}}.coro_frame_ty"
+
+; Key validation: Check that nested structs have the correct scope hierarchy
+; 1. Promise should be scoped to the frame
+; CHECK: ![[PROMISE:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "class_Promise", scope: ![[FRAME_TYPE]]
+
+; 2. Outer should be scoped to Promise (not the frame!)
+; CHECK: ![[OUTER:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "struct_Outer", scope: ![[PROMISE]]
+
+; 3. Inner should be scoped to Outer (proper nesting)
+; CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "struct_Inner", scope: ![[OUTER]]
+
+declare token @llvm.coro.id(i32, ptr readnone, ptr readonly, ptr)
+declare ptr @llvm.coro.begin(token, ptr writeonly)
+declare token @llvm.coro.save(ptr)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "test.cpp", directory: ".")
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = distinct !DISubprogram(name: "test_coro_function", scope: !1, file: !1, line: 1, type: !11, spFlags: DISPFlagDefinition, unit: !0)
+!11 = !DISubroutineType(types: !12)
+!12 = !{null}
|
MatzeB
requested review from
artempyanykh,
yuxuanchen1997,
ChuanqiXu9 and
vogelsgesang
llvm/test/Transforms/Coroutines/coro-split-dbg-nested-struct.ll
Outdated
Show resolved
Hide resolved
| target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128" | ||
| target triple = "x86_64-unknown-linux-gnu" | ||
|
|
||
| ; Minimal nested struct types that trigger the scoping issue |
There was a problem hiding this comment.
| ; Minimal nested struct types that trigger the scoping issue | |
| ; Minimal nested struct types that used to trigger a scoping issue: | |
| ; We used to set the wrong `scope` for the `DIDerivedType` member entries of the `DICompositeType` |
| DIType *DITy = solveDIType(Builder, StructTy->getElementType(I), Layout, | ||
| Scope, LineNum, DITypeCache); | ||
| DIStruct, LineNum, DITypeCache); | ||
| assert(DITy); | ||
| Elements.push_back(Builder.createMemberType( | ||
| Scope, DITy->getName(), Scope->getFile(), LineNum, | ||
| DIStruct, DITy->getName(), DIStruct->getFile(), LineNum, |
There was a problem hiding this comment.
afaict, the second change in createMemberType is the actually important one, correct? The change for the recursive call of solveDIType should be unnecessary to fix the crash?
Afaict, the parameter to solveDIType should only influence if the structs are modeled as "nested structs" or "sibling structs" in the DWARF debug info. Not sure which one we prefer here?
I guess sibling structs would feel more natural, since LLVM has no concept of nested structs, and solveDIType tries to mirror an LLVM struct hierarchy into debug info? If so, we should leave the parameters to solveDIType as they are
There was a problem hiding this comment.
Actually, the first change (on line 642) is what resolves the crash. When Builder.createStructType is called, the code below expects the object returned it to be newly created object. The code generates the same generic name for un-named structs. Thus, if the Scope is inherited from the top level function, Builder.createStructType might return previously cached object for different un-named struct.
DWARF supports nested structs. See for example here https://github.com/llvm/llvm-project/blob/main/llvm/test/DebugInfo/Generic/tu-composite.ll#L142C1-L146C154.
| ; CHECK: define void @test_coro_function() | ||
|
|
||
| ; Check that frame debug info is generated | ||
| ; CHECK: ![[FRAME_TYPE:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "{{.*}}.coro_frame_ty" |
There was a problem hiding this comment.
the test expectations only verify the nesting of the DICompositeType. Afaict, the root cause of the crash was not the nesting of the DICompositeType but rather of the DIDerivedTypes representing the members of the composite type, though?
There was a problem hiding this comment.
Added checks for DIDerivedType's as well.
When generating debug info for coroutine frames, nested struct types were incorrectly inheriting the top-level function scope instead of having their parent struct as scope. This caused assertion failures in DebugInfoMetadata.h during member list replacement for complex nested struct hierarchies.
Fix by passing the parent DIStruct as scope when recursively calling solveDIType for nested struct fields, ensuring proper debug info scoping hierarchy.
Add regression test that validates proper nested struct scoping hierarchy and
prevents future regressions.
cc: @MatzeB, @yuxuanchen1997