[Lldb-commits] [lldb] [lldb] Renaissance LineTable sequences (PR #127800)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Feb 19 06:15:56 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
LineSeqeunce is basically a vector, except that users aren't supposed to know that. This implements the same concept in a slightly simpler fashion.
---
Patch is 21.79 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127800.diff
7 Files Affected:
- (modified) lldb/include/lldb/Symbol/LineTable.h (+38-51)
- (modified) lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp (+5-7)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+3-4)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp (+5-6)
- (modified) lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (+12-14)
- (modified) lldb/source/Symbol/LineTable.cpp (+25-49)
- (modified) lldb/unittests/Symbol/LineTableTest.cpp (+7-12)
``````````diff
diff --git a/lldb/include/lldb/Symbol/LineTable.h b/lldb/include/lldb/Symbol/LineTable.h
index f66081b6ee110..2ce5d8cee6e95 100644
--- a/lldb/include/lldb/Symbol/LineTable.h
+++ b/lldb/include/lldb/Symbol/LineTable.h
@@ -20,25 +20,11 @@
namespace lldb_private {
-/// \class LineSequence LineTable.h "lldb/Symbol/LineTable.h" An abstract base
-/// class used during symbol table creation.
-class LineSequence {
-public:
- LineSequence();
-
- virtual ~LineSequence() = default;
-
- virtual void Clear() = 0;
-
-private:
- LineSequence(const LineSequence &) = delete;
- const LineSequence &operator=(const LineSequence &) = delete;
-};
-
/// \class LineTable LineTable.h "lldb/Symbol/LineTable.h"
/// A line table class.
class LineTable {
public:
+ class Sequence;
/// Construct with compile unit.
///
/// \param[in] comp_unit
@@ -49,8 +35,7 @@ class LineTable {
///
/// \param[in] sequences
/// Unsorted list of line sequences.
- LineTable(CompileUnit *comp_unit,
- std::vector<std::unique_ptr<LineSequence>> &&sequences);
+ LineTable(CompileUnit *comp_unit, std::vector<Sequence> &&sequences);
/// Destructor.
~LineTable();
@@ -73,20 +58,17 @@ class LineTable {
bool is_start_of_basic_block, bool is_prologue_end,
bool is_epilogue_begin, bool is_terminal_entry);
- // Used to instantiate the LineSequence helper class
- static std::unique_ptr<LineSequence> CreateLineSequenceContainer();
-
// Append an entry to a caller-provided collection that will later be
// inserted in this line table.
- static void AppendLineEntryToSequence(LineSequence *sequence, lldb::addr_t file_addr,
- uint32_t line, uint16_t column,
- uint16_t file_idx, bool is_start_of_statement,
- bool is_start_of_basic_block,
- bool is_prologue_end, bool is_epilogue_begin,
- bool is_terminal_entry);
+ static void
+ AppendLineEntryToSequence(Sequence &sequence, lldb::addr_t file_addr,
+ uint32_t line, uint16_t column, uint16_t file_idx,
+ bool is_start_of_statement,
+ bool is_start_of_basic_block, bool is_prologue_end,
+ bool is_epilogue_begin, bool is_terminal_entry);
// Insert a sequence of entries into this line table.
- void InsertSequence(LineSequence *sequence);
+ void InsertSequence(Sequence sequence);
/// Dump all line entries in this line table to the stream \a s.
///
@@ -273,17 +255,6 @@ class LineTable {
return 0;
}
- class LessThanBinaryPredicate {
- public:
- LessThanBinaryPredicate(LineTable *line_table);
- bool operator()(const LineTable::Entry &, const LineTable::Entry &) const;
- bool operator()(const std::unique_ptr<LineSequence> &,
- const std::unique_ptr<LineSequence> &) const;
-
- protected:
- LineTable *m_line_table;
- };
-
static bool EntryAddressLessThan(const Entry &lhs, const Entry &rhs) {
return lhs.file_addr < rhs.file_addr;
}
@@ -315,6 +286,35 @@ class LineTable {
uint16_t file_idx = 0;
};
+ class Sequence {
+ public:
+ Sequence() = default;
+ // Moving clears moved-from object so it can be used anew. Copying is
+ // generally an error. C++ doesn't guarantee that a moved-from vector is
+ // empty(), so we clear it explicitly.
+ Sequence(Sequence &&rhs) : m_entries(std::exchange(rhs.m_entries, {})) {}
+ Sequence &operator=(Sequence &&rhs) {
+ m_entries = std::exchange(rhs.m_entries, {});
+ return *this;
+ }
+ Sequence(const Sequence &) = delete;
+ Sequence &operator=(const Sequence &) = delete;
+
+ private:
+ std::vector<Entry> m_entries;
+ friend class LineTable;
+ };
+
+ class LessThanBinaryPredicate {
+ public:
+ LessThanBinaryPredicate(LineTable *line_table) : m_line_table(line_table) {}
+ bool operator()(const LineTable::Entry &, const LineTable::Entry &) const;
+ bool operator()(const Sequence &, const Sequence &) const;
+
+ protected:
+ LineTable *m_line_table;
+ };
+
protected:
struct EntrySearchInfo {
LineTable *line_table;
@@ -333,19 +333,6 @@ class LineTable {
entry_collection
m_entries; ///< The collection of line entries in this line table.
- // Helper class
- class LineSequenceImpl : public LineSequence {
- public:
- LineSequenceImpl() = default;
-
- ~LineSequenceImpl() override = default;
-
- void Clear() override;
-
- entry_collection
- m_entries; ///< The collection of line entries in this sequence.
- };
-
bool ConvertEntryAtIndexToLineEntry(uint32_t idx, LineEntry &line_entry);
private:
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index c7229568e1a0c..dee5a7ce2876d 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -837,18 +837,16 @@ void SymbolFileBreakpad::ParseLineTableAndSupportFiles(CompileUnit &cu,
"How did we create compile units without a base address?");
SupportFileMap map;
- std::vector<std::unique_ptr<LineSequence>> sequences;
- std::unique_ptr<LineSequence> line_seq_up =
- LineTable::CreateLineSequenceContainer();
+ std::vector<LineTable::Sequence> sequences;
+ LineTable::Sequence sequence;
std::optional<addr_t> next_addr;
auto finish_sequence = [&]() {
LineTable::AppendLineEntryToSequence(
- line_seq_up.get(), *next_addr, /*line=*/0, /*column=*/0,
+ sequence, *next_addr, /*line=*/0, /*column=*/0,
/*file_idx=*/0, /*is_start_of_statement=*/false,
/*is_start_of_basic_block=*/false, /*is_prologue_end=*/false,
/*is_epilogue_begin=*/false, /*is_terminal_entry=*/true);
- sequences.push_back(std::move(line_seq_up));
- line_seq_up = LineTable::CreateLineSequenceContainer();
+ sequences.push_back(std::move(sequence));
};
LineIterator It(*m_objfile_sp, Record::Func, data.bookmark),
@@ -870,7 +868,7 @@ void SymbolFileBreakpad::ParseLineTableAndSupportFiles(CompileUnit &cu,
finish_sequence();
}
LineTable::AppendLineEntryToSequence(
- line_seq_up.get(), record->Address, record->LineNum, /*column=*/0,
+ sequence, record->Address, record->LineNum, /*column=*/0,
map[record->FileNum], /*is_start_of_statement=*/true,
/*is_start_of_basic_block=*/false, /*is_prologue_end=*/false,
/*is_epilogue_begin=*/false, /*is_terminal_entry=*/false);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index a96757afabddf..58b544a9a137b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1232,7 +1232,7 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
// FIXME: Rather than parsing the whole line table and then copying it over
// into LLDB, we should explore using a callback to populate the line table
// while we parse to reduce memory usage.
- std::vector<std::unique_ptr<LineSequence>> sequences;
+ std::vector<LineTable::Sequence> sequences;
// The Sequences view contains only valid line sequences. Don't iterate over
// the Rows directly.
for (const llvm::DWARFDebugLine::Sequence &seq : line_table->Sequences) {
@@ -1242,12 +1242,11 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
// m_first_code_address declaration for more details on this.
if (seq.LowPC < m_first_code_address)
continue;
- std::unique_ptr<LineSequence> sequence =
- LineTable::CreateLineSequenceContainer();
+ LineTable::Sequence sequence;
for (unsigned idx = seq.FirstRowIndex; idx < seq.LastRowIndex; ++idx) {
const llvm::DWARFDebugLine::Row &row = line_table->Rows[idx];
LineTable::AppendLineEntryToSequence(
- sequence.get(), row.Address.Address, row.Line, row.Column, row.File,
+ sequence, row.Address.Address, row.Line, row.Column, row.File,
row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
row.EndSequence);
}
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 6338f12402b73..4e472d0a0b0f2 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1310,18 +1310,17 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) {
cii->m_global_line_table.Clear();
// Add line entries in line_set to line_table.
- auto line_table = std::make_unique<LineTable>(&comp_unit);
- std::unique_ptr<LineSequence> sequence(
- line_table->CreateLineSequenceContainer());
+ std::vector<LineTable::Sequence> sequence(1);
for (const auto &line_entry : line_set) {
- line_table->AppendLineEntryToSequence(
- sequence.get(), line_entry.file_addr, line_entry.line,
+ LineTable::AppendLineEntryToSequence(
+ sequence.back(), line_entry.file_addr, line_entry.line,
line_entry.column, line_entry.file_idx,
line_entry.is_start_of_statement, line_entry.is_start_of_basic_block,
line_entry.is_prologue_end, line_entry.is_epilogue_begin,
line_entry.is_terminal_entry);
}
- line_table->InsertSequence(sequence.get());
+ auto line_table =
+ std::make_unique<LineTable>(&comp_unit, std::move(sequence));
if (line_table->GetSize() == 0)
return false;
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 293be12ee6333..352163ceaae9e 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1761,11 +1761,10 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
if (!files)
return false;
- // For each source and header file, create a LineSequence for contributions
- // to the compiland from that file, and add the sequence.
+ // For each source and header file, create a LineTable::Sequence for
+ // contributions to the compiland from that file, and add the sequence.
while (auto file = files->getNext()) {
- std::unique_ptr<LineSequence> sequence(
- line_table->CreateLineSequenceContainer());
+ LineTable::Sequence sequence;
auto lines = m_session_up->findLineNumbers(*compiland_up, *file);
if (!lines)
continue;
@@ -1794,12 +1793,11 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
// of the previous entry's address range if the current entry resulted in
// a gap from the previous entry.
if (is_gap && ShouldAddLine(match_line, prev_line, prev_length)) {
- line_table->AppendLineEntryToSequence(
- sequence.get(), prev_addr + prev_length, prev_line, 0,
- prev_source_idx, false, false, false, false, true);
+ line_table->AppendLineEntryToSequence(sequence, prev_addr + prev_length,
+ prev_line, 0, prev_source_idx,
+ false, false, false, false, true);
- line_table->InsertSequence(sequence.get());
- sequence = line_table->CreateLineSequenceContainer();
+ line_table->InsertSequence(std::move(sequence));
}
if (ShouldAddLine(match_line, lno, length)) {
@@ -1818,7 +1816,7 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
is_epilogue = (addr == epilogue->getVirtualAddress());
}
- line_table->AppendLineEntryToSequence(sequence.get(), addr, lno, col,
+ line_table->AppendLineEntryToSequence(sequence, addr, lno, col,
source_idx, is_statement, false,
is_prologue, is_epilogue, false);
}
@@ -1831,12 +1829,12 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
if (entry_count > 0 && ShouldAddLine(match_line, prev_line, prev_length)) {
// The end is always a terminal entry, so insert it regardless.
- line_table->AppendLineEntryToSequence(
- sequence.get(), prev_addr + prev_length, prev_line, 0,
- prev_source_idx, false, false, false, false, true);
+ line_table->AppendLineEntryToSequence(sequence, prev_addr + prev_length,
+ prev_line, 0, prev_source_idx,
+ false, false, false, false, true);
}
- line_table->InsertSequence(sequence.get());
+ line_table->InsertSequence(std::move(sequence));
}
if (line_table->GetSize()) {
diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp
index aae4ab59ff156..25ef8ed79d138 100644
--- a/lldb/source/Symbol/LineTable.cpp
+++ b/lldb/source/Symbol/LineTable.cpp
@@ -21,15 +21,13 @@ using namespace lldb_private;
LineTable::LineTable(CompileUnit *comp_unit)
: m_comp_unit(comp_unit), m_entries() {}
-LineTable::LineTable(CompileUnit *comp_unit,
- std::vector<std::unique_ptr<LineSequence>> &&sequences)
+LineTable::LineTable(CompileUnit *comp_unit, std::vector<Sequence> &&sequences)
: m_comp_unit(comp_unit), m_entries() {
- LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
+ LessThanBinaryPredicate less_than_bp(this);
llvm::stable_sort(sequences, less_than_bp);
- for (const auto &sequence : sequences) {
- LineSequenceImpl *seq = static_cast<LineSequenceImpl *>(sequence.get());
- m_entries.insert(m_entries.end(), seq->m_entries.begin(),
- seq->m_entries.end());
+ for (const Sequence &seq : sequences) {
+ m_entries.insert(m_entries.end(), seq.m_entries.begin(),
+ seq.m_entries.end());
}
}
@@ -46,7 +44,7 @@ void LineTable::InsertLineEntry(lldb::addr_t file_addr, uint32_t line,
is_start_of_basic_block, is_prologue_end, is_epilogue_begin,
is_terminal_entry);
- LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
+ LessThanBinaryPredicate less_than_bp(this);
entry_collection::iterator pos =
llvm::upper_bound(m_entries, entry, less_than_bp);
@@ -58,25 +56,14 @@ void LineTable::InsertLineEntry(lldb::addr_t file_addr, uint32_t line,
// Dump (&s, Address::DumpStyleFileAddress);
}
-LineSequence::LineSequence() = default;
-
-void LineTable::LineSequenceImpl::Clear() { m_entries.clear(); }
-
-std::unique_ptr<LineSequence> LineTable::CreateLineSequenceContainer() {
- return std::make_unique<LineTable::LineSequenceImpl>();
-}
-
void LineTable::AppendLineEntryToSequence(
- LineSequence *sequence, lldb::addr_t file_addr, uint32_t line,
- uint16_t column, uint16_t file_idx, bool is_start_of_statement,
- bool is_start_of_basic_block, bool is_prologue_end, bool is_epilogue_begin,
- bool is_terminal_entry) {
- assert(sequence != nullptr);
- LineSequenceImpl *seq = reinterpret_cast<LineSequenceImpl *>(sequence);
+ Sequence &sequence, lldb::addr_t file_addr, uint32_t line, uint16_t column,
+ uint16_t file_idx, bool is_start_of_statement, bool is_start_of_basic_block,
+ bool is_prologue_end, bool is_epilogue_begin, bool is_terminal_entry) {
Entry entry(file_addr, line, column, file_idx, is_start_of_statement,
is_start_of_basic_block, is_prologue_end, is_epilogue_begin,
is_terminal_entry);
- entry_collection &entries = seq->m_entries;
+ entry_collection &entries = sequence.m_entries;
// Replace the last entry if the address is the same, otherwise append it. If
// we have multiple line entries at the same address, this indicates illegal
// DWARF so this "fixes" the line table to be correct. If not fixed this can
@@ -102,26 +89,24 @@ void LineTable::AppendLineEntryToSequence(
entries.push_back(entry);
}
-void LineTable::InsertSequence(LineSequence *sequence) {
- assert(sequence != nullptr);
- LineSequenceImpl *seq = reinterpret_cast<LineSequenceImpl *>(sequence);
- if (seq->m_entries.empty())
+void LineTable::InsertSequence(Sequence sequence) {
+ if (sequence.m_entries.empty())
return;
- Entry &entry = seq->m_entries.front();
+ const Entry &entry = sequence.m_entries.front();
// If the first entry address in this sequence is greater than or equal to
// the address of the last item in our entry collection, just append.
if (m_entries.empty() ||
!Entry::EntryAddressLessThan(entry, m_entries.back())) {
- m_entries.insert(m_entries.end(), seq->m_entries.begin(),
- seq->m_entries.end());
+ m_entries.insert(m_entries.end(), sequence.m_entries.begin(),
+ sequence.m_entries.end());
return;
}
// Otherwise, find where this belongs in the collection
entry_collection::iterator begin_pos = m_entries.begin();
entry_collection::iterator end_pos = m_entries.end();
- LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
+ LessThanBinaryPredicate less_than_bp(this);
entry_collection::iterator pos =
std::upper_bound(begin_pos, end_pos, entry, less_than_bp);
@@ -139,15 +124,11 @@ void LineTable::InsertSequence(LineSequence *sequence) {
assert(prev_pos->is_terminal_entry);
}
#endif
- m_entries.insert(pos, seq->m_entries.begin(), seq->m_entries.end());
+ m_entries.insert(pos, sequence.m_entries.begin(), sequence.m_entries.end());
}
-LineTable::Entry::LessThanBinaryPredicate::LessThanBinaryPredicate(
- LineTable *line_table)
- : m_line_table(line_table) {}
-
-bool LineTable::Entry::LessThanBinaryPredicate::
-operator()(const LineTable::Entry &a, const LineTable::Entry &b) const {
+bool LineTable::LessThanBinaryPredicate::operator()(const Entry &a,
+ const Entry &b) const {
#define LT_COMPARE(a, b) \
if (a != b) \
return a < b
@@ -166,12 +147,9 @@ operator()(const LineTable::Entry &a, const LineTable::Entry &b) const {
#undef LT_COMPARE
}
-bool LineTable::Entry::LessThanBinaryPredicate::
-operator()(const std::unique_ptr<LineSequence> &sequence_a,
- const std::unique_ptr<LineSequence> &sequence_b) const {
- auto *seq_a = static_cast<const LineSequenceImpl *>(sequence_a.get());
- auto *seq_b = static_cast<const LineSequenceImpl *>(sequence_b.get());
- return (*this)(seq_a->m_entries.front(), seq_b->m_entries.front());
+bool LineTable::LessThanBinaryPredicate::operator()(
+ const Sequence &seq_a, const Sequence &seq_b) const {
+ return (*this)(seq_a.m_entries.front(), seq_b.m_entries.front());
}
uint32_t LineTable::GetSize() const { return m_entries.size(); }
@@ -447,7 +425,7 @@ size_t LineTable::GetContiguousFileAddressRanges(FileAddressRanges &file_ranges,
LineTable *LineTable::LinkLineTable(const FileRangeMap &file_range_map) {
std::unique_ptr<LineTable> line_table_up(new LineTable(m_comp_unit));
- LineSequenceImpl sequence;
+ Sequence sequence;
const size_t count = m_entries.size();
LineEntry line_entry;
const FileRangeMap::Entry *file_range_entry = nullptr;
@@ -509,8 +487,7 @@ LineTable *LineTable::LinkLineTable(const FileRangeMap &file_range_map) {
sequence.m_entries.back().is_terminal_entry = true;
// Append the sequence since we just terminated the previous one
- line_table_up->InsertSequence(&sequence);
- sequence.Clear();
+ line_table_up->InsertSequence(std::move(sequence));
}
// Now link the current entry
@@ -525,8 +502,7 @@ LineTable *LineTable::LinkLineTable(const FileRangeMap &file_range_map) {
// insert this sequence into our new line table.
if (!sequence.m_entries.empty() &&
sequence.m_entries.back().is_terminal_entry) {
- line_table_up->InsertSequence(&sequence);
- sequence.Clear();
+ line_table_up->InsertSequence(std::move(sequence));
prev_entry_was_linked = false;
} else ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/127800
More information about the lldb-commits
mailing list