Skip to content

Commit b169f05

Browse files
labathadrian-prantl
labath
authored and
adrian-prantl
committed
Reapply "[lldb] Tolerate multiple compile units with the same DWO ID (llvm#100577)" (llvm#104041)
The only change vs. the first version of the patch is that I've made DWARFUnit linking thread-safe/unit. This was necessary because, during the indexing step, two skeleton units could attempt to associate themselves with the split unit. The original commit message was: I ran into this when LTO completely emptied two compile units, so they ended up with the same hash (see llvm#100375). Although, ideally, the compiler would try to ensure we don't end up with a hash collision even in this case, guaranteeing their absence is practically impossible. This patch ensures this situation does not bring down lldb. (cherry picked from commit 57a19ac)
1 parent 4afd16c commit b169f05

File tree

3 files changed

+168
-19
lines changed

3 files changed

+168
-19
lines changed

‎lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,14 @@ void DWARFUnit::ExtractUnitDIEIfNeeded() {
9797
*m_dwo_id, m_first_die.GetOffset()));
9898
return; // Can't fetch the compile unit from the dwo file.
9999
}
100-
// If the skeleton compile unit gets its unit DIE parsed first, then this
101-
// will fill in the DWO file's back pointer to this skeleton compile unit.
102-
// If the DWO files get parsed on their own first the skeleton back link
103-
// can be done manually in DWARFUnit::GetSkeletonCompileUnit() which will
104-
// do a reverse lookup and cache the result.
105-
dwo_cu->SetSkeletonUnit(this);
100+
101+
// Link the DWO unit to this object, if it hasn't been linked already (this
102+
// can happen when we have an index, and the DWO unit is parsed first).
103+
if (!dwo_cu->LinkToSkeletonUnit(*this)) {
104+
SetDwoError(Status::createWithFormat(
105+
"multiple compile units with Dwo ID {0:x16}", *m_dwo_id));
106+
return;
107+
}
106108

