[PATCH] D94976: [DWARF] Create subprogram's DIE in the unit specified by its DISubprogram
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 19 15:27:07 PST 2021
dblaikie added a comment.
(seems there is perhaps some nearby pretty problematic issues I'm just writing down here because I saw it: Generally there are two kinds of DISubprograms: definition DISubprograms are "distinct" and referenced/kept alive from an llvm::Function's subprogram attachment. Declaration DISubprograms are not distinct and referenced from the member list of a DICompositeType, for instance. That's the simple case - if IR Linking occurs, one copy of an llvm::Function definition is retained, along with its referenced DISubprogram, simple enough. But it seems we are keeping definition DISubprograms alive through a few other means (or declaration DISubprograms, perhaps) - such as from call site metadata, using declarations, and the scope chains for types like lambdas. These DISubprograms aren't properly deduplicated because they can't necessarily piggyback on the llvm::Function deduplication: They are referenced from multiple parts of the debug info metadata - this can lead to buggy/weird DWARF like this:
$ cat a.cpp
namespace ns {
inline void f1() {
}
}
void f3() {
ns::f1();
}
$ cat b.cpp
namespace ns {
inline void f1() {
}
}
void f2() { ns::f1(); }
using ns::f1;
void f3();
int main() {
f3();
f2();
}
$ ~/dev/llvm/build/default/bin/clang++ -fuse-ld=lld -g -flto a.cpp b.cpp && llvm-dwarfdump-tot a.out
a.out: file format elf64-x86-64
.debug_info contents:
0x00000000: Compile Unit: length = 0x0000006e, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x00000072)
0x0000000b: DW_TAG_compile_unit
DW_AT_producer ("clang version 12.0.0 (git at github.com:llvm/llvm-project.git 05f846b97e222815b36efa815cbc67802c3e283c)")
DW_AT_language (DW_LANG_C_plus_plus_14)
DW_AT_name ("b.cpp")
DW_AT_stmt_list (0x00000000)
DW_AT_comp_dir ("/usr/local/google/home/blaikie/dev/scratch")
DW_AT_low_pc (0x0000000000000000)
DW_AT_ranges (0x00000000
[0x00000000002017c0, 0x00000000002017cb)
[0x00000000002017d0, 0x00000000002017e2))
0x0000002a: DW_TAG_namespace
DW_AT_name ("ns")
0x0000002f: DW_TAG_subprogram
0x00000030: NULL
0x00000031: DW_TAG_imported_declaration
DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/b.cpp")
DW_AT_decl_line (6)
DW_AT_import (0x0000002f)
0x00000038: DW_TAG_subprogram
DW_AT_low_pc (0x00000000002017c0)
DW_AT_high_pc (0x00000000002017cb)
DW_AT_frame_base (DW_OP_reg6 RBP)
DW_AT_linkage_name ("_Z2f2v")
DW_AT_name ("f2")
DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/b.cpp")
DW_AT_decl_line (5)
DW_AT_external (true)
0x00000051: DW_TAG_subprogram
DW_AT_low_pc (0x00000000002017d0)
DW_AT_high_pc (0x00000000002017e2)
DW_AT_frame_base (DW_OP_reg6 RBP)
DW_AT_name ("main")
DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/b.cpp")
DW_AT_decl_line (8)
DW_AT_type (0x0000006a "int")
DW_AT_external (true)
0x0000006a: DW_TAG_base_type
DW_AT_name ("int")
DW_AT_encoding (DW_ATE_signed)
DW_AT_byte_size (0x04)
0x00000071: NULL
0x00000072: Compile Unit: length = 0x0000005f, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x000000d5)
0x0000007d: DW_TAG_compile_unit
DW_AT_producer ("clang version 12.0.0 (git at github.com:llvm/llvm-project.git 05f846b97e222815b36efa815cbc67802c3e283c)")
DW_AT_language (DW_LANG_C_plus_plus_14)
DW_AT_name ("a.cpp")
DW_AT_stmt_list (0x00000059)
DW_AT_comp_dir ("/usr/local/google/home/blaikie/dev/scratch")
DW_AT_low_pc (0x0000000000000000)
DW_AT_ranges (0x00000030
[0x00000000002017a0, 0x00000000002017ab)
[0x00000000002017b0, 0x00000000002017b6))
0x0000009c: DW_TAG_subprogram
DW_AT_low_pc (0x00000000002017a0)
DW_AT_high_pc (0x00000000002017ab)
DW_AT_frame_base (DW_OP_reg6 RBP)
DW_AT_linkage_name ("_Z2f3v")
DW_AT_name ("f3")
DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/a.cpp")
DW_AT_decl_line (5)
DW_AT_external (true)
0x000000b5: DW_TAG_namespace
DW_AT_name ("ns")
0x000000ba: DW_TAG_subprogram
DW_AT_low_pc (0x00000000002017b0)
DW_AT_high_pc (0x00000000002017b6)
DW_AT_frame_base (DW_OP_reg6 RBP)
DW_AT_linkage_name ("_ZN2ns2f1Ev")
DW_AT_name ("f1")
DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/a.cpp")
DW_AT_decl_line (2)
DW_AT_external (true)
0x000000d3: NULL
0x000000d4: NULL
$ cat ab.ll | grep f1
$_ZN2ns2f1Ev = comdat any
call void @_ZN2ns2f1Ev(), !dbg !18
define linkonce_odr dso_local void @_ZN2ns2f1Ev() #1 comdat !dbg !20 {
call void @_ZN2ns2f1Ev(), !dbg !23
!7 = distinct !DISubprogram(name: "f1", linkageName: "_ZN2ns2f1Ev", scope: !8, file: !4, line: 2, type: !9, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !3, retainedNodes: !2)
!20 = distinct !DISubprogram(name: "f1", linkageName: "_ZN2ns2f1Ev", scope: !8, file: !1, line: 2, type: !9, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
Which is a bit problematic. Almost feels like the solution to that is to have the debug info metadata refer (weakly) to the llvm::Function instead of the distinct DISubprogram...
I would expect this should show up for call site debug info too, which might catch more interest from the folks who've been implementing that more recently. Oh, maybe it doesn't show up there - precisely because that debug info does essentially refer to the llvm::Function (more specifically call site debug info isn't carried in the IR at all (except by adding DISubprograms to llvm::Function declarations, which isn't otherwise used/needed) - it's created at the backend).
Might be able to tickle something like this with the scoping issue you've got here - even with this bug fixed, if you had the sort of cross-CU referencing I've created in this example)
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1077-1086
+ // If this unit is neither the SPs compile unit, nor its skeleton, then
+ // create the SP in the compile unit it specifies.
+ if (const DICompileUnit *SPCU = SP->getUnit()) {
+ DwarfCompileUnit &SPUnit = DD->getOrCreateDwarfCompileUnit(SPCU);
+ DwarfCompileUnit *Skeleton = SPUnit.getSkeleton();
+ const DIE *ThisDie = &getUnitDie();
+ if (&SPUnit.getUnitDie() != ThisDie && // SP in this unit?
----------------
This seems a bit more complicated than it needs to be (does it need to check skeleton DIEs, etc?)
The code I'd expect would be something like DwarfDebug's constructSubprogramDefinitionDIE - perhaps updating the caller to call constructSubprogramDefinitionDIE rather than modifying getOrCreateSubprogramDIE would be suitable.
(I realize this leaves open the possibility that other callers might need the same treatment - an assertion here that DD->getOrCreateDwarfCompileUnit(SP) == this might be good to catch those & see if there's a more systemic issue to be addressed?)
================
Comment at: llvm/test/DebugInfo/X86/subprogram-across-cus.ll:6-28
+;$ cat -n 1.cpp
+; 1 struct HHH;
+; 2
+; 3 struct xxx {
+; 4 HHH yyy();
+; 5 } zzz;
+; 6
----------------
Would be good to simplify this down a bit as I did in the bug - it doesn't need a lambda (a named local type will do - without a need for any members, etc) or a while loop, or the extra 'xxx' type, 'ccc' can be a static member to avoid the need to describe a parameter.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94976/new/
https://reviews.llvm.org/D94976
More information about the llvm-commits
mailing list