Skip to content

Commit db979da

Browse files
committed
Make DWARFUnitVector threadsafe.
The DWARFUnitVector class lives inside of the DWARFContextState. Prior to this fix a non const reference was being handed out to clients. When fetching the DWO units, there used to be a "bool Lazy" parameter that could be passed that would allow the DWARFUnitVector to parse individual units on the fly. There were two major issues with this approach: - not thread safe and causes crashes - the accessor would check if DWARFUnitVector was empty and if not empty it would return a partially filled in DWARFUnitVector if it was constructed with "Lazy = true" This patch fixes the issues by always fully parsing the DWARFUnitVector when it is requested and only hands out a "const DWARFUnitVector &". This allows the thread safety mechanism built into the DWARFContext class to work corrrectly, and avoids the issue where if someone construct DWARFUnitVector with "Lazy = true", and then calls an API that partially fills in the DWARFUnitVector with individual entries, and then someone accesses the DWARFUnitVector, they would get a partial and incomplete listing of the DWARF units for the DWOs.
1 parent 3c11ac5 commit db979da

File tree

5 files changed

+80
-115
lines changed

5 files changed

+80
-115
lines changed

llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ class DWARFContext : public DIContext {
6969
public:
7070
DWARFContextState(DWARFContext &DC) : D(DC) {}
7171
virtual ~DWARFContextState() = default;
72-
virtual DWARFUnitVector &getNormalUnits() = 0;
73-
virtual DWARFUnitVector &getDWOUnits(bool Lazy = false) = 0;
72+
virtual const DWARFUnitVector &getNormalUnits() = 0;
73+
virtual const DWARFUnitVector &getDWOUnits() = 0;
7474
virtual const DWARFDebugAbbrev *getDebugAbbrevDWO() = 0;
7575
virtual const DWARFUnitIndex &getCUIndex() = 0;
7676
virtual const DWARFUnitIndex &getTUIndex() = 0;
@@ -119,11 +119,8 @@ class DWARFContext : public DIContext {
119119
std::function<void(Error)> WarningHandler = WithColor::defaultWarningHandler;
120120

121121
/// Read compile units from the debug_info.dwo section (if necessary)
122-
/// and type units from the debug_types.dwo section (if necessary)
123-
/// and store them in DWOUnits.
124-
/// If \p Lazy is true, set up to parse but don't actually parse them.
125-
enum { EagerParse = false, LazyParse = true };
126-
DWARFUnitVector &getDWOUnits(bool Lazy = false);
122+
/// and type units from the debug_types.dwo section (if necessary).
123+
const DWARFUnitVector &getDWOUnits();
127124

128125
std::unique_ptr<const DWARFObject> DObj;
129126

@@ -167,7 +164,7 @@ class DWARFContext : public DIContext {
167164

168165
/// Get units from .debug_info in this context.
169166
unit_iterator_range info_section_units() {
170-
DWARFUnitVector &NormalUnits = State->getNormalUnits();
167+
const DWARFUnitVector &NormalUnits = State->getNormalUnits();
171168
return unit_iterator_range(NormalUnits.begin(),
172169
NormalUnits.begin() +
173170
NormalUnits.getNumInfoUnits());
@@ -179,7 +176,7 @@ class DWARFContext : public DIContext {
179176

180177
/// Get units from .debug_types in this context.
181178
unit_iterator_range types_section_units() {
182-
DWARFUnitVector &NormalUnits = State->getNormalUnits();
179+
const DWARFUnitVector &NormalUnits = State->getNormalUnits();
183180
return unit_iterator_range(
184181
NormalUnits.begin() + NormalUnits.getNumInfoUnits(), NormalUnits.end());
185182
}
@@ -194,13 +191,13 @@ class DWARFContext : public DIContext {
194191

195192
/// Get all normal compile/type units in this context.
196193
unit_iterator_range normal_units() {
197-
DWARFUnitVector &NormalUnits = State->getNormalUnits();
194+
const DWARFUnitVector &NormalUnits = State->getNormalUnits();
198195
return unit_iterator_range(NormalUnits.begin(), NormalUnits.end());
199196
}
200197

201198
/// Get units from .debug_info..dwo in the DWO context.
202199
unit_iterator_range dwo_info_section_units() {
203-
DWARFUnitVector &DWOUnits = State->getDWOUnits();
200+
const DWARFUnitVector &DWOUnits = State->getDWOUnits();
204201
return unit_iterator_range(DWOUnits.begin(),
205202
DWOUnits.begin() + DWOUnits.getNumInfoUnits());
206203
}
@@ -211,7 +208,7 @@ class DWARFContext : public DIContext {
211208

212209
/// Get units from .debug_types.dwo in the DWO context.
213210
unit_iterator_range dwo_types_section_units() {
214-
DWARFUnitVector &DWOUnits = State->getDWOUnits();
211+
const DWARFUnitVector &DWOUnits = State->getDWOUnits();
215212
return unit_iterator_range(DWOUnits.begin() + DWOUnits.getNumInfoUnits(),
216213
DWOUnits.end());
217214
}
@@ -227,7 +224,7 @@ class DWARFContext : public DIContext {
227224

228225
/// Get all units in the DWO context.
229226
unit_iterator_range dwo_units() {
230-
DWARFUnitVector &DWOUnits = State->getDWOUnits();
227+
const DWARFUnitVector &DWOUnits = State->getDWOUnits();
231228
return unit_iterator_range(DWOUnits.begin(), DWOUnits.end());
232229
}
233230

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -123,23 +123,21 @@ bool isCompileUnit(const std::unique_ptr<DWARFUnit> &U);
123123

124124
/// Describe a collection of units. Intended to hold all units either from
125125
/// .debug_info and .debug_types, or from .debug_info.dwo and .debug_types.dwo.
126-
class DWARFUnitVector final : public SmallVector<std::unique_ptr<DWARFUnit>, 1> {
127-
std::function<std::unique_ptr<DWARFUnit>(uint64_t, DWARFSectionKind,
128-
const DWARFSection *,
129-
const DWARFUnitIndex::Entry *)>
130-
Parser;
126+
class DWARFUnitVector final
127+
: public SmallVector<std::unique_ptr<DWARFUnit>, 1> {
131128
int NumInfoUnits = -1;
132129

133130
public:
134131
using UnitVector = SmallVectorImpl<std::unique_ptr<DWARFUnit>>;
135-
using iterator = typename UnitVector::iterator;
136-
using iterator_range = llvm::iterator_range<typename UnitVector::iterator>;
132+
using iterator = typename UnitVector::const_iterator;
133+
using iterator_range =
134+
llvm::iterator_range<typename UnitVector::const_iterator>;
137135

138136
using compile_unit_range =
139137
decltype(make_filter_range(std::declval<iterator_range>(), isCompileUnit));
140138

141139
DWARFUnit *getUnitForOffset(uint64_t Offset) const;
142-
DWARFUnit *getUnitForIndexEntry(const DWARFUnitIndex::Entry &E);
140+
DWARFUnit *getUnitForIndexEntry(const DWARFUnitIndex::Entry &E) const;
143141

144142
/// Read units from a .debug_info or .debug_types section. Calls made
145143
/// before finishedInfoUnits() are assumed to be for .debug_info sections,
@@ -153,11 +151,7 @@ class DWARFUnitVector final : public SmallVector<std::unique_ptr<DWARFUnit>, 1>
153151
/// sections. Caller must not mix calls to addUnitsForSection and
154152
/// addUnitsForDWOSection.
155153
void addUnitsForDWOSection(DWARFContext &C, const DWARFSection &DWOSection,
156-
DWARFSectionKind SectionKind, bool Lazy = false);
157-
158-
/// Add an existing DWARFUnit to this UnitVector. This is used by the DWARF
159-
/// verifier to process unit separately.
160-
DWARFUnit *addUnit(std::unique_ptr<DWARFUnit> Unit);
154+
DWARFSectionKind SectionKind);
161155

162156
/// Returns number of all units held by this instance.
163157
unsigned getNumUnits() const { return size(); }
@@ -177,7 +171,7 @@ class DWARFUnitVector final : public SmallVector<std::unique_ptr<DWARFUnit>, 1>
177171
const DWARFSection *RS, const DWARFSection *LocSection,
178172
StringRef SS, const DWARFSection &SOS,
179173
const DWARFSection *AOS, const DWARFSection &LS, bool LE,
180-
bool IsDWO, bool Lazy, DWARFSectionKind SectionKind);
174+
bool IsDWO, DWARFSectionKind SectionKind);
181175
};
182176

183177
/// Represents base address of the CU.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ class ThreadUnsafeDWARFContextState : public DWARFContext::DWARFContextState {
286286
DWARFContext::DWARFContextState(DC),
287287
DWPName(std::move(DWP)) {}
288288

289-
DWARFUnitVector &getNormalUnits() override {
289+
const DWARFUnitVector &getNormalUnits() override {
290290
if (NormalUnits.empty()) {
291291
const DWARFObject &DObj = D.getDWARFObj();
292292
DObj.forEachInfoSections([&](const DWARFSection &S) {
@@ -300,16 +300,16 @@ class ThreadUnsafeDWARFContextState : public DWARFContext::DWARFContextState {
300300
return NormalUnits;
301301
}
302302

303-
DWARFUnitVector &getDWOUnits(bool Lazy) override {
303+
const DWARFUnitVector &getDWOUnits() override {
304304
if (DWOUnits.empty()) {
305305
const DWARFObject &DObj = D.getDWARFObj();
306306

307307
DObj.forEachInfoDWOSections([&](const DWARFSection &S) {
308-
DWOUnits.addUnitsForDWOSection(D, S, DW_SECT_INFO, Lazy);
308+
DWOUnits.addUnitsForDWOSection(D, S, DW_SECT_INFO);
309309
});
310310
DWOUnits.finishedInfoUnits();
311311
DObj.forEachTypesDWOSections([&](const DWARFSection &S) {
312-
DWOUnits.addUnitsForDWOSection(D, S, DW_SECT_EXT_TYPES, Lazy);
312+
DWOUnits.addUnitsForDWOSection(D, S, DW_SECT_EXT_TYPES);
313313
});
314314
}
315315
return DWOUnits;
@@ -633,13 +633,13 @@ class ThreadSafeState : public ThreadUnsafeDWARFContextState {
633633
ThreadSafeState(DWARFContext &DC, std::string &DWP) :
634634
ThreadUnsafeDWARFContextState(DC, DWP) {}
635635

636-
DWARFUnitVector &getNormalUnits() override {
636+
const DWARFUnitVector &getNormalUnits() override {
637637
std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
638638
return ThreadUnsafeDWARFContextState::getNormalUnits();
639639
}
640-
DWARFUnitVector &getDWOUnits(bool Lazy) override {
640+
const DWARFUnitVector &getDWOUnits() override {
641641
std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
642-
return ThreadUnsafeDWARFContextState::getDWOUnits(Lazy);
642+
return ThreadUnsafeDWARFContextState::getDWOUnits();
643643
}
644644
const DWARFUnitIndex &getCUIndex() override {
645645
std::unique_lock<std::recursive_mutex> LockGuard(Mutex);
@@ -1335,7 +1335,7 @@ void DWARFContext::dump(
13351335

13361336
DWARFTypeUnit *DWARFContext::getTypeUnitForHash(uint16_t Version, uint64_t Hash,
13371337
bool IsDWO) {
1338-
DWARFUnitVector &DWOUnits = State->getDWOUnits();
1338+
const DWARFUnitVector &DWOUnits = State->getDWOUnits();
13391339
if (const auto &TUI = getTUIndex()) {
13401340
if (const auto *R = TUI.getFromHash(Hash))
13411341
return dyn_cast_or_null<DWARFTypeUnit>(
@@ -1350,7 +1350,7 @@ DWARFTypeUnit *DWARFContext::getTypeUnitForHash(uint16_t Version, uint64_t Hash,
13501350
}
13511351

13521352
DWARFCompileUnit *DWARFContext::getDWOCompileUnitForHash(uint64_t Hash) {
1353-
DWARFUnitVector &DWOUnits = State->getDWOUnits(LazyParse);
1353+
const DWARFUnitVector &DWOUnits = State->getDWOUnits();
13541354

13551355
if (const auto &CUI = getCUIndex()) {
13561356
if (const auto *R = CUI.getFromHash(Hash))
@@ -1497,8 +1497,8 @@ void DWARFContext::clearLineTableForUnit(DWARFUnit *U) {
14971497
return State->clearLineTableForUnit(U);
14981498
}
14991499

1500-
DWARFUnitVector &DWARFContext::getDWOUnits(bool Lazy) {
1501-
return State->getDWOUnits(Lazy);
1500+
const DWARFUnitVector &DWARFContext::getDWOUnits() {
1501+
return State->getDWOUnits();
15021502
}
15031503

15041504
DWARFCompileUnit *DWARFContext::getCompileUnitForOffset(uint64_t Offset) {
@@ -1526,7 +1526,7 @@ DWARFCompileUnit *DWARFContext::getCompileUnitForDataAddress(uint64_t Address) {
15261526
//
15271527
// So, we walk the CU's and their child DI's manually, looking for the
15281528
// specific global variable.
1529-
for (std::unique_ptr<DWARFUnit> &CU : compile_units()) {
1529+
for (const std::unique_ptr<DWARFUnit> &CU : compile_units()) {
15301530
if (CU->getVariableForAddress(Address)) {
15311531
return static_cast<DWARFCompileUnit *>(CU.get());
15321532
}

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

Lines changed: 49 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -44,76 +44,69 @@ void DWARFUnitVector::addUnitsForSection(DWARFContext &C,
4444
DWARFSectionKind SectionKind) {
4545
const DWARFObject &D = C.getDWARFObj();
4646
addUnitsImpl(C, D, Section, C.getDebugAbbrev(), &D.getRangesSection(),
47-
&D.getLocSection(), D.getStrSection(),
48-
D.getStrOffsetsSection(), &D.getAddrSection(),
49-
D.getLineSection(), D.isLittleEndian(), false, false,
50-
SectionKind);
47+
&D.getLocSection(), D.getStrSection(), D.getStrOffsetsSection(),
48+
&D.getAddrSection(), D.getLineSection(), D.isLittleEndian(),
49+
false, SectionKind);
5150
}
5251

5352
void DWARFUnitVector::addUnitsForDWOSection(DWARFContext &C,
5453
const DWARFSection &DWOSection,
55-
DWARFSectionKind SectionKind,
56-
bool Lazy) {
54+
DWARFSectionKind SectionKind) {
5755
const DWARFObject &D = C.getDWARFObj();
58-
addUnitsImpl(C, D, DWOSection, C.getDebugAbbrevDWO(), &D.getRangesDWOSection(),
59-
&D.getLocDWOSection(), D.getStrDWOSection(),
60-
D.getStrOffsetsDWOSection(), &D.getAddrSection(),
61-
D.getLineDWOSection(), C.isLittleEndian(), true, Lazy,
62-
SectionKind);
56+
addUnitsImpl(C, D, DWOSection, C.getDebugAbbrevDWO(),
57+
&D.getRangesDWOSection(), &D.getLocDWOSection(),
58+
D.getStrDWOSection(), D.getStrOffsetsDWOSection(),
59+
&D.getAddrSection(), D.getLineDWOSection(), C.isLittleEndian(),
60+
true, SectionKind);
6361
}
6462

6563
void DWARFUnitVector::addUnitsImpl(
6664
DWARFContext &Context, const DWARFObject &Obj, const DWARFSection &Section,
6765
const DWARFDebugAbbrev *DA, const DWARFSection *RS,
6866
const DWARFSection *LocSection, StringRef SS, const DWARFSection &SOS,
6967
const DWARFSection *AOS, const DWARFSection &LS, bool LE, bool IsDWO,
70-
bool Lazy, DWARFSectionKind SectionKind) {
68+
DWARFSectionKind SectionKind) {
7169
DWARFDataExtractor Data(Obj, Section, LE, 0);
72-
// Lazy initialization of Parser, now that we have all section info.
73-
if (!Parser) {
74-
Parser = [=, &Context, &Obj, &Section, &SOS,
75-
&LS](uint64_t Offset, DWARFSectionKind SectionKind,
76-
const DWARFSection *CurSection,
77-
const DWARFUnitIndex::Entry *IndexEntry)
78-
-> std::unique_ptr<DWARFUnit> {
79-
const DWARFSection &InfoSection = CurSection ? *CurSection : Section;
80-
DWARFDataExtractor Data(Obj, InfoSection, LE, 0);
81-
if (!Data.isValidOffset(Offset))
82-
return nullptr;
83-
DWARFUnitHeader Header;
84-
if (Error ExtractErr =
85-
Header.extract(Context, Data, &Offset, SectionKind)) {
86-
Context.getWarningHandler()(std::move(ExtractErr));
87-
return nullptr;
88-
}
89-
if (!IndexEntry && IsDWO) {
90-
const DWARFUnitIndex &Index = getDWARFUnitIndex(
91-
Context, Header.isTypeUnit() ? DW_SECT_EXT_TYPES : DW_SECT_INFO);
92-
if (Index) {
93-
if (Header.isTypeUnit())
94-
IndexEntry = Index.getFromHash(Header.getTypeHash());
95-
else if (auto DWOId = Header.getDWOId())
96-
IndexEntry = Index.getFromHash(*DWOId);
97-
}
98-
if (!IndexEntry)
99-
IndexEntry = Index.getFromOffset(Header.getOffset());
70+
auto Parser = [=, &Context, &Obj, &Section, &SOS,
71+
&LS](uint64_t Offset, DWARFSectionKind SectionKind,
72+
const DWARFSection *CurSection,
73+
const DWARFUnitIndex::Entry *IndexEntry)
74+
-> std::unique_ptr<DWARFUnit> {
75+
const DWARFSection &InfoSection = CurSection ? *CurSection : Section;
76+
DWARFDataExtractor Data(Obj, InfoSection, LE, 0);
77+
if (!Data.isValidOffset(Offset))
78+
return nullptr;
79+
DWARFUnitHeader Header;
80+
if (Error ExtractErr =
81+
Header.extract(Context, Data, &Offset, SectionKind)) {
82+
Context.getWarningHandler()(std::move(ExtractErr));
83+
return nullptr;
84+
}
85+
if (!IndexEntry && IsDWO) {
86+
const DWARFUnitIndex &Index = getDWARFUnitIndex(
87+
Context, Header.isTypeUnit() ? DW_SECT_EXT_TYPES : DW_SECT_INFO);
88+
if (Index) {
89+
if (Header.isTypeUnit())
90+
IndexEntry = Index.getFromHash(Header.getTypeHash());
91+
else if (auto DWOId = Header.getDWOId())
92+
IndexEntry = Index.getFromHash(*DWOId);
10093
}
101-
if (IndexEntry && !Header.applyIndexEntry(IndexEntry))
102-
return nullptr;
103-
std::unique_ptr<DWARFUnit> U;
104-
if (Header.isTypeUnit())
105-
U = std::make_unique<DWARFTypeUnit>(Context, InfoSection, Header, DA,
94+
if (!IndexEntry)
95+
IndexEntry = Index.getFromOffset(Header.getOffset());
96+
}
97+
if (IndexEntry && !Header.applyIndexEntry(IndexEntry))
98+
return nullptr;
99+
std::unique_ptr<DWARFUnit> U;
100+
if (Header.isTypeUnit())
101+
U = std::make_unique<DWARFTypeUnit>(Context, InfoSection, Header, DA, RS,
102+
LocSection, SS, SOS, AOS, LS, LE,
103+
IsDWO, *this);
104+
else
105+
U = std::make_unique<DWARFCompileUnit>(Context, InfoSection, Header, DA,
106106
RS, LocSection, SS, SOS, AOS, LS,
107107
LE, IsDWO, *this);
108-
else
109-
U = std::make_unique<DWARFCompileUnit>(Context, InfoSection, Header,
110-
DA, RS, LocSection, SS, SOS,
111-
AOS, LS, LE, IsDWO, *this);
112-
return U;
113-
};
114-
}
115-
if (Lazy)
116-
return;
108+
return U;
109+
};
117110
// Find a reasonable insertion point within the vector. We skip over
118111
// (a) units from a different section, (b) units from the same section
119112
// but with lower offset-within-section. This keeps units in order
@@ -136,15 +129,6 @@ void DWARFUnitVector::addUnitsImpl(
136129
}
137130
}
138131

139-
DWARFUnit *DWARFUnitVector::addUnit(std::unique_ptr<DWARFUnit> Unit) {
140-
auto I = llvm::upper_bound(*this, Unit,
141-
[](const std::unique_ptr<DWARFUnit> &LHS,
142-
const std::unique_ptr<DWARFUnit> &RHS) {
143-
return LHS->getOffset() < RHS->getOffset();
144-
});
145-
return this->insert(I, std::move(Unit))->get();
146-
}
147-
148132
DWARFUnit *DWARFUnitVector::getUnitForOffset(uint64_t Offset) const {
149133
auto end = begin() + getNumInfoUnits();
150134
auto *CU =
@@ -158,7 +142,7 @@ DWARFUnit *DWARFUnitVector::getUnitForOffset(uint64_t Offset) const {
158142
}
159143

160144
DWARFUnit *
161-
DWARFUnitVector::getUnitForIndexEntry(const DWARFUnitIndex::Entry &E) {
145+
DWARFUnitVector::getUnitForIndexEntry(const DWARFUnitIndex::Entry &E) const {
162146
const auto *CUOff = E.getContribution(DW_SECT_INFO);
163147
if (!CUOff)
164148
return nullptr;
@@ -174,17 +158,7 @@ DWARFUnitVector::getUnitForIndexEntry(const DWARFUnitIndex::Entry &E) {
174158
if (CU != end && (*CU)->getOffset() <= Offset)
175159
return CU->get();
176160

177-
if (!Parser)
178-
return nullptr;
179-
180-
auto U = Parser(Offset, DW_SECT_INFO, nullptr, &E);
181-
if (!U)
182-
return nullptr;
183-
184-
auto *NewCU = U.get();
185-
this->insert(CU, std::move(U));
186-
++NumInfoUnits;
187-
return NewCU;
161+
return nullptr;
188162
}
189163

190164
DWARFUnit::DWARFUnit(DWARFContext &DC, const DWARFSection &Section,

llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class ObjFileAddressMap : public AddressMapBase {
5555
}
5656

5757
// Check CU address ranges for tombstone value.
58-
for (std::unique_ptr<DWARFUnit> &CU : Context.compile_units()) {
58+
for (const std::unique_ptr<DWARFUnit> &CU : Context.compile_units()) {
5959
Expected<llvm::DWARFAddressRangesVector> ARanges =
6060
CU->getUnitDIE().getAddressRanges();
6161
if (!ARanges) {

0 commit comments

Comments
 (0)