107109
DWARFBaseDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
108110
if (!dwo_cu_die.IsValid()) {
@@ -708,23 +710,28 @@ uint8_t DWARFUnit::GetAddressByteSize(const DWARFUnit *cu) {
708710
uint8_t DWARFUnit::GetDefaultAddressSize() { return 4; }
709711

710712
DWARFCompileUnit *DWARFUnit::GetSkeletonUnit() {
711-
if (m_skeleton_unit == nullptr && IsDWOUnit()) {
713+
if (m_skeleton_unit.load() == nullptr && IsDWOUnit()) {
712714
SymbolFileDWARFDwo *dwo =
713715
llvm::dyn_cast_or_null<SymbolFileDWARFDwo>(&GetSymbolFileDWARF());
714716
// Do a reverse lookup if the skeleton compile unit wasn't set.
715-
if (dwo)
716-
m_skeleton_unit = dwo->GetBaseSymbolFile().GetSkeletonUnit(this);
717+
DWARFUnit *candidate_skeleton_unit =
718+
dwo ? dwo->GetBaseSymbolFile().GetSkeletonUnit(this) : nullptr;
719+
if (candidate_skeleton_unit)
720+
(void)LinkToSkeletonUnit(*candidate_skeleton_unit);
721+
// Linking may fail due to a race, so be sure to return the actual value.
717722
}
718-
return llvm::dyn_cast_or_null<DWARFCompileUnit>(m_skeleton_unit);
723+
return llvm::dyn_cast_or_null<DWARFCompileUnit>(m_skeleton_unit.load());
719724
}
720725

721-
void DWARFUnit::SetSkeletonUnit(DWARFUnit *skeleton_unit) {
722-
// If someone is re-setting the skeleton compile unit backlink, make sure
723-
// it is setting it to a valid value when it wasn't valid, or if the
724-
// value in m_skeleton_unit was valid, it should be the same value.
725-
assert(skeleton_unit);
726-
assert(m_skeleton_unit == nullptr || m_skeleton_unit == skeleton_unit);
727-
m_skeleton_unit = skeleton_unit;
726+
bool DWARFUnit::LinkToSkeletonUnit(DWARFUnit &skeleton_unit) {
727+
DWARFUnit *expected_unit = nullptr;
728+
if (m_skeleton_unit.compare_exchange_strong(expected_unit, &skeleton_unit))
729+
return true;
730+
if (expected_unit == &skeleton_unit) {
731+
// Exchange failed because it already contained the right value.
732+
return true;
733+
}
734+
return false; // Already linked to a different unit.
728735
}
729736

730737
bool DWARFUnit::Supports_DW_AT_APPLE_objc_complete_type() {

‎lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class DWARFUnit : public UserID {
170170
/// both cases correctly and avoids crashes.
171171
DWARFCompileUnit *GetSkeletonUnit();
172172

173-
void SetSkeletonUnit(DWARFUnit *skeleton_unit);
173+
bool LinkToSkeletonUnit(DWARFUnit &skeleton_unit);
174174

175175
bool Supports_DW_AT_APPLE_objc_complete_type();
176176

@@ -308,7 +308,7 @@ class DWARFUnit : public UserID {
308308
const llvm::DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
309309
lldb_private::CompileUnit *m_lldb_cu = nullptr;
310310
// If this is a DWO file, we have a backlink to our skeleton compile unit.
311-
DWARFUnit *m_skeleton_unit = nullptr;
311+
std::atomic<DWARFUnit *> m_skeleton_unit = nullptr;
312312
// The compile unit debug information entry item
313313
DWARFDebugInfoEntry::collection m_die_array;
314314
mutable llvm::sys::RWMutex m_die_array_mutex;

‎lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
## Test that lldb handles (mainly, that it doesn't crash) the situation where
2+
## two skeleton compile units have the same DWO ID (and try to claim the same
3+
## split unit from the DWP file. This can sometimes happen when the compile unit
4+
## is nearly empty (e.g. because LTO has optimized all of it away).
5+
6+
# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym MAIN=0 > %t
7+
# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t.dwp
8+
# RUN: %lldb %t -o "image lookup -t my_enum_type" \
9+
# RUN: -o "image dump separate-debug-info" -o exit | FileCheck %s
10+
11+
## Check that we're able to access the type within the split unit (no matter
12+
## which skeleton unit it ends up associated with). Completely ignoring the unit
13+
## might also be reasonable.
14+
# CHECK: image lookup -t my_enum_type
15+
# CHECK: 1 match found
16+
# CHECK: name = "my_enum_type", byte-size = 4, compiler_type = "enum my_enum_type {
17+
# CHECK-NEXT: }"
18+
#
19+
## Check that we get some indication of the error.
20+
# CHECK: image dump separate-debug-info
21+
# CHECK: Dwo ID Err Dwo Path
22+
# CHECK: 0xdeadbeefbaadf00d E multiple compile units with Dwo ID 0xdeadbeefbaadf00d
23+
24+
.set DWO_ID, 0xdeadbeefbaadf00d
25+
26+
## The main file.
27+
.ifdef MAIN
28+
.section .debug_abbrev,"",@progbits
29+
.byte 1 # Abbreviation Code
30+
.byte 74 # DW_TAG_compile_unit
31+
.byte 0 # DW_CHILDREN_no
32+
.byte 0x76 # DW_AT_dwo_name
33+
.byte 8 # DW_FORM_string
34+
.byte 0 # EOM(1)
35+
.byte 0 # EOM(2)
36+
.byte 0 # EOM(3)
37+
38+
39+
.section .debug_info,"",@progbits
40+
.irpc I,01
41+
.Lcu_begin\I:
42+
.long .Ldebug_info_end\I-.Ldebug_info_start\I # Length of Unit
43+
.Ldebug_info_start\I:
44+
.short 5 # DWARF version number
45+
.byte 4 # DWARF Unit Type
46+
.byte 8 # Address Size (in bytes)
47+
.long .debug_abbrev # Offset Into Abbrev. Section
48+
.quad DWO_ID # DWO id
49+
.byte 1 # Abbrev [1] DW_TAG_compile_unit
50+
.ascii "foo"
51+
.byte '0' + \I
52+
.asciz ".dwo\0" # DW_AT_dwo_name
53+
.Ldebug_info_end\I:
54+
.endr
55+
56+
.else
57+
## DWP file starts here.
58+
59+
.section .debug_abbrev.dwo,"e",@progbits
60+
.LAbbrevBegin:
61+
.byte 1 # Abbreviation Code
62+
.byte 17 # DW_TAG_compile_unit
63+
.byte 1 # DW_CHILDREN_yes
64+
.byte 37 # DW_AT_producer
65+
.byte 8 # DW_FORM_string
66+
.byte 19 # DW_AT_language
67+
.byte 5 # DW_FORM_data2
68+
.byte 0 # EOM(1)
69+
.byte 0 # EOM(2)
70+
.byte 2 # Abbreviation Code
71+
.byte 4 # DW_TAG_enumeration_type
72+
.byte 0 # DW_CHILDREN_no
73+
.byte 3 # DW_AT_name
74+
.byte 8 # DW_FORM_string
75+
.byte 73 # DW_AT_type
76+
.byte 19 # DW_FORM_ref4
77+
.byte 11 # DW_AT_byte_size
78+
.byte 11 # DW_FORM_data1
79+
.byte 0 # EOM(1)
80+
.byte 0 # EOM(2)
81+
.byte 4 # Abbreviation Code
82+
.byte 36 # DW_TAG_base_type
83+
.byte 0 # DW_CHILDREN_no
84+
.byte 3 # DW_AT_name
85+
.byte 8 # DW_FORM_string
86+
.byte 62 # DW_AT_encoding
87+
.byte 11 # DW_FORM_data1
88+
.byte 11 # DW_AT_byte_size
89+
.byte 11 # DW_FORM_data1
90+
.byte 0 # EOM(1)
91+
.byte 0 # EOM(2)
92+
.byte 0 # EOM(3)
93+
.LAbbrevEnd:
94+
.section .debug_info.dwo,"e",@progbits
95+
.LCUBegin:
96+
.Lcu_begin1:
97+
.long .Ldebug_info_end1-.Ldebug_info_start1 # Length of Unit
98+
.Ldebug_info_start1:
99+
.short 5 # DWARF version number
100+
.byte 5 # DWARF Unit Type
101+
.byte 8 # Address Size (in bytes)
102+
.long 0 # Offset Into Abbrev. Section
103+
.quad DWO_ID # DWO id
104+
.byte 1 # Abbrev [1] DW_TAG_compile_unit
105+
.asciz "Hand-written DWARF" # DW_AT_producer
106+
.short 12 # DW_AT_language
107+
.byte 2 # Abbrev [2] DW_TAG_enumeration_type
108+
.asciz "my_enum_type" # DW_AT_name
109+
.long .Lint-.Lcu_begin1 # DW_AT_type
110+
.byte 4 # DW_AT_byte_size
111+
.Lint:
112+
.byte 4 # Abbrev [4] DW_TAG_base_type
113+
.asciz "int" # DW_AT_name
114+
.byte 5 # DW_AT_encoding
115+
.byte 4 # DW_AT_byte_size
116+
.byte 0 # End Of Children Mark
117+
.Ldebug_info_end1:
118+
.LCUEnd:
119+
.section .debug_cu_index, "", @progbits
120+
## Header:
121+
.short 5 # Version
122+
.short 0 # Padding
123+
.long 2 # Section count
124+
.long 1 # Unit count
125+
.long 2 # Slot count
126+
## Hash Table of Signatures:
127+
.quad 0
128+
.quad DWO_ID
129+
## Parallel Table of Indexes:
130+
.long 0
131+
.long 1
132+
## Table of Section Offsets:
133+
## Row 0:
134+
.long 1 # DW_SECT_INFO
135+
.long 3 # DW_SECT_ABBREV
136+
## Row 1:
137+
.long 0 # Offset in .debug_info.dwo
138+
.long 0 # Offset in .debug_abbrev.dwo
139+
## Table of Section Sizes:
140+
.long .LCUEnd-.LCUBegin # Size in .debug_info.dwo
141+
.long .LAbbrevEnd-.LAbbrevBegin # Size in .debug_abbrev.dwo
142+
.endif

0 commit comments

Comments
 (0)