[Lldb-commits] [lldb] r333987 - Protect DWARFCompileUnit::m_die_array by new mutexes
Jan Kratochvil via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 5 01:52:18 PDT 2018
Author: jankratochvil
Date: Tue Jun 5 01:52:18 2018
New Revision: 333987
URL: http://llvm.org/viewvc/llvm-project?rev=333987&view=rev
Log:
Protect DWARFCompileUnit::m_die_array by new mutexes
If BuildAddressRangeTable called ExtractDIEsIfNeeded(false), then another
thread started processing data from m_die_array and then the first thread
called final ClearDIEs() the second thread would crash.
It is also required without multithreaded debugger using DW_TAG_partial_unit
for DWZ.
Differential revision: https://reviews.llvm.org/D40470
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp?rev=333987&r1=333986&r2=333987&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Tue Jun 5 01:52:18 2018
@@ -31,7 +31,8 @@ using namespace std;
extern int g_verbose;
-DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf) : m_dwarf(dwarf) {}
+DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf)
+ : m_dwarf(dwarf), m_cancel_scopes(false) {}
DWARFUnit::~DWARFUnit() {}
@@ -39,6 +40,12 @@ DWARFUnit::~DWARFUnit() {}
// Parses first DIE of a compile unit.
//----------------------------------------------------------------------
void DWARFUnit::ExtractUnitDIEIfNeeded() {
+ {
+ llvm::sys::ScopedReader lock(m_first_die_mutex);
+ if (m_first_die)
+ return; // Already parsed
+ }
+ llvm::sys::ScopedWriter lock(m_first_die_mutex);
if (m_first_die)
return; // Already parsed
@@ -67,10 +74,88 @@ void DWARFUnit::ExtractUnitDIEIfNeeded()
//----------------------------------------------------------------------
// Parses a compile unit and indexes its DIEs if it hasn't already been done.
+// It will leave this compile unit extracted forever.
//----------------------------------------------------------------------
-bool DWARFUnit::ExtractDIEsIfNeeded() {
+void DWARFUnit::ExtractDIEsIfNeeded() {
+ m_cancel_scopes = true;
+
+ {
+ llvm::sys::ScopedReader lock(m_die_array_mutex);
+ if (!m_die_array.empty())
+ return; // Already parsed
+ }
+ llvm::sys::ScopedWriter lock(m_die_array_mutex);
if (!m_die_array.empty())
- return 0; // Already parsed
+ return; // Already parsed
+
+ ExtractDIEsRWLocked();
+}
+
+//----------------------------------------------------------------------
+// Parses a compile unit and indexes its DIEs if it hasn't already been done.
+// It will clear this compile unit after returned instance gets out of scope,
+// no other ScopedExtractDIEs instance is running for this compile unit
+// and no ExtractDIEsIfNeeded() has been executed during this ScopedExtractDIEs
+// lifetime.
+//----------------------------------------------------------------------
+DWARFUnit::ScopedExtractDIEs DWARFUnit::ExtractDIEsScoped() {
+ ScopedExtractDIEs scoped(this);
+
+ {
+ llvm::sys::ScopedReader lock(m_die_array_mutex);
+ if (!m_die_array.empty())
+ return std::move(scoped); // Already parsed
+ }
+ llvm::sys::ScopedWriter lock(m_die_array_mutex);
+ if (!m_die_array.empty())
+ return std::move(scoped); // Already parsed
+
+ // Otherwise m_die_array would be already populated.
+ lldbassert(!m_cancel_scopes);
+
+ ExtractDIEsRWLocked();
+ scoped.m_clear_dies = true;
+ return scoped;
+}
+
+DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(DWARFUnit *cu) : m_cu(cu) {
+ lldbassert(m_cu);
+ m_cu->m_die_array_scoped_mutex.lock_shared();
+}
+
+DWARFUnit::ScopedExtractDIEs::~ScopedExtractDIEs() {
+ if (!m_cu)
+ return;
+ m_cu->m_die_array_scoped_mutex.unlock_shared();
+ if (!m_clear_dies || m_cu->m_cancel_scopes)
+ return;
+ // Be sure no other ScopedExtractDIEs is running anymore.
+ llvm::sys::ScopedWriter lock_scoped(m_cu->m_die_array_scoped_mutex);
+ llvm::sys::ScopedWriter lock(m_cu->m_die_array_mutex);
+ if (m_cu->m_cancel_scopes)
+ return;
+ m_cu->ClearDIEsRWLocked();
+}
+
+DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(ScopedExtractDIEs &&rhs)
+ : m_cu(rhs.m_cu), m_clear_dies(rhs.m_clear_dies) {
+ rhs.m_cu = nullptr;
+}
+
+DWARFUnit::ScopedExtractDIEs &DWARFUnit::ScopedExtractDIEs::operator=(
+ DWARFUnit::ScopedExtractDIEs &&rhs) {
+ m_cu = rhs.m_cu;
+ rhs.m_cu = nullptr;
+ m_clear_dies = rhs.m_clear_dies;
+ return *this;
+}
+
+//----------------------------------------------------------------------
+// Parses a compile unit and indexes its DIEs, m_die_array_mutex must be
+// held R/W and m_die_array must be empty.
+//----------------------------------------------------------------------
+void DWARFUnit::ExtractDIEsRWLocked() {
+ llvm::sys::ScopedWriter first_die_lock(m_first_die_mutex);
static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
Timer scoped_timer(
@@ -188,8 +273,6 @@ bool DWARFUnit::ExtractDIEsIfNeeded() {
DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
dwo_cu->ExtractDIEsIfNeeded();
}
-
- return true;
}
//--------------------------------------------------------------------------
@@ -218,6 +301,7 @@ void DWARFUnit::ExtractDIEsEndCheck(lldb
}
}
+// m_die_array_mutex must be already held as read/write.
void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry &cu_die) {
uint64_t base_addr = cu_die.GetAttributeValueAsAddress(
m_dwarf, this, DW_AT_low_pc, LLDB_INVALID_ADDRESS);
@@ -271,11 +355,14 @@ size_t DWARFUnit::AppendDIEsWithTag(cons
DWARFDIECollection &dies,
uint32_t depth) const {
size_t old_size = dies.Size();
- DWARFDebugInfoEntry::const_iterator pos;
- DWARFDebugInfoEntry::const_iterator end = m_die_array.end();
- for (pos = m_die_array.begin(); pos != end; ++pos) {
- if (pos->Tag() == tag)
- dies.Append(DWARFDIE(this, &(*pos)));
+ {
+ llvm::sys::ScopedReader lock(m_die_array_mutex);
+ DWARFDebugInfoEntry::const_iterator pos;
+ DWARFDebugInfoEntry::const_iterator end = m_die_array.end();
+ for (pos = m_die_array.begin(); pos != end; ++pos) {
+ if (pos->Tag() == tag)
+ dies.Append(DWARFDIE(this, &(*pos)));
+ }
}
// Return the number of DIEs added to the collection
@@ -316,12 +403,13 @@ void DWARFUnit::SetAddrBase(dw_addr_t ad
m_base_obj_offset = base_obj_offset;
}
-void DWARFUnit::ClearDIEs() {
+// It may be called only with m_die_array_mutex held R/W.
+void DWARFUnit::ClearDIEsRWLocked() {
m_die_array.clear();
m_die_array.shrink_to_fit();
if (m_dwo_symbol_file)
- m_dwo_symbol_file->GetCompileUnit()->ClearDIEs();
+ m_dwo_symbol_file->GetCompileUnit()->ClearDIEsRWLocked();
}
void DWARFUnit::BuildAddressRangeTable(SymbolFileDWARF *dwarf,
@@ -359,7 +447,7 @@ void DWARFUnit::BuildAddressRangeTable(S
// If the DIEs weren't parsed, then we don't want all dies for all compile
// units to stay loaded when they weren't needed. So we can end up parsing
// the DWARF and then throwing them all away to keep memory usage down.
- const bool clear_dies = ExtractDIEsIfNeeded();
+ ScopedExtractDIEs clear_dies(ExtractDIEsScoped());
die = DIEPtr();
if (die)
@@ -415,11 +503,6 @@ void DWARFUnit::BuildAddressRangeTable(S
}
}
}
-
- // Keep memory down by clearing DIEs if this generate function caused them to
- // be parsed
- if (clear_dies)
- ClearDIEs();
}
lldb::ByteOrder DWARFUnit::GetByteOrder() const {
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h?rev=333987&r1=333986&r2=333987&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h Tue Jun 5 01:52:18 2018
@@ -13,6 +13,7 @@
#include "DWARFDIE.h"
#include "DWARFDebugInfoEntry.h"
#include "lldb/lldb-enumerations.h"
+#include "llvm/Support/RWMutex.h"
class DWARFUnit;
class DWARFCompileUnit;
@@ -40,7 +41,20 @@ public:
virtual ~DWARFUnit();
void ExtractUnitDIEIfNeeded();
- bool ExtractDIEsIfNeeded();
+ void ExtractDIEsIfNeeded();
+
+ class ScopedExtractDIEs {
+ DWARFUnit *m_cu;
+ public:
+ bool m_clear_dies = false;
+ ScopedExtractDIEs(DWARFUnit *cu);
+ ~ScopedExtractDIEs();
+ DISALLOW_COPY_AND_ASSIGN(ScopedExtractDIEs);
+ ScopedExtractDIEs(ScopedExtractDIEs &&rhs);
+ ScopedExtractDIEs &operator=(ScopedExtractDIEs &&rhs);
+ };
+ ScopedExtractDIEs ExtractDIEsScoped();
+
DWARFDIE LookupAddress(const dw_addr_t address);
size_t AppendDIEsWithTag(const dw_tag_t tag,
DWARFDIECollection &matching_dies,
@@ -100,7 +114,6 @@ public:
dw_addr_t GetRangesBase() const { return m_ranges_base; }
void SetAddrBase(dw_addr_t addr_base, dw_addr_t ranges_base,
dw_offset_t base_obj_offset);
- void ClearDIEs();
void BuildAddressRangeTable(SymbolFileDWARF *dwarf,
DWARFDebugAranges *debug_aranges);
@@ -172,10 +185,17 @@ protected:
void *m_user_data = nullptr;
// The compile unit debug information entry item
DWARFDebugInfoEntry::collection m_die_array;
+ mutable llvm::sys::RWMutex m_die_array_mutex;
+ // It is used for tracking of ScopedExtractDIEs instances.
+ mutable llvm::sys::RWMutex m_die_array_scoped_mutex;
+ // ScopedExtractDIEs instances should not call ClearDIEsRWLocked()
+ // as someone called ExtractDIEsIfNeeded().
+ std::atomic<bool> m_cancel_scopes;
// GetUnitDIEPtrOnly() needs to return pointer to the first DIE.
// But the first element of m_die_array after ExtractUnitDIEIfNeeded()
// would possibly move in memory after later ExtractDIEsIfNeeded().
DWARFDebugInfoEntry m_first_die;
+ llvm::sys::RWMutex m_first_die_mutex;
// A table similar to the .debug_aranges table, but this one points to the
// exact DW_TAG_subprogram DIEs
std::unique_ptr<DWARFDebugAranges> m_func_aranges_ap;
@@ -201,11 +221,14 @@ protected:
private:
void ParseProducerInfo();
+ void ExtractDIEsRWLocked();
+ void ClearDIEsRWLocked();
// Get the DWARF unit DWARF debug informration entry. Parse the single DIE
// if needed.
const DWARFDebugInfoEntry *GetUnitDIEPtrOnly() {
ExtractUnitDIEIfNeeded();
+ // m_first_die_mutex is not required as m_first_die is never cleared.
if (!m_first_die)
return NULL;
return &m_first_die;
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp?rev=333987&r1=333986&r2=333987&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp Tue Jun 5 01:52:18 2018
@@ -38,9 +38,12 @@ void ManualDWARFIndex::Index() {
std::vector<IndexSet> sets(num_compile_units);
- // std::vector<bool> might be implemented using bit test-and-set, so use
- // uint8_t instead.
- std::vector<uint8_t> clear_cu_dies(num_compile_units, false);
+ //----------------------------------------------------------------------
+ // Keep memory down by clearing DIEs for any compile units if indexing
+ // caused us to load the compile unit's DIEs.
+ //----------------------------------------------------------------------
+ std::vector<llvm::Optional<DWARFUnit::ScopedExtractDIEs>>
+ clear_cu_dies(num_compile_units);
auto parser_fn = [&](size_t cu_idx) {
DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
if (dwarf_cu)
@@ -49,12 +52,8 @@ void ManualDWARFIndex::Index() {
auto extract_fn = [&debug_info, &clear_cu_dies](size_t cu_idx) {
DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
- if (dwarf_cu) {
- // dwarf_cu->ExtractDIEsIfNeeded() will return false if the DIEs
- // for a compile unit have already been parsed.
- if (dwarf_cu->ExtractDIEsIfNeeded())
- clear_cu_dies[cu_idx] = true;
- }
+ if (dwarf_cu)
+ clear_cu_dies[cu_idx] = dwarf_cu->ExtractDIEsScoped();
};
// Create a task runner that extracts dies for each DWARF compile unit in a
@@ -89,15 +88,6 @@ void ManualDWARFIndex::Index() {
[&]() { finalize_fn(&IndexSet::globals); },
[&]() { finalize_fn(&IndexSet::types); },
[&]() { finalize_fn(&IndexSet::namespaces); });
-
- //----------------------------------------------------------------------
- // Keep memory down by clearing DIEs for any compile units if indexing
- // caused us to load the compile unit's DIEs.
- //----------------------------------------------------------------------
- for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx) {
- if (clear_cu_dies[cu_idx])
- debug_info.GetCompileUnitAtIndex(cu_idx)->ClearDIEs();
- }
}
void ManualDWARFIndex::IndexUnit(DWARFUnit &unit, IndexSet &set) {
More information about the lldb-commits
mailing list