[lldb] Renaissance LineTable sequences #127800
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LineSeqeunce is basically a vector, except that users aren't supposed to know that. This implements the same concept in a slightly simpler fashion.
Member
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesLineSeqeunce 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:
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]
|
JDevlieghere
approved these changes
Feb 19, 2025
Member
JDevlieghere
left a comment
JDevlieghere
left a comment
There was a problem hiding this comment.
The title of this PR made me laugh.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LineSeqeunce is basically a vector, except that users aren't supposed to know that. This implements the same concept in a slightly simpler fashion.