Author: Pavel Labath Date: 2020-01-21T14:44:11+01:00 New Revision: 18a96fd573b134fed7d8ea6b87930e7a059d6c90
URL: https://github.com/llvm/llvm-project/commit/18a96fd573b134fed7d8ea6b87930e7a059d6c90 DIFF: https://github.com/llvm/llvm-project/commit/18a96fd573b134fed7d8ea6b87930e7a059d6c90.diff LOG: [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. Added: Modified: lldb/include/lldb/Symbol/LineTable.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Symbol/LineTable.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/LineTable.h b/lldb/include/lldb/Symbol/LineTable.h index 363966aae9f3..0323bf5e8039 100644 --- a/lldb/include/lldb/Symbol/LineTable.h +++ b/lldb/include/lldb/Symbol/LineTable.h @@ -46,7 +46,8 @@ class LineTable { /// /// \param[in] sequences /// Unsorted list of line sequences. - LineTable(CompileUnit *comp_unit, std::vector<LineSequence *> &sequences); + LineTable(CompileUnit *comp_unit, + std::vector<std::unique_ptr<LineSequence>> &&sequences); /// Destructor. ~LineTable(); @@ -70,7 +71,7 @@ class LineTable { bool is_epilogue_begin, bool is_terminal_entry); // Used to instantiate the LineSequence helper class - static LineSequence *CreateLineSequenceContainer(); + static std::unique_ptr<LineSequence> CreateLineSequenceContainer(); // Append an entry to a caller-provided collection that will later be // inserted in this line table. @@ -265,7 +266,8 @@ class LineTable { public: LessThanBinaryPredicate(LineTable *line_table); bool operator()(const LineTable::Entry &, const LineTable::Entry &) const; - bool operator()(const LineSequence*, const LineSequence*) const; + bool operator()(const std::unique_ptr<LineSequence> &, + const std::unique_ptr<LineSequence> &) const; protected: LineTable *m_line_table; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index a07297414e05..19e15b527903 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -995,21 +995,22 @@ 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. - LineSequence *sequence = LineTable::CreateLineSequenceContainer(); - std::vector<LineSequence *> sequences; + std::unique_ptr<LineSequence> sequence = + LineTable::CreateLineSequenceContainer(); + std::vector<std::unique_ptr<LineSequence>> sequences; for (auto &row : line_table->Rows) { LineTable::AppendLineEntryToSequence( - sequence, row.Address.Address, row.Line, row.Column, row.File, + sequence.get(), row.Address.Address, row.Line, row.Column, row.File, row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin, row.EndSequence); if (row.EndSequence) { - sequences.push_back(sequence); + sequences.push_back(std::move(sequence)); sequence = LineTable::CreateLineSequenceContainer(); } } std::unique_ptr<LineTable> line_table_up = - std::make_unique<LineTable>(&comp_unit, sequences); + std::make_unique<LineTable>(&comp_unit, std::move(sequences)); if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) { // We have an object file that has a line table with addresses that are not diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp index 917ab68af418..7bef47e8eaee 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -1817,7 +1817,7 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit, prev_source_idx, false, false, false, false, true); line_table->InsertSequence(sequence.release()); - sequence.reset(line_table->CreateLineSequenceContainer()); + sequence = line_table->CreateLineSequenceContainer(); } if (ShouldAddLine(match_line, lno, length)) { @@ -1854,7 +1854,7 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit, prev_source_idx, false, false, false, false, true); } - line_table->InsertSequence(sequence.release()); + line_table->InsertSequence(sequence.get()); } if (line_table->GetSize()) { diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp index e95d09908f32..79d08024f20c 100644 --- a/lldb/source/Symbol/LineTable.cpp +++ b/lldb/source/Symbol/LineTable.cpp @@ -21,14 +21,15 @@ using namespace lldb_private; LineTable::LineTable(CompileUnit *comp_unit) : m_comp_unit(comp_unit), m_entries() {} -LineTable::LineTable(CompileUnit *comp_unit, std::vector<LineSequence *> &sequences) +LineTable::LineTable(CompileUnit *comp_unit, + std::vector<std::unique_ptr<LineSequence>> &&sequences) : m_comp_unit(comp_unit), m_entries() { LineTable::Entry::LessThanBinaryPredicate less_than_bp(this); llvm::stable_sort(sequences, less_than_bp); - for (auto *sequence : sequences) { - LineSequenceImpl *seq = reinterpret_cast<LineSequenceImpl *>(sequence); + 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()); + seq->m_entries.end()); } } @@ -61,8 +62,8 @@ LineSequence::LineSequence() {} void LineTable::LineSequenceImpl::Clear() { m_entries.clear(); } -LineSequence *LineTable::CreateLineSequenceContainer() { - return new LineTable::LineSequenceImpl(); +std::unique_ptr<LineSequence> LineTable::CreateLineSequenceContainer() { + return std::make_unique<LineTable::LineSequenceImpl>(); } void LineTable::AppendLineEntryToSequence( @@ -166,9 +167,10 @@ operator()(const LineTable::Entry &a, const LineTable::Entry &b) const { } bool LineTable::Entry::LessThanBinaryPredicate:: -operator()(const LineSequence *sequence_a, const LineSequence *sequence_b) const { - auto *seq_a = static_cast<const LineSequenceImpl *>(sequence_a); - auto *seq_b = static_cast<const LineSequenceImpl *>(sequence_b); +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()); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
