Skip to content

[lldb] (Begin to) support discontinuous lldb_private::Functions #115730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lldb/include/lldb/Symbol/Function.h
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ class Function : public UserID, public SymbolContextScope {
/// The section offset based address for this function.
Function(CompileUnit *comp_unit, lldb::user_id_t func_uid,
lldb::user_id_t func_type_uid, const Mangled &mangled,
Type *func_type, const AddressRange &range);
Type *func_type, AddressRanges ranges);

/// Destructor.
~Function() override;
Expand Down Expand Up @@ -649,8 +649,12 @@ 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
/// all blocks. DEPRECATED: do not use this field in new code as the range may
/// include addresses belonging to other functions.
AddressRange m_range;

/// The frame base expression for variables that are relative to the frame
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ FunctionSP SymbolFileBreakpad::GetOrCreateFunction(CompileUnit &comp_unit) {
section_sp, address - section_sp->GetFileAddress(), record->Size);
// Use the CU's id because every CU has only one function inside.
func_sp = std::make_shared<Function>(&comp_unit, id, 0, func_name,
nullptr, func_range);
nullptr, AddressRanges{func_range});
comp_unit.AddFunction(func_sp);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ size_t SymbolFileCTF::ParseFunctions(CompileUnit &cu) {
lldb::user_id_t func_uid = m_functions.size();
FunctionSP function_sp = std::make_shared<Function>(
&cu, func_uid, function_type_uid, symbol->GetMangled(), type_sp.get(),
func_range);
AddressRanges{func_range});
m_functions.emplace_back(function_sp);
cu.AddFunction(function_sp);
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class DWARFASTParser {

virtual Function *ParseFunctionFromDWARF(CompileUnit &comp_unit,
const DWARFDIE &die,
const AddressRange &range) = 0;
AddressRanges ranges) = 0;

virtual bool CompleteTypeFromDWARF(const DWARFDIE &die, Type *type,
const CompilerType &compiler_type) = 0;
Expand Down
24 changes: 10 additions & 14 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2340,12 +2340,9 @@ DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
return ConstString(sstr.GetString());
}

Function *
DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
const DWARFDIE &die,
const AddressRange &func_range) {
assert(func_range.GetBaseAddress().IsValid());
DWARFRangeList func_ranges;
Function *DWARFASTParserClang::ParseFunctionFromDWARF(
CompileUnit &comp_unit, const DWARFDIE &die, AddressRanges func_ranges) {
DWARFRangeList unused_func_ranges;
const char *name = nullptr;
const char *mangled = nullptr;
std::optional<int> decl_file;
Expand All @@ -2361,9 +2358,9 @@ DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
if (tag != DW_TAG_subprogram)
return nullptr;

if (die.GetDIENamesAndRanges(name, mangled, func_ranges, decl_file, decl_line,
decl_column, call_file, call_line, call_column,
&frame_base)) {
if (die.GetDIENamesAndRanges(name, mangled, unused_func_ranges, decl_file,
decl_line, decl_column, call_file, call_line,
call_column, &frame_base)) {
Mangled func_name;
if (mangled)
func_name.SetValue(ConstString(mangled));
Expand Down Expand Up @@ -2395,11 +2392,10 @@ DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
assert(func_type == nullptr || func_type != DIE_IS_BEING_PARSED);

const user_id_t func_user_id = die.GetID();
func_sp =
std::make_shared<Function>(&comp_unit,
func_user_id, // UserID is the DIE offset
func_user_id, func_name, func_type,
func_range); // first address range
func_sp = std::make_shared<Function>(
&comp_unit,
func_user_id, // UserID is the DIE offset
func_user_id, func_name, func_type, std::move(func_ranges));

if (func_sp.get() != nullptr) {
if (frame_base.IsValid())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser {
lldb_private::Function *
ParseFunctionFromDWARF(lldb_private::CompileUnit &comp_unit,
const lldb_private::plugin::dwarf::DWARFDIE &die,
const lldb_private::AddressRange &func_range) override;
lldb_private::AddressRanges func_ranges) override;

bool CompleteTypeFromDWARF(
const lldb_private::plugin::dwarf::DWARFDIE &die,
Expand Down
34 changes: 11 additions & 23 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -918,32 +918,20 @@ Function *SymbolFileDWARF::ParseFunction(CompileUnit &comp_unit,
if (!dwarf_ast)
return nullptr;

DWARFRangeList ranges = die.GetDIE()->GetAttributeAddressRanges(
die.GetCU(), /*check_hi_lo_pc=*/true);
if (ranges.IsEmpty())
return nullptr;

// Union of all ranges in the function DIE (if the function is
// discontiguous)
lldb::addr_t lowest_func_addr = ranges.GetMinRangeBase(0);
lldb::addr_t highest_func_addr = ranges.GetMaxRangeEnd(0);
if (lowest_func_addr == LLDB_INVALID_ADDRESS ||
lowest_func_addr >= highest_func_addr ||
lowest_func_addr < m_first_code_address)
return nullptr;

AddressRanges ranges;
ModuleSP module_sp(die.GetModule());
AddressRange func_range;
func_range.GetBaseAddress().ResolveAddressUsingFileSections(
lowest_func_addr, module_sp->GetSectionList());
if (!func_range.GetBaseAddress().IsValid())
return nullptr;

func_range.SetByteSize(highest_func_addr - lowest_func_addr);
if (!FixupAddress(func_range.GetBaseAddress()))
for (const auto &range : die.GetDIE()->GetAttributeAddressRanges(
die.GetCU(), /*check_hi_lo_pc=*/true)) {
if (range.base < m_first_code_address)
continue;
if (Address base_addr(range.base, module_sp->GetSectionList());
base_addr.IsValid() && FixupAddress(base_addr))
ranges.emplace_back(std::move(base_addr), range.size);
}
if (ranges.empty())
return nullptr;

return dwarf_ast->ParseFunctionFromDWARF(comp_unit, die, func_range);
return dwarf_ast->ParseFunctionFromDWARF(comp_unit, die, std::move(ranges));
}

ConstString
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ lldb::FunctionSP SymbolFileNativePDB::CreateFunction(PdbCompilandSymId func_id,
Mangled mangled(proc.Name);
FunctionSP func_sp = std::make_shared<Function>(
&comp_unit, toOpaqueUid(func_id), toOpaqueUid(sig_id), mangled,
func_type.get(), func_range);
func_type.get(), AddressRanges{func_range});

comp_unit.AddFunction(func_sp);

Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,9 @@ SymbolFilePDB::ParseCompileUnitFunctionForPDBFunc(const PDBSymbolFunc &pdb_func,

Mangled mangled = GetMangledForPDBFunc(pdb_func);

FunctionSP func_sp =
std::make_shared<Function>(&comp_unit, pdb_func.getSymIndexId(),
func_type_uid, mangled, func_type, func_range);
FunctionSP func_sp = std::make_shared<Function>(
&comp_unit, pdb_func.getSymIndexId(), func_type_uid, mangled, func_type,
AddressRanges{func_range});

comp_unit.AddFunction(func_sp);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ size_t SymbolFileSymtab::ParseFunctions(CompileUnit &comp_unit) {
// for this function
curr_symbol->GetMangled(), // Linker/mangled name
nullptr, // no return type for a code symbol...
func_range)); // first address range
AddressRanges{func_range}));

if (func_sp.get() != nullptr) {
comp_unit.AddFunction(func_sp);
Expand Down
39 changes: 29 additions & 10 deletions lldb/source/Symbol/Function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,32 @@ Function *IndirectCallEdge::GetCallee(ModuleList &images,

/// @}

AddressRange CollapseRanges(llvm::ArrayRef<AddressRange> ranges) {
if (ranges.empty())
return AddressRange();
if (ranges.size() == 1)
return ranges[0];

Address lowest_addr = ranges[0].GetBaseAddress();
addr_t highest_addr = lowest_addr.GetFileAddress() + ranges[0].GetByteSize();
for (const AddressRange &range : ranges.drop_front()) {
Address range_begin = range.GetBaseAddress();
addr_t range_end = range_begin.GetFileAddress() + range.GetByteSize();
if (range_begin.GetFileAddress() < lowest_addr.GetFileAddress())
lowest_addr = range_begin;
if (range_end > highest_addr)
highest_addr = range_end;
}
return AddressRange(lowest_addr, highest_addr - lowest_addr.GetFileAddress());
Comment on lines +264 to +273
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't help but think you don't actually need the highest_addr and you could just keep extending the byte size of the range, but maybe I'm missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the range may need extending at both ends (there's no guarantee the lowest address will be the first one). Extending it an the lower end is tricky as you need to both change the base address and increase the range size. Storing it decomposed and only constructing the range object at the end is simpler.

}

//
Function::Function(CompileUnit *comp_unit, lldb::user_id_t func_uid,
lldb::user_id_t type_uid, const Mangled &mangled, Type *type,
const AddressRange &range)
AddressRanges ranges)
: UserID(func_uid), m_comp_unit(comp_unit), m_type_uid(type_uid),
m_type(type), m_mangled(mangled), m_block(func_uid), m_range(range),
m_type(type), m_mangled(mangled), m_block(func_uid),
m_ranges(std::move(ranges)), m_range(CollapseRanges(m_ranges)),
m_frame_base(), m_flags(), m_prologue_byte_size(0) {
m_block.SetParentScope(this);
assert(comp_unit != nullptr);
Expand Down Expand Up @@ -406,14 +426,13 @@ void Function::GetDescription(Stream *s, lldb::DescriptionLevel level,
llvm::interleaveComma(decl_context, *s, [&](auto &ctx) { ctx.Dump(*s); });
*s << "}";
}
*s << ", range = ";
Address::DumpStyle fallback_style;
if (level == eDescriptionLevelVerbose)
fallback_style = Address::DumpStyleModuleWithFileAddress;
else
fallback_style = Address::DumpStyleFileAddress;
GetAddressRange().Dump(s, target, Address::DumpStyleLoadAddress,
fallback_style);
*s << ", range" << (m_ranges.size() > 1 ? "s" : "") << " = ";
Address::DumpStyle fallback_style =
level == eDescriptionLevelVerbose
? Address::DumpStyleModuleWithFileAddress
: Address::DumpStyleFileAddress;
for (const AddressRange &range : m_ranges)
range.Dump(s, target, Address::DumpStyleLoadAddress, fallback_style);
}

void Function::Dump(Stream *s, bool show_context) const {
Expand Down
Loading
Loading