Skip to content

Commit f4a456d

Browse files
Kevin Freikevinfrei
authored andcommitted
Using a helper to make the double checked locking more legible
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D57863981
1 parent 02301bc commit f4a456d

File tree

2 files changed

+58
-24
lines changed

2 files changed

+58
-24
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,10 @@ class DWARFUnit {
585585
/// hasn't already been done
586586
void extractDIEsIfNeeded(bool CUDieOnly);
587587

588+
/// extractAllDIEsHelper - helper to be invoked *only* from inside
589+
/// tryExtractDIEsIfNeeded, which holds correct locks.
590+
Error extractAllDIEsHelper();
591+
588592
/// extractDIEsToVector - Appends all parsed DIEs to a vector.
589593
void extractDIEsToVector(bool AppendCUDie, bool AppendNonCUDIEs,
590594
std::vector<DWARFDebugInfoEntry> &DIEs) const;

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -495,36 +495,66 @@ void DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
495495
Context.getRecoverableErrorHandler()(std::move(e));
496496
}
497497

498-
Error DWARFUnit::tryExtractDIEsIfNeeded(bool CUDieOnly) {
499-
llvm::sys::ScopedReader FreeLock(CUDieFreeMutex);
498+
static bool DoubleCheckedRWLocker(llvm::sys::RWMutex &Mutex,
499+
const std::function<bool()> &reader,
500+
const std::function<void()> &writer) {
500501
{
501-
llvm::sys::ScopedReader Lock(CUDieArrayMutex);
502-
if ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1)
503-
return Error::success(); // Already parsed.
502+
llvm::sys::ScopedReader Lock(Mutex);
503+
if (reader())
504+
return true;
504505
}
506+
llvm::sys::ScopedWriter Lock(Mutex);
507+
if (reader())
508+
return true;
509+
// If we get here, then the reader function returned false. This means that
510+
// no one else is currently writing to this data structure and it's safe for
511+
// us to write to it now. We can use a scoped writer lock since there are no
512+
// other readers or writers at this point.
513+
writer();
514+
return false;
515+
}
516+
517+
Error DWARFUnit::tryExtractDIEsIfNeeded(bool CUDieOnly) {
518+
llvm::sys::ScopedReader FreeLock(CUDieFreeMutex);
505519
bool HasCUDie = false;
506-
{
507-
llvm::sys::ScopedWriter Lock(CUDieArrayMutex);
508-
if ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1)
509-
return Error::success(); // Already parsed.
510-
HasCUDie = !DieArray.empty();
511-
extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
512-
}
513-
{
514-
llvm::sys::ScopedReader Lock(AllDieArrayMutex);
515-
if (DieArray.empty())
516-
return Error::success();
520+
Error Result = Error::success();
517521

518-
if (HasCUDie)
519-
return Error::success();
520-
}
521-
llvm::sys::ScopedWriter Lock(AllDieArrayMutex);
522-
if (DieArray.empty())
523-
return Error::success();
522+
// Lambda to check if the CU DIE has been extracted already.
523+
auto CheckIfCUDieExtracted = [this, CUDieOnly]() {
524+
// True means already parsed.
525+
return ((CUDieOnly && !DieArray.empty()) || DieArray.size() > 1);
526+
};
524527

525-
if (HasCUDie)
526-
return Error::success();
528+
// Lambda to extract the CU DIE.
529+
auto ExtractCUDie = [this, CUDieOnly, &HasCUDie]() {
530+
HasCUDie = !DieArray.empty();
531+
extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
532+
};
533+
534+
// Safely check if the CU DIE has been extract already. If not, then extract
535+
// it.
536+
if (DoubleCheckedRWLocker(CUDieArrayMutex, CheckIfCUDieExtracted,
537+
ExtractCUDie))
538+
return Result;
539+
540+
// Lambda to check if all DIEs have been extract already.
541+
auto CheckIfAllDiesExtracted = [this, HasCUDie]() {
542+
return (DieArray.empty() || HasCUDie);
543+
};
544+
545+
// Lambda to extract all the DIEs using the helper function
546+
auto ExtractAllDies = [this, &Result]() { Result = extractAllDIEsHelper(); };
547+
548+
// Safely check if *all* the DIEs have been parsed already. If not, then parse
549+
// them.
550+
DoubleCheckedRWLocker(AllDieArrayMutex, CheckIfAllDiesExtracted,
551+
ExtractAllDies);
552+
return Result;
553+
}
527554

555+
// This should only be used from the tryExtractDIEsIfNeeded function:
556+
// it must already have all the proper locks acquired.
557+
Error DWARFUnit::extractAllDIEsHelper() {
528558
// If CU DIE was just parsed, copy several attribute values from it.
529559
DWARFDie UnitDie(this, &DieArray[0]);
530560
if (std::optional<uint64_t> DWOId =

0 commit comments

Comments
 (0)