Skip to content

Commit 18a96fd

Browse files
committed
[lldb/DWARF] Fix a leak in line table construction
We were creating a bunch of LineSequence objects but never deleting them. This fixes the leak and changes the code to use std::unique_ptr, to make it harder to make the same mistake again.
1 parent 65a31a9 commit 18a96fd

File tree

4 files changed

+24
-19
lines changed

4 files changed

+24
-19
lines changed

lldb/include/lldb/Symbol/LineTable.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ class LineTable {
4646
///
4747
/// \param[in] sequences
4848
/// Unsorted list of line sequences.
49-
LineTable(CompileUnit *comp_unit, std::vector<LineSequence *> &sequences);
49+
LineTable(CompileUnit *comp_unit,
50+
std::vector<std::unique_ptr<LineSequence>> &&sequences);
5051

5152
/// Destructor.
5253
~LineTable();
@@ -70,7 +71,7 @@ class LineTable {
7071
bool is_epilogue_begin, bool is_terminal_entry);
7172

7273
// Used to instantiate the LineSequence helper class
73-
static LineSequence *CreateLineSequenceContainer();
74+
static std::unique_ptr<LineSequence> CreateLineSequenceContainer();
7475

7576
// Append an entry to a caller-provided collection that will later be
7677
// inserted in this line table.
@@ -265,7 +266,8 @@ class LineTable {
265266
public:
266267
LessThanBinaryPredicate(LineTable *line_table);
267268
bool operator()(const LineTable::Entry &, const LineTable::Entry &) const;
268-
bool operator()(const LineSequence*, const LineSequence*) const;
269+
bool operator()(const std::unique_ptr<LineSequence> &,
270+
const std::unique_ptr<LineSequence> &) const;
269271

270272
protected:
271273
LineTable *m_line_table;

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -995,21 +995,22 @@ bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
995995
// FIXME: Rather than parsing the whole line table and then copying it over
996996
// into LLDB, we should explore using a callback to populate the line table
997997
// while we parse to reduce memory usage.
998-
LineSequence *sequence = LineTable::CreateLineSequenceContainer();
999-
std::vector<LineSequence *> sequences;
998+
std::unique_ptr<LineSequence> sequence =
999+
LineTable::CreateLineSequenceContainer();
1000+
std::vector<std::unique_ptr<LineSequence>> sequences;
10001001
for (auto &row : line_table->Rows) {
10011002
LineTable::AppendLineEntryToSequence(
1002-
sequence, row.Address.Address, row.Line, row.Column, row.File,
1003+
sequence.get(), row.Address.Address, row.Line, row.Column, row.File,
10031004
row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
10041005
row.EndSequence);
10051006
if (row.EndSequence) {
1006-
sequences.push_back(sequence);
1007+
sequences.push_back(std::move(sequence));
10071008
sequence = LineTable::CreateLineSequenceContainer();
10081009
}
10091010
}
10101011

10111012
std::unique_ptr<LineTable> line_table_up =
1012-
std::make_unique<LineTable>(&comp_unit, sequences);
1013+
std::make_unique<LineTable>(&comp_unit, std::move(sequences));
10131014

10141015
if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) {
10151016
// We have an object file that has a line table with addresses that are not

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,7 +1817,7 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
18171817
prev_source_idx, false, false, false, false, true);
18181818

18191819
line_table->InsertSequence(sequence.release());
1820-
sequence.reset(line_table->CreateLineSequenceContainer());
1820+
sequence = line_table->CreateLineSequenceContainer();
18211821
}
18221822

18231823
if (ShouldAddLine(match_line, lno, length)) {
@@ -1854,7 +1854,7 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit,
18541854
prev_source_idx, false, false, false, false, true);
18551855
}
18561856

1857-
line_table->InsertSequence(sequence.release());
1857+
line_table->InsertSequence(sequence.get());
18581858
}
18591859

18601860
if (line_table->GetSize()) {

lldb/source/Symbol/LineTable.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,15 @@ using namespace lldb_private;
2121
LineTable::LineTable(CompileUnit *comp_unit)
2222
: m_comp_unit(comp_unit), m_entries() {}
2323

24-
LineTable::LineTable(CompileUnit *comp_unit, std::vector<LineSequence *> &sequences)
24+
LineTable::LineTable(CompileUnit *comp_unit,
25+
std::vector<std::unique_ptr<LineSequence>> &&sequences)
2526
: m_comp_unit(comp_unit), m_entries() {
2627
LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
2728
llvm::stable_sort(sequences, less_than_bp);
28-
for (auto *sequence : sequences) {
29-
LineSequenceImpl *seq = reinterpret_cast<LineSequenceImpl *>(sequence);
29+
for (const auto &sequence : sequences) {
30+
LineSequenceImpl *seq = static_cast<LineSequenceImpl *>(sequence.get());
3031
m_entries.insert(m_entries.end(), seq->m_entries.begin(),
31-
seq->m_entries.end());
32+
seq->m_entries.end());
3233
}
3334
}
3435

@@ -61,8 +62,8 @@ LineSequence::LineSequence() {}
6162

6263
void LineTable::LineSequenceImpl::Clear() { m_entries.clear(); }
6364

64-
LineSequence *LineTable::CreateLineSequenceContainer() {
65-
return new LineTable::LineSequenceImpl();
65+
std::unique_ptr<LineSequence> LineTable::CreateLineSequenceContainer() {
66+
return std::make_unique<LineTable::LineSequenceImpl>();
6667
}
6768

6869
void LineTable::AppendLineEntryToSequence(
@@ -166,9 +167,10 @@ operator()(const LineTable::Entry &a, const LineTable::Entry &b) const {
166167
}
167168

168169
bool LineTable::Entry::LessThanBinaryPredicate::
169-
operator()(const LineSequence *sequence_a, const LineSequence *sequence_b) const {
170-
auto *seq_a = static_cast<const LineSequenceImpl *>(sequence_a);
171-
auto *seq_b = static_cast<const LineSequenceImpl *>(sequence_b);
170+
operator()(const std::unique_ptr<LineSequence> &sequence_a,
171+
const std::unique_ptr<LineSequence> &sequence_b) const {
172+
auto *seq_a = static_cast<const LineSequenceImpl *>(sequence_a.get());
173+
auto *seq_b = static_cast<const LineSequenceImpl *>(sequence_b.get());
172174
return (*this)(seq_a->m_entries.front(), seq_b->m_entries.front());
173175
}
174176

0 commit comments

Comments
 (0)