[Lldb-commits] [lldb] 2526d5b - Revert "[lldb] Use the function block as a source for function ranges (#117996)"
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 3 01:28:24 PST 2024
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"
More information about the lldb-commits
mailing list