Author: Pavel Labath Date: 2024-12-03T10:27:31+01:00 New Revision: 2526d5b1689389da9b194b5ec2878cfb2f4aca93
URL: https://github.com/llvm/llvm-project/commit/2526d5b1689389da9b194b5ec2878cfb2f4aca93 DIFF: https://github.com/llvm/llvm-project/commit/2526d5b1689389da9b194b5ec2878cfb2f4aca93.diff LOG: Revert "[lldb] Use the function block as a source for function ranges (#117996)" This reverts commit ba14dac481564000339ba22ab867617590184f4c. I guess "has no conflicts" doesn't mean "it will build". Added: Modified: lldb/include/lldb/Symbol/Function.h lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Symbol/Function.cpp lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s lldb/test/Shell/SymbolFile/PDB/function-nested-block.test Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h index 51289f0f74ddfa..855940a6415d72 100644 --- a/lldb/include/lldb/Symbol/Function.h +++ b/lldb/include/lldb/Symbol/Function.h @@ -653,6 +653,9 @@ class Function : public UserID, public SymbolContextScope { /// All lexical blocks contained in this function. Block m_block; + /// List of address ranges belonging to the function. + AddressRanges m_ranges; + /// The function address range that covers the widest range needed to contain /// all blocks. DEPRECATED: do not use this field in new code as the range may /// include addresses belonging to other functions. diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp index bc886259d6fa5f..df3bf157278daf 100644 --- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp +++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp @@ -299,7 +299,9 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function &func) { // "INLINE 0 ...", the current level is 0 and its parent block is the // function block at index 0. std::vector<Block *> blocks; - blocks.push_back(&func.GetBlock(false)); + Block &block = func.GetBlock(false); + block.AddRange(Block::Range(0, func.GetAddressRange().GetByteSize())); + blocks.push_back(&block); size_t blocks_added = 0; addr_t func_base = func.GetAddressRange().GetBaseAddress().GetOffset(); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 6f19b264eb3dda..fe711c56958c44 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1305,76 +1305,121 @@ bool SymbolFileDWARF::ParseDebugMacros(CompileUnit &comp_unit) { return true; } -size_t SymbolFileDWARF::ParseBlocksRecursive(CompileUnit &comp_unit, - Block *parent_block, DWARFDIE die, - addr_t subprogram_low_pc) { +size_t SymbolFileDWARF::ParseBlocksRecursive( + lldb_private::CompileUnit &comp_unit, Block *parent_block, + const DWARFDIE &orig_die, addr_t subprogram_low_pc, uint32_t depth) { size_t blocks_added = 0; - for (; die; die = die.GetSibling()) { + DWARFDIE die = orig_die; + while (die) { dw_tag_t tag = die.Tag(); - if (tag != DW_TAG_inlined_subroutine && tag != DW_TAG_lexical_block) - continue; + switch (tag) { + case DW_TAG_inlined_subroutine: + case DW_TAG_subprogram: + case DW_TAG_lexical_block: { + Block *block = nullptr; + if (tag == DW_TAG_subprogram) { + // Skip any DW_TAG_subprogram DIEs that are inside of a normal or + // inlined functions. These will be parsed on their own as separate + // entities. - Block *block = parent_block->CreateChild(die.GetID()).get(); - DWARFRangeList ranges; - const char *name = nullptr; - const char *mangled_name = nullptr; - - std::optional<int> decl_file; - std::optional<int> decl_line; - std::optional<int> decl_column; - std::optional<int> call_file; - std::optional<int> call_line; - std::optional<int> call_column; - if (die.GetDIENamesAndRanges(name, mangled_name, ranges, decl_file, - decl_line, decl_column, call_file, call_line, - call_column, nullptr)) { - const size_t num_ranges = ranges.GetSize(); - for (size_t i = 0; i < num_ranges; ++i) { - const DWARFRangeList::Entry &range = ranges.GetEntryRef(i); - const addr_t range_base = range.GetRangeBase(); - if (range_base >= subprogram_low_pc) - block->AddRange(Block::Range(range_base - subprogram_low_pc, - range.GetByteSize())); - else { - GetObjectFile()->GetModule()->ReportError( - "{0:x8}: adding range [{1:x16}-{2:x16}) which has a base " - "that is less than the function's low PC {3:x16}. Please file " - "a bug and attach the file at the " - "start of this error message", - block->GetID(), range_base, range.GetRangeEnd(), - subprogram_low_pc); - } - } - block->FinalizeRanges(); - - if (tag != DW_TAG_subprogram && - (name != nullptr || mangled_name != nullptr)) { - std::unique_ptr<Declaration> decl_up; - if (decl_file || decl_line || decl_column) - decl_up = std::make_unique<Declaration>( - comp_unit.GetSupportFiles().GetFileSpecAtIndex( - decl_file ? *decl_file : 0), - decl_line ? *decl_line : 0, decl_column ? *decl_column : 0); - - std::unique_ptr<Declaration> call_up; - if (call_file || call_line || call_column) - call_up = std::make_unique<Declaration>( - comp_unit.GetSupportFiles().GetFileSpecAtIndex( - call_file ? *call_file : 0), - call_line ? *call_line : 0, call_column ? *call_column : 0); - - block->SetInlinedFunctionInfo(name, mangled_name, decl_up.get(), - call_up.get()); + if (depth > 0) + break; + + block = parent_block; + } else { + block = parent_block->CreateChild(die.GetID()).get(); } + DWARFRangeList ranges; + const char *name = nullptr; + const char *mangled_name = nullptr; + + std::optional<int> decl_file; + std::optional<int> decl_line; + std::optional<int> decl_column; + std::optional<int> call_file; + std::optional<int> call_line; + std::optional<int> call_column; + if (die.GetDIENamesAndRanges(name, mangled_name, ranges, decl_file, + decl_line, decl_column, call_file, call_line, + call_column, nullptr)) { + if (tag == DW_TAG_subprogram) { + assert(subprogram_low_pc == LLDB_INVALID_ADDRESS); + subprogram_low_pc = ranges.GetMinRangeBase(0); + } else if (tag == DW_TAG_inlined_subroutine) { + // We get called here for inlined subroutines in two ways. The first + // time is when we are making the Function object for this inlined + // concrete instance. Since we're creating a top level block at + // here, the subprogram_low_pc will be LLDB_INVALID_ADDRESS. So we + // need to adjust the containing address. The second time is when we + // are parsing the blocks inside the function that contains the + // inlined concrete instance. Since these will be blocks inside the + // containing "real" function the offset will be for that function. + if (subprogram_low_pc == LLDB_INVALID_ADDRESS) { + subprogram_low_pc = ranges.GetMinRangeBase(0); + } + } - ++blocks_added; + const size_t num_ranges = ranges.GetSize(); + for (size_t i = 0; i < num_ranges; ++i) { + const DWARFRangeList::Entry &range = ranges.GetEntryRef(i); + const addr_t range_base = range.GetRangeBase(); + if (range_base >= subprogram_low_pc) + block->AddRange(Block::Range(range_base - subprogram_low_pc, + range.GetByteSize())); + else { + GetObjectFile()->GetModule()->ReportError( + "{0:x8}: adding range [{1:x16}-{2:x16}) which has a base " + "that is less than the function's low PC {3:x16}. Please file " + "a bug and attach the file at the " + "start of this error message", + block->GetID(), range_base, range.GetRangeEnd(), + subprogram_low_pc); + } + } + block->FinalizeRanges(); + + if (tag != DW_TAG_subprogram && + (name != nullptr || mangled_name != nullptr)) { + std::unique_ptr<Declaration> decl_up; + if (decl_file || decl_line || decl_column) + decl_up = std::make_unique<Declaration>( + comp_unit.GetSupportFiles().GetFileSpecAtIndex( + decl_file ? *decl_file : 0), + decl_line ? *decl_line : 0, decl_column ? *decl_column : 0); + + std::unique_ptr<Declaration> call_up; + if (call_file || call_line || call_column) + call_up = std::make_unique<Declaration>( + comp_unit.GetSupportFiles().GetFileSpecAtIndex( + call_file ? *call_file : 0), + call_line ? *call_line : 0, call_column ? *call_column : 0); + + block->SetInlinedFunctionInfo(name, mangled_name, decl_up.get(), + call_up.get()); + } + + ++blocks_added; - if (die.HasChildren()) { - blocks_added += ParseBlocksRecursive( - comp_unit, block, die.GetFirstChild(), subprogram_low_pc); + if (die.HasChildren()) { + blocks_added += + ParseBlocksRecursive(comp_unit, block, die.GetFirstChild(), + subprogram_low_pc, depth + 1); + } } + } break; + default: + break; } + + // Only parse siblings of the block if we are not at depth zero. A depth of + // zero indicates we are currently parsing the top level DW_TAG_subprogram + // DIE + + if (depth == 0) + die.Clear(); + else + die = die.GetSibling(); } return blocks_added; } @@ -3195,16 +3240,8 @@ size_t SymbolFileDWARF::ParseBlocksRecursive(Function &func) { DWARFDIE function_die = dwarf_cu->GetNonSkeletonUnit().GetDIE(function_die_offset); if (function_die) { - // We can't use the file address from the Function object as (in the OSO - // case) it will already be remapped to the main module. - DWARFRangeList ranges = function_die.GetDIE()->GetAttributeAddressRanges( - function_die.GetCU(), - /*check_hi_lo_pc=*/true); - lldb::addr_t function_file_addr = - ranges.GetMinRangeBase(LLDB_INVALID_ADDRESS); - if (function_file_addr != LLDB_INVALID_ADDRESS) - ParseBlocksRecursive(*comp_unit, &func.GetBlock(false), - function_die.GetFirstChild(), function_file_addr); + ParseBlocksRecursive(*comp_unit, &func.GetBlock(false), function_die, + LLDB_INVALID_ADDRESS, 0); } return functions_added; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 76f4188fdf4afb..ac25a0c48ee7d7 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -395,7 +395,8 @@ class SymbolFileDWARF : public SymbolFileCommon { Function *ParseFunction(CompileUnit &comp_unit, const DWARFDIE &die); size_t ParseBlocksRecursive(CompileUnit &comp_unit, Block *parent_block, - DWARFDIE die, lldb::addr_t subprogram_low_pc); + const DWARFDIE &die, + lldb::addr_t subprogram_low_pc, uint32_t depth); size_t ParseTypes(const SymbolContext &sc, const DWARFDIE &die, bool parse_siblings, bool parse_children); diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index 27d51bbd1cb563..d17fedf26b4c48 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -394,12 +394,18 @@ Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { switch (sym.kind()) { case S_GPROC32: - case S_LPROC32: + case S_LPROC32: { // This is a function. It must be global. Creating the Function entry // for it automatically creates a block for it. - if (FunctionSP func = GetOrCreateFunction(block_id, *comp_unit)) - return &func->GetBlock(false); + FunctionSP func = GetOrCreateFunction(block_id, *comp_unit); + if (func) { + Block &block = func->GetBlock(false); + if (block.GetNumRanges() == 0) + block.AddRange(Block::Range(0, func->GetAddressRange().GetByteSize())); + return █ + } break; + } case S_BLOCK32: { // This is a block. Its parent is either a function or another block. In // either case, its parent can be viewed as a block (e.g. a function diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp index b7854c05d345a8..4935b0fbdfd877 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -402,32 +402,44 @@ static size_t ParseFunctionBlocksForPDBSymbol( assert(pdb_symbol && parent_block); size_t num_added = 0; + switch (pdb_symbol->getSymTag()) { + case PDB_SymType::Block: + case PDB_SymType::Function: { + Block *block = nullptr; + auto &raw_sym = pdb_symbol->getRawSymbol(); + if (auto *pdb_func = llvm::dyn_cast<PDBSymbolFunc>(pdb_symbol)) { + if (pdb_func->hasNoInlineAttribute()) + break; + if (is_top_parent) + block = parent_block; + else + break; + } else if (llvm::isa<PDBSymbolBlock>(pdb_symbol)) { + auto uid = pdb_symbol->getSymIndexId(); + if (parent_block->FindBlockByID(uid)) + break; + if (raw_sym.getVirtualAddress() < func_file_vm_addr) + break; - if (!is_top_parent) { - // Ranges for the top block were parsed together with the function. - if (pdb_symbol->getSymTag() != PDB_SymType::Block) - return num_added; + block = parent_block->CreateChild(pdb_symbol->getSymIndexId()).get(); + } else + llvm_unreachable("Unexpected PDB symbol!"); - auto &raw_sym = pdb_symbol->getRawSymbol(); - assert(llvm::isa<PDBSymbolBlock>(pdb_symbol)); - auto uid = pdb_symbol->getSymIndexId(); - if (parent_block->FindBlockByID(uid)) - return num_added; - if (raw_sym.getVirtualAddress() < func_file_vm_addr) - return num_added; - - Block *block = parent_block->CreateChild(pdb_symbol->getSymIndexId()).get(); block->AddRange(Block::Range( raw_sym.getVirtualAddress() - func_file_vm_addr, raw_sym.getLength())); block->FinalizeRanges(); - } - auto results_up = pdb_symbol->findAllChildren(); - if (!results_up) - return num_added; + ++num_added; - while (auto symbol_up = results_up->getNext()) { - num_added += ParseFunctionBlocksForPDBSymbol( - func_file_vm_addr, symbol_up.get(), parent_block, false); + auto results_up = pdb_symbol->findAllChildren(); + if (!results_up) + break; + while (auto symbol_up = results_up->getNext()) { + num_added += ParseFunctionBlocksForPDBSymbol( + func_file_vm_addr, symbol_up.get(), block, false); + } + } break; + default: + break; } return num_added; } diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp index 4f07b946353a44..b346749ca06ec3 100644 --- a/lldb/source/Symbol/Function.cpp +++ b/lldb/source/Symbol/Function.cpp @@ -279,14 +279,9 @@ Function::Function(CompileUnit *comp_unit, lldb::user_id_t func_uid, AddressRanges ranges) : UserID(func_uid), m_comp_unit(comp_unit), m_type_uid(type_uid), m_type(type), m_mangled(mangled), m_block(*this, func_uid), - m_range(CollapseRanges(ranges)), m_prologue_byte_size(0) { + m_ranges(std::move(ranges)), m_range(CollapseRanges(m_ranges)), + m_frame_base(), m_flags(), m_prologue_byte_size(0) { assert(comp_unit != nullptr); - lldb::addr_t base_file_addr = m_range.GetBaseAddress().GetFileAddress(); - for (const AddressRange &range : ranges) - m_block.AddRange( - Block::Range(range.GetBaseAddress().GetFileAddress() - base_file_addr, - range.GetByteSize())); - m_block.FinalizeRanges(); } Function::~Function() = default; @@ -431,16 +426,13 @@ void Function::GetDescription(Stream *s, lldb::DescriptionLevel level, llvm::interleaveComma(decl_context, *s, [&](auto &ctx) { ctx.Dump(*s); }); *s << "}"; } - *s << ", range" << (m_block.GetNumRanges() > 1 ? "s" : "") << " = "; + *s << ", range" << (m_ranges.size() > 1 ? "s" : "") << " = "; Address::DumpStyle fallback_style = level == eDescriptionLevelVerbose ? Address::DumpStyleModuleWithFileAddress : Address::DumpStyleFileAddress; - for (unsigned idx = 0; idx < m_block.GetNumRanges(); ++idx) { - AddressRange range; - m_block.GetRangeAtIndex(idx, range); + for (const AddressRange &range : m_ranges) range.Dump(s, target, Address::DumpStyleLoadAddress, fallback_style); - } } void Function::Dump(Stream *s, bool show_context) const { diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s b/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s index b03d5d12ad2a1d..2584158207cc8e 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s @@ -10,7 +10,7 @@ # CHECK: 1 match found in {{.*}} # CHECK: Summary: {{.*}}`foo -# CHECK: Function: id = {{.*}}, name = "foo", ranges = [0x0000000000000000-0x000000000000000e)[0x0000000000000014-0x000000000000001c) +# CHECK: Function: id = {{.*}}, name = "foo", ranges = [0x0000000000000000-0x0000000000000007)[0x0000000000000007-0x000000000000000e)[0x0000000000000014-0x000000000000001b)[0x000000000000001b-0x000000000000001c) .text diff --git a/lldb/test/Shell/SymbolFile/PDB/function-nested-block.test b/lldb/test/Shell/SymbolFile/PDB/function-nested-block.test index 9057d01c25840f..1cb20a40363827 100644 --- a/lldb/test/Shell/SymbolFile/PDB/function-nested-block.test +++ b/lldb/test/Shell/SymbolFile/PDB/function-nested-block.test @@ -2,6 +2,7 @@ REQUIRES: system-windows, lld RUN: %build --compiler=clang-cl --nodefaultlib --output=%t.exe %S/Inputs/FunctionNestedBlockTest.cpp RUN: lldb-test symbols -find=function -file FunctionNestedBlockTest.cpp -line 4 %t.exe | FileCheck --check-prefix=CHECK-FUNCTION %s RUN: lldb-test symbols -find=block -file FunctionNestedBlockTest.cpp -line 4 %t.exe | FileCheck --check-prefix=CHECK-BLOCK %s +XFAIL: * CHECK-FUNCTION: Found 1 functions: CHECK-FUNCTION: name = "{{.*}}", mangled = "{{_?}}main" _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
