jankratochvil created this revision.
jankratochvil added reviewers: clayborg, labath.
Herald added subscribers: JDevlieghere, aprantl.
+ // GetUnitDIEPtrOnly() needs to return pointer to the first DIE.
+ // But the first element of m_die_array after ExtractDIEsIfNeeded(true)
+ // may move in memory after later ExtractDIEsIfNeeded(false).
I haven't tried to reproduce it. `DWARFDebugInfoEntry::collection m_die_array`
is `std::vector`, its data may move during its expansion.
I would not mind but I have found I cannot make the code thread-safe for
https://reviews.llvm.org/D40470 when it looks already incorrect to me.
https://reviews.llvm.org/D46810
Files:
source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
source/Plugins/SymbolFile/DWARF/DWARFUnit.h
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -170,6 +170,13 @@
void *m_user_data = nullptr;
// The compile unit debug information entry item
DWARFDebugInfoEntry::collection m_die_array;
+ // GetUnitDIEPtrOnly() needs to return pointer to the first DIE.
+ // But the first element of m_die_array after ExtractDIEsIfNeeded(true)
+ // may move in memory after later ExtractDIEsIfNeeded(false).
+ DWARFDebugInfoEntry m_first_die;
+ // m_die_array_size() is like m_die_array.size() but
+ // it considers also m_die_array.empty() with m_first_die.
+ size_t m_die_array_size() const;
// 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;
@@ -208,9 +215,9 @@
// if needed.
const DWARFDebugInfoEntry *GetUnitDIEPtrOnly() {
ExtractDIEsIfNeeded(true);
- if (m_die_array.empty())
+ if (!m_first_die)
return NULL;
- return &m_die_array[0];
+ return &m_first_die;
}
// Get all DWARF debug informration entries. Parse all DIEs if needed.
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -41,7 +41,7 @@
// Parses a compile unit and indexes its DIEs if it hasn't already been done.
//----------------------------------------------------------------------
size_t DWARFUnit::ExtractDIEsIfNeeded(bool cu_die_only) {
- const size_t initial_die_array_size = m_die_array.size();
+ const size_t initial_die_array_size = m_die_array_size();
if ((cu_die_only && initial_die_array_size > 0) || initial_die_array_size > 1)
return 0; // Already parsed
@@ -93,6 +93,22 @@
if (depth == 0) {
if (initial_die_array_size == 0)
AddUnitDIE(die);
+
+ if (!cu_die_only) {
+ assert(m_die_array.empty() && "Compile unit DIE already added");
+
+ // The average bytes per DIE entry has been seen to be around 14-20 so
+ // lets pre-reserve half of that since we are now stripping the NULL
+ // tags.
+
+ // Only reserve the memory if we are adding children of the main
+ // compile unit DIE. The compile unit DIE is always the first entry, so
+ // if our size is 1, then we are adding the first compile unit child
+ // DIE and should reserve the memory.
+ m_die_array.reserve(GetDebugInfoSize() / 24);
+ m_die_array.push_back(die);
+ }
+
uint64_t base_addr = die.GetAttributeValueAsAddress(
m_dwarf, this, DW_AT_low_pc, LLDB_INVALID_ADDRESS);
if (base_addr == LLDB_INVALID_ADDRESS)
@@ -102,14 +118,17 @@
if (cu_die_only)
return 1;
} else {
+ assert(!cu_die_only);
if (null_die) {
if (prev_die_had_children) {
// This will only happen if a DIE says is has children but all it
// contains is a NULL tag. Since we are removing the NULL DIEs from
// the list (saves up to 25% in C++ code), we need a way to let the
// DIE know that it actually doesn't have children.
- if (!m_die_array.empty())
+ if (!m_die_array.empty()) {
m_die_array.back().SetEmptyChildren(true);
+ m_first_die.SetEmptyChildren(true);
+ }
}
} else {
die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]);
@@ -135,7 +154,7 @@
prev_die_had_children = false;
} else {
- die_index_stack.back() = m_die_array.size() - 1;
+ die_index_stack.back() = m_die_array_size() - 1;
// Normal DIE
const bool die_has_children = die.HasChildren();
if (die_has_children) {
@@ -169,36 +188,38 @@
if (log && log->GetVerbose()) {
StreamString strm;
Dump(&strm);
- if (m_die_array.empty())
+ if (!m_first_die)
strm.Printf("error: no DIE for compile unit");
else
- m_die_array[0].Dump(m_dwarf, this, strm, UINT32_MAX);
+ m_first_die.Dump(m_dwarf, this, strm, UINT32_MAX);
log->PutString(strm.GetString());
}
+ assert(m_die_array.empty() || !cu_die_only);
+ assert(m_die_array.empty() || m_die_array.front() == m_first_die);
+
if (!m_dwo_symbol_file)
- return m_die_array.size();
+ return m_die_array_size();
DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit();
size_t dwo_die_count = dwo_cu->ExtractDIEsIfNeeded(cu_die_only);
- return m_die_array.size() + dwo_die_count -
+ return m_die_array_size() + dwo_die_count -
1; // We have 2 CU die, but we want to count it only as one
}
+size_t DWARFUnit::m_die_array_size() const {
+ if (!m_die_array.empty())
+ return m_die_array.size();
+ return m_first_die ? 1 : 0;
+}
+
void DWARFUnit::AddUnitDIE(DWARFDebugInfoEntry &die) {
assert(m_die_array.empty() && "Compile unit DIE already added");
+ assert(!m_first_die);
+
+ m_first_die = die;
- // The average bytes per DIE entry has been seen to be around 14-20 so lets
- // pre-reserve half of that since we are now stripping the NULL tags.
-
- // Only reserve the memory if we are adding children of the main compile unit
- // DIE. The compile unit DIE is always the first entry, so if our size is 1,
- // then we are adding the first compile unit child DIE and should reserve the
- // memory.
- m_die_array.reserve(GetDebugInfoSize() / 24);
- m_die_array.push_back(die);
-
- const DWARFDebugInfoEntry &cu_die = m_die_array.front();
+ const DWARFDebugInfoEntry &cu_die = m_first_die;
std::unique_ptr<SymbolFileDWARFDwo> dwo_symbol_file =
m_dwarf->GetDwoSymbolFileForCompileUnit(*this, cu_die);
if (!dwo_symbol_file)
@@ -290,20 +311,8 @@
}
void DWARFUnit::ClearDIEs(bool keep_compile_unit_die) {
- if (m_die_array.size() > 1) {
- // std::vectors never get any smaller when resized to a smaller size, or
- // when clear() or erase() are called, the size will report that it is
- // smaller, but the memory allocated remains intact (call capacity() to see
- // this). So we need to create a temporary vector and swap the contents
- // which will cause just the internal pointers to be swapped so that when
- // "tmp_array" goes out of scope, it will destroy the contents.
-
- // Save at least the compile unit DIE
- DWARFDebugInfoEntry::collection tmp_array;
- m_die_array.swap(tmp_array);
- if (keep_compile_unit_die)
- m_die_array.push_back(tmp_array.front());
- }
+ m_die_array.clear();
+ m_die_array.shrink_to_fit();
if (m_dwo_symbol_file)
m_dwo_symbol_file->GetCompileUnit()->ClearDIEs(keep_compile_unit_die);
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -60,6 +60,10 @@
m_empty_children(false), m_abbr_idx(0), m_has_children(false),
m_tag(0) {}
+ explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
+ bool operator==(const DWARFDebugInfoEntry &rhs) const;
+ bool operator!=(const DWARFDebugInfoEntry &rhs) const;
+
void BuildAddressRangeTable(SymbolFileDWARF *dwarf2Data,
const DWARFUnit *cu,
DWARFDebugAranges *debug_aranges) const;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -1825,3 +1825,15 @@
die_ref.HasChildren() ? " *" : "");
}
}
+
+bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const {
+ return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx &&
+ m_sibling_idx == rhs.m_sibling_idx &&
+ m_empty_children == rhs.m_empty_children &&
+ m_abbr_idx == rhs.m_abbr_idx && m_has_children == rhs.m_has_children &&
+ m_tag == rhs.m_tag;
+}
+
+bool DWARFDebugInfoEntry::operator!=(const DWARFDebugInfoEntry &rhs) const {
+ return !(*this == rhs);
+}
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
