From bb73210ba9f16c1516f564235c86cbddccd1bd6d Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 28 Jan 2020 15:03:09 +0100 Subject: [PATCH] [lldb/PDB] Use the new line table constructor The old method of adding line sequences one by one can easily go quadratic if the sequences are not perfectly sorted. The equivalent change in DWARF brought a considerable improvement in line table parsing. It is not clear if the same will be the case for PDB, but this does bring us a step closer towards removing the dangerous API. --- .../SymbolFile/NativePDB/SymbolFileNativePDB.cpp | 38 +++++++++++----------- .../Plugins/SymbolFile/PDB/SymbolFilePDB.cpp | 24 +++++++------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index 9ecaa04..5757efe 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -1021,7 +1021,7 @@ uint32_t SymbolFileNativePDB::ResolveSymbolContext( return 0; } -static void AppendLineEntryToSequence(LineTable &table, LineSequence &sequence, +static void AppendLineEntryToSequence(LineSequence &sequence, const CompilandIndexItem &cci, lldb::addr_t base_addr, uint32_t file_number, @@ -1040,21 +1040,18 @@ static void AppendLineEntryToSequence(LineTable &table, LineSequence &sequence, uint32_t lno = cur_info.getStartLine(); - table.AppendLineEntryToSequence(&sequence, addr, lno, 0, file_number, - is_statement, false, is_prologue, is_epilogue, - false); + LineTable::AppendLineEntryToSequence(&sequence, addr, lno, 0, file_number, + is_statement, false, is_prologue, + is_epilogue, false); } -static void TerminateLineSequence(LineTable &table, - const LineFragmentHeader &block, +static void TerminateLineSequence(const LineFragmentHeader &block, lldb::addr_t base_addr, uint32_t file_number, - uint32_t last_line, - std::unique_ptr seq) { + uint32_t last_line, LineSequence &seq) { // The end is always a terminal entry, so insert it regardless. - table.AppendLineEntryToSequence(seq.get(), base_addr + block.CodeSize, - last_line, 0, file_number, false, false, - false, false, true); - table.InsertSequence(seq.release()); + LineTable::AppendLineEntryToSequence(&seq, base_addr + block.CodeSize, + last_line, 0, file_number, false, false, + false, false, true); } bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) { @@ -1068,11 +1065,11 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) { CompilandIndexItem *cci = m_index->compilands().GetCompiland(cu_id.asCompiland().modi); lldbassert(cci); - auto line_table = std::make_unique(&comp_unit); // This is basically a copy of the .debug$S subsections from all original COFF // object files merged together with address relocations applied. We are // looking for all DEBUG_S_LINES subsections. + std::vector> sequences; for (const DebugSubsectionRecord &dssr : cci->m_debug_stream.getSubsectionsArray()) { if (dssr.kind() != DebugSubsectionKind::Lines) @@ -1111,20 +1108,23 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) { lldbassert(fn_iter != cci->m_file_list.end()); uint32_t file_index = std::distance(cci->m_file_list.begin(), fn_iter); - std::unique_ptr sequence( - line_table->CreateLineSequenceContainer()); + std::unique_ptr sequence = + LineTable::CreateLineSequenceContainer(); lldbassert(!group.LineNumbers.empty()); for (const LineNumberEntry &entry : group.LineNumbers) { - AppendLineEntryToSequence(*line_table, *sequence, *cci, virtual_addr, - file_index, *lfh, entry); + AppendLineEntryToSequence(*sequence, *cci, virtual_addr, file_index, + *lfh, entry); } LineInfo last_line(group.LineNumbers.back().Flags); - TerminateLineSequence(*line_table, *lfh, virtual_addr, file_index, - last_line.getEndLine(), std::move(sequence)); + TerminateLineSequence(*lfh, virtual_addr, file_index, + last_line.getEndLine(), *sequence); + sequences.push_back(std::move(sequence)); } } + auto line_table = + std::make_unique(&comp_unit, std::move(sequences)); 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 7696ec2..f9ce8d9 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -1772,7 +1772,6 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit, // to do a mapping so that we can hand out indices. llvm::DenseMap index_map; BuildSupportFileIdToSupportFileIndexMap(*compiland_up, index_map); - auto line_table = std::make_unique(&comp_unit); // Find contributions to `compiland` from all source and header files. auto files = m_session_up->getSourceFilesForCompiland(*compiland_up); @@ -1781,9 +1780,10 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit, // For each source and header file, create a LineSequence for contributions // to the compiland from that file, and add the sequence. + std::vector> sequences; while (auto file = files->getNext()) { - std::unique_ptr sequence( - line_table->CreateLineSequenceContainer()); + std::unique_ptr sequence = + LineTable::CreateLineSequenceContainer(); auto lines = m_session_up->findLineNumbers(*compiland_up, *file); if (!lines) continue; @@ -1812,12 +1812,12 @@ 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( + LineTable::AppendLineEntryToSequence( sequence.get(), prev_addr + prev_length, prev_line, 0, prev_source_idx, false, false, false, false, true); - line_table->InsertSequence(sequence.release()); - sequence = line_table->CreateLineSequenceContainer(); + sequences.push_back(std::move(sequence)); + sequence = LineTable::CreateLineSequenceContainer(); } if (ShouldAddLine(match_line, lno, length)) { @@ -1836,9 +1836,9 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit &comp_unit, is_epilogue = (addr == epilogue->getVirtualAddress()); } - line_table->AppendLineEntryToSequence(sequence.get(), addr, lno, col, - source_idx, is_statement, false, - is_prologue, is_epilogue, false); + LineTable::AppendLineEntryToSequence(sequence.get(), addr, lno, col, + source_idx, is_statement, false, + is_prologue, is_epilogue, false); } prev_addr = addr; @@ -1849,14 +1849,16 @@ 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( + LineTable::AppendLineEntryToSequence( sequence.get(), prev_addr + prev_length, prev_line, 0, prev_source_idx, false, false, false, false, true); } - line_table->InsertSequence(sequence.get()); + sequences.push_back(std::move(sequence)); } + auto line_table = + std::make_unique(&comp_unit, std::move(sequences)); if (line_table->GetSize()) { comp_unit.SetLineTable(line_table.release()); return true; -- 2.7.4