[DebugInfo] Use correct unit when creating variable across CU boundary #133282

jmorse · 2025-03-27T17:26:39Z

When creating a static member DIE, we place it in a potentially pre-existing context DIE, and that DIE might be located in a different CU if we're in an LTO context. When we then add the source-file-ID to the static member DIE, use the correct Unit to do so -- the one that owns the context DIE. Otherwise we might assign a file-ID from one CU to another, and there isn't a guarantee that they'll be the same file, or even exist.

Fixes #109227

(I'd normally remove my home directory from these tests, but in this circumstances the same-file-but-with-a-different-name nature of the DIFile is part of the test).

When creating a static member DIE, we place it in a potentially pre-existing context DIE, and that DIE might be located in a different CU if we're in an LTO context. When we then add the source-file-ID to the static member DIE, use the correct Unit to do so -- the one that owns the context DIE. Otherwise we might assign a file-ID from one CU to another, and there isn't a guarantee that they'll be the same file, or even exist. Fixes llvm#109227

llvmbot · 2025-03-27T17:27:27Z

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

When creating a static member DIE, we place it in a potentially pre-existing context DIE, and that DIE might be located in a different CU if we're in an LTO context. When we then add the source-file-ID to the static member DIE, use the correct Unit to do so -- the one that owns the context DIE. Otherwise we might assign a file-ID from one CU to another, and there isn't a guarantee that they'll be the same file, or even exist.

Fixes #109227

(I'd normally remove my home directory from these tests, but in this circumstances the same-file-but-with-a-different-name nature of the DIFile is part of the test).

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

2 Files Affected:

(modified) llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (+2-1)
(added) llvm/test/DebugInfo/X86/file-index-across-cu.ll (+78)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index 383fbfb3fbd2b..640428c0fc34f 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1856,13 +1856,14 @@ DIE *DwarfUnit::getOrCreateStaticMemberDIE(const DIDerivedType *DT) {
   if (DIE *StaticMemberDIE = getDIE(DT))
     return StaticMemberDIE;
 
+  DwarfUnit *ContextUnit = static_cast<DwarfUnit *>(ContextDIE->getUnit());
   DIE &StaticMemberDIE = createAndAddDIE(DT->getTag(), *ContextDIE, DT);
 
   const DIType *Ty = DT->getBaseType();
 
   addString(StaticMemberDIE, dwarf::DW_AT_name, DT->getName());
   addType(StaticMemberDIE, Ty);
-  addSourceLine(StaticMemberDIE, DT);
+  ContextUnit->addSourceLine(StaticMemberDIE, DT);
   addFlag(StaticMemberDIE, dwarf::DW_AT_external);
   addFlag(StaticMemberDIE, dwarf::DW_AT_declaration);
 
diff --git a/llvm/test/DebugInfo/X86/file-index-across-cu.ll b/llvm/test/DebugInfo/X86/file-index-across-cu.ll
new file mode 100644
index 0000000000000..7ad66378cd572
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/file-index-across-cu.ll
@@ -0,0 +1,78 @@
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -verify -
+; RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -debug-info - | FileCheck %s
+
+;; This test exercises the cross-CU production of type information. Due to
+;; unfortunate inputs, when the DW_TAG_variable for the global
+;; @a_struct_field8typeDataE is produced it will be done in the CU for 2.cpp.
+;; However, the type information for the "a_struct" structure type will have
+;; already been produced in 3.cpp's CU. Thus, we'll be creating a DIE in one
+;; CU from another CU. Check that when we do so, we get the source file
+;; identifier correct -- if the FileID came from the wrong CU then we'd end
+;; up with a false declaration file.
+;;
+;; Due to unrelated reasons there are two copies of the "typeData" static member
+;; in this LTO-linked IR -- if that ever gets fitxed, it's fine to drop the
+;; duplicate DW_TAG_variable, but we want to keep testing the DW_AT_decl_file
+;; that's created across a CU boundary.
+;;
+;; https://github.com/llvm/llvm-project/issues/109227
+
+; CHECK-LABEL: DW_TAG_structure_type
+; CHECK-NOT:   DW_TAG
+; CHECK:       DW_AT_name      ("a_struct")
+; CHECK-NOT:   DW_TAG
+; CHECK:       DW_TAG_variable
+; CHECK-NOT:   DW_TAG
+; CHECK:       DW_AT_decl_file       ("C:\Users\gbmorsej\source\bees/3.cpp")
+; CHECK-NOT:   DW_TAG
+; CHECK:       DW_TAG_variable
+; CHECK-NOT:   DW_TAG
+; CHECK:       DW_AT_decl_file       ("C:\users\gbmorsej\source/bees/2.cpp")
+
+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-sie-ps5"
+
+%struct.a_struct = type { i8 }
+
+@__trans_tmp_2 = hidden local_unnamed_addr global %struct.a_struct zeroinitializer, align 1, !dbg !0
+@a_struct_field8typeDataE = hidden local_unnamed_addr global i64 0, align 8, !dbg !13
+
+!llvm.dbg.cu = !{!2, !15}
+!llvm.linker.options = !{}
+!llvm.ident = !{!22, !22}
+!llvm.module.flags = !{!23, !24, !25, !26, !27, !28, !29, !30, !31, !32}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "__trans_tmp_2", scope: !2, file: !3, line: 9, type: !5, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_20, file: !3, producer: "clang version 21.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!3 = !DIFile(filename: "3.cpp", directory: "C:\\Users\\gbmorsej\\source\\bees", checksumkind: CSK_MD5, checksum: "05973c817251e916cc8ba01e728764dc")
+!4 = !{!0}
+!5 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "a_struct", file: !3, line: 4, size: 8, flags: DIFlagTypePassByValue, elements: !6, identifier: "redacted_struct_name")
+!6 = !{!7}
+!7 = !DIDerivedType(tag: DW_TAG_variable, name: "typeData", scope: !5, file: !3, line: 6, baseType: !8, flags: DIFlagStaticMember)
+!8 = !DIDerivedType(tag: DW_TAG_typedef, name: "TypeData", scope: !10, file: !9, line: 6, baseType: !12)
+!9 = !DIFile(filename: ".\\h1.h", directory: "C:\\Users\\gbmorsej\\source\\bees", checksumkind: CSK_MD5, checksum: "0c90de8c8e867df533d869035e11cf8c")
+!10 = !DINamespace(name: "b", scope: !11)
+!11 = !DINamespace(name: "a", scope: null)
+!12 = !DIBasicType(name: "long", size: 64, encoding: DW_ATE_signed)
+!13 = !DIGlobalVariableExpression(var: !14, expr: !DIExpression())
+!14 = distinct !DIGlobalVariable(name: "typeData", linkageName: "a_struct_field8typeDataE", scope: !15, file: !18, line: 10, type: !19, isLocal: false, isDefinition: true, declaration: !21)
+!15 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_20, file: !16, producer: "clang version 21.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !17, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!16 = !DIFile(filename: "C:\\users\\gbmorsej\\source/bees\\2.cpp", directory: "C:\\Users\\gbmorsej\\source\\bees", checksumkind: CSK_MD5, checksum: "231f25b4978512f0d961a1d7baa9cb01")
+!17 = !{!13}
+!18 = !DIFile(filename: "C:\\users\\gbmorsej\\source/bees/2.cpp", directory: "", checksumkind: CSK_MD5, checksum: "231f25b4978512f0d961a1d7baa9cb01")
+!19 = !DIDerivedType(tag: DW_TAG_typedef, name: "TypeData", scope: !10, file: !20, line: 6, baseType: !12)
+!20 = !DIFile(filename: "h1.h", directory: "C:\\Users\\gbmorsej\\source\\bees", checksumkind: CSK_MD5, checksum: "0c90de8c8e867df533d869035e11cf8c")
+!21 = !DIDerivedType(tag: DW_TAG_variable, name: "typeData", scope: !5, file: !18, line: 6, baseType: !19, flags: DIFlagStaticMember)
+!22 = !{!"clang version 21.0.0"}
+!23 = !{i32 7, !"Dwarf Version", i32 5}
+!24 = !{i32 2, !"Debug Info Version", i32 3}
+!25 = !{i32 1, !"wchar_size", i32 2}
+!26 = !{i32 1, !"SIE:somestuff", i32 2}
+!27 = !{i32 8, !"PIC Level", i32 2}
+!28 = !{i32 7, !"uwtable", i32 2}
+!29 = !{i32 7, !"frame-pointer", i32 1}
+!30 = !{i32 1, !"MaxTLSAlign", i32 256}
+!31 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+!32 = !{i32 1, !"UnifiedLTO", i32 1}
+

dwblaikie

Could remove the duplicate DIGlobalVariable to simplify the test case/not have that oddity in this case?

Shouldn't be /too/ hard to create this situation with a legitimate situation using LTO (even just llvm-link to merge a couple of files) - is it sufficient to have one CU with a type (any use of the type should get that emitted - like use the type as a function parameter type) and then the static member defined in another translation unit? Seems like that'd be easy to create...

jmorse · 2025-04-01T10:15:19Z

Hmmm -- I tried to exercise that path for a while without success. It rather looks like the window for error is very narrow:

The DIE for the class/struct must be constructed in CU "a",
The global referring to a (duplicate) DIDerivedType+DW_TAG_variable must then be processed, in a different CU "b"

And globals are processed before functions in DwarfDebug it seems, thus I can't get the class/struct type to be produced by a function before step 2. Hence, two globals seems to be necessary. I guess this explains why we don't see this kind of error very frequently.

dwblaikie · 2025-04-01T14:53:46Z

Hmmm -- I tried to exercise that path for a while without success. It rather looks like the window for error is very narrow:

The DIE for the class/struct must be constructed in CU "a",

The global referring to a (duplicate) DIDerivedType+DW_TAG_variable must then be processed, in a different CU "b"

And globals are processed before functions in DwarfDebug it seems, thus I can't get the class/struct type to be produced by a function before step 2. Hence, two globals seems to be necessary. I guess this explains why we don't see this kind of error very frequently.

Could you walk me through that in a bit more detail?

One global variable definition in CU1 is created, that creates the type in CU1, then another global variable definition in CU2 is created, its declaration injected into the type in CU1 and then gets the wrong file index (based on the files in CU2, instead of CU1)?

If two static member variable definitions/DIGlobalVariables are required to reproduce the issue - is it possible for those to be actually distinct globals, rather than "meant to be the same but distinct by unfortunate circumstances"?

jmorse · 2025-04-01T15:42:34Z

Sure, here's what I believe the workflow for creating these DIEs looks at -- it's rooted in DwarfDebug::beginModule, where the global-variable DIEs are created with getOrCreateGlobalVariableDIE. Using the reproducer LLVM-IR from #109227 ,

The global-variable DIE for __trans_tmp_2 is created, of type a_struct,
The a_struct DIE is place inside the CU for 3.cpp, where __trans_tmp_2 is defined,
All the elements of the struct are added to the a_struct DIE, and we get this (printed using DIE::dump()) :

Die: 0x555562c02ce0, Offset: 0, Size: 0
DW_TAG_structure_type DW_CHILDREN_yes
DW_AT_calling_convention  DW_FORM_data1 Int: 5  0x5
DW_AT_name  DW_FORM_strx1 String: a_struct
DW_AT_byte_size  DW_FORM_data1 Int: 1  0x1
DW_AT_decl_file  DW_FORM_data1 Int: 0  0x0
DW_AT_decl_line  DW_FORM_data1 Int: 4  0x4
    Die: 0x555562c02d10, Offset: 0, Size: 0
    DW_TAG_variable DW_CHILDREN_no
    DW_AT_name  DW_FORM_strx1 String: typeData
    DW_AT_type  DW_FORM_ref4 Die: 0x555562c02f50
    DW_AT_decl_file  DW_FORM_data1 Int: 0  0x0
    DW_AT_decl_line  DW_FORM_data1 Int: 6  0x6
    DW_AT_external  DW_FORM_flag_present Int: 1  0x1
    DW_AT_declaration  DW_FORM_flag_present Int: 1  0x1

We move on to creating the DIE for global variable a_struct_field8typeDataE ,
We reach DwarfUnit::getOrCreateStaticMemberDIE with "this" DwarfUnit being "2.cpp", a separate compile unit that contains a_struct_field8typeDataE,
Due to the DIDerivedTypes and other metadata nodes not being uniqued because the DIFiles have different spellings, the DIDerivedType for a_struct_field8typeDataE is duplicate and doesn't have a DIE yet,
Thus DwarfUnit::getOrCreateStaticMemberDIE creates a new DIE in the context of the a_struct DW_TAG_composite_type, which happens to be across a CU boundary, attaching FileIDs from "2.cpp"s CU to a_struct's DIE in the "3.cpp" CU.
We get this in memory:

Die: 0x555562c02ce0, Offset: 0, Size: 0
DW_TAG_structure_type DW_CHILDREN_yes
DW_AT_calling_convention  DW_FORM_data1 Int: 5  0x5
DW_AT_name  DW_FORM_strx1 String: a_struct
DW_AT_byte_size  DW_FORM_data1 Int: 1  0x1
DW_AT_decl_file  DW_FORM_data1 Int: 0  0x0
DW_AT_decl_line  DW_FORM_data1 Int: 4  0x4
    Die: 0x555562c02d10, Offset: 0, Size: 0
    DW_TAG_variable DW_CHILDREN_no
    DW_AT_name  DW_FORM_strx1 String: typeData
    DW_AT_type  DW_FORM_ref4 Die: 0x555562c02f50
    DW_AT_decl_file  DW_FORM_data1 Int: 0  0x0
    DW_AT_decl_line  DW_FORM_data1 Int: 6  0x6
    DW_AT_external  DW_FORM_flag_present Int: 1  0x1
    DW_AT_declaration  DW_FORM_flag_present Int: 1  0x1

    Die: 0x555562c05a70, Offset: 0, Size: 0
    DW_TAG_variable DW_CHILDREN_no
    DW_AT_name  DW_FORM_strx1 String: typeData
    DW_AT_type  DW_FORM_ref_addr Die: 0x555562c05c20
    DW_AT_decl_file  DW_FORM_data1 Int: 2  0x2
    DW_AT_decl_line  DW_FORM_data1 Int: 6  0x6
    DW_AT_external  DW_FORM_flag_present Int: 1  0x1
    DW_AT_declaration  DW_FORM_flag_present Int: 1  0x1

Note the cross-CU reference for the type of the second DW_TAG_variable.

In all the alternative paths I tried, everything that refers to the a_struct type causes it to be entirely emitted inside a CU without anything remarkable happening. If CU "2.cpp", containing a_struct_field8typeDataE is the first to create a_struct then we create the structs DIE, then add the duplicate static member to it, but without any cross-CU behaviour. Thus, I think it's fundamental to this bug that the DIE for a_struct must already have been constructed in a separate CU (here, "3.cpp") before the duplicate static member is then encountered in a different CU ("2.cpp"). And as global variables appear to be the earliest thing to have their DIEs created, it necessitates two globals.

I tried some experiments to see whether this behaviour could be replicated with other kinds of DIDerivedType to insert, for example, duplicate DW_TAG_member's into a DW_TAG_structure_type. However, according to the langref those are unique'd by name: and scope: fields, possibly to prevent situations like this. Perhaps given that precedent, changing the uniquing rules for DW_TAG_variable DIDerivedTypes is a cleaner path?

jmorse · 2025-04-01T15:45:37Z

If two static member variable definitions/DIGlobalVariables are required to reproduce the issue - is it possible for those to be actually distinct globals, rather than "meant to be the same but distinct by unfortunate circumstances"?

Ah, yeah -- they are already distinct globals in this reproducer, however one of them is of the class type, wheras the second refers to the static-member type within that class. Thus, the order in which they're seen affects the order/CU in which the class type is created.

dwblaikie · 2025-04-01T16:40:17Z

Ah, OK, the two globals are a separate thing from the "two but kind of one" static members - I misunderstood there, OK.

I tried some experiments to see whether this behaviour could be replicated with other kinds of DIDerivedType to insert, for example, duplicate DW_TAG_member's into a DW_TAG_structure_type. However, according to the langref those are unique'd by name: and scope: fields, possibly to prevent situations like this. Perhaps given that precedent, changing the uniquing rules for DW_TAG_variable DIDerivedTypes is a cleaner path?

Eh, I think this patch is probably good on principle - or, I guess the other principle might be to /assert/ that a static member variable declaration is already present, rather than injecting it... except for the cases where we don't do that.

OK, so maybe here's a place for a less-awkward test case: We do inject certain things into classes that aren't in the classes existing member list: nested types, implicit special members (copy ctor, etc), and member template instantiations.

Can you reproduce this with a member variable template, perhaps?

jmorse · 2025-05-02T16:07:10Z

I got drawn away from this; last I recall was putting some time into finding a better test but without any success. If it's not a major issue, acceptable to land as-is?

jmorse · 2025-06-05T09:32:01Z

(Merging on the basis of the earlier LGTM)

llvm#133282) When creating a static member DIE, we place it in a potentially pre-existing context DIE, and that DIE might be located in a different CU if we're in an LTO context. When we then add the source-file-ID to the static member DIE, use the correct Unit to do so -- the one that owns the context DIE. Otherwise we might assign a file-ID from one CU to another, and there isn't a guarantee that they'll be the same file, or even exist. Fixes llvm#109227 (I'd normally remove my home directory from these tests, but in this circumstances the same-file-but-with-a-different-name nature of the DIFile is part of the test).

jmorse requested review from adrian-prantl, SLTozer, OCHyams and dwblaikie March 27, 2025 17:26

llvmbot added the debuginfo label Mar 27, 2025

dwblaikie approved these changes Mar 27, 2025

View reviewed changes

jmorse added 2 commits April 1, 2025 11:41

Tolerate windows path separators

3f0237f

I don't always commit changes, but when I do, I commit typos

034fc12

jmorse merged commit df4199c into llvm:main Jun 5, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[DebugInfo] Use correct unit when creating variable across CU boundary #133282

[DebugInfo] Use correct unit when creating variable across CU boundary #133282

Uh oh!

jmorse commented Mar 27, 2025

Uh oh!

llvmbot commented Mar 27, 2025

Uh oh!

dwblaikie left a comment

Uh oh!

jmorse commented Apr 1, 2025

Uh oh!

dwblaikie commented Apr 1, 2025

Uh oh!

jmorse commented Apr 1, 2025

Uh oh!

jmorse commented Apr 1, 2025

Uh oh!

dwblaikie commented Apr 1, 2025

Uh oh!

jmorse commented May 2, 2025

Uh oh!

jmorse commented Jun 5, 2025

Uh oh!

Uh oh!

Uh oh!

[DebugInfo] Use correct unit when creating variable across CU boundary #133282

[DebugInfo] Use correct unit when creating variable across CU boundary #133282

Uh oh!

Conversation

jmorse commented Mar 27, 2025

Uh oh!

llvmbot commented Mar 27, 2025

Uh oh!

dwblaikie left a comment

Choose a reason for hiding this comment

Uh oh!

jmorse commented Apr 1, 2025

Uh oh!

dwblaikie commented Apr 1, 2025

Uh oh!

jmorse commented Apr 1, 2025

Uh oh!

jmorse commented Apr 1, 2025

Uh oh!

dwblaikie commented Apr 1, 2025

Uh oh!

jmorse commented May 2, 2025

Uh oh!

jmorse commented Jun 5, 2025

Uh oh!

Uh oh!

Uh oh!