[Lldb-commits] [lldb] [lldb] Make sure Blocks always have a parent (PR #117683)
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 26 00:09:15 PST 2024
https://github.com/labath created https://github.com/llvm/llvm-project/pull/117683
It's basically true already (except for a brief time during construction). This patch makes sure the objects are constructed with a valid parent and enforces this in the type system, which allows us to get rid of some nullptr checks.
>From 458e98135c15550ba6cd140aee8582c19e6a8c8d Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Tue, 26 Nov 2024 09:03:01 +0100
Subject: [PATCH] [lldb] Make sure Blocks always have a parent
It's basically true already (except for a brief time during
construction). This patch makes sure the objects are constructed with a
valid parent and enforces this in the type system, which allows us to
get rid of some nullptr checks.
---
lldb/include/lldb/Symbol/Block.h | 34 +++++---------
.../Breakpad/SymbolFileBreakpad.cpp | 4 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 +-
.../NativePDB/SymbolFileNativePDB.cpp | 6 +--
.../Plugins/SymbolFile/PDB/SymbolFilePDB.cpp | 4 +-
lldb/source/Symbol/Block.cpp | 47 +++++++------------
lldb/source/Symbol/Function.cpp | 3 +-
7 files changed, 36 insertions(+), 66 deletions(-)
diff --git a/lldb/include/lldb/Symbol/Block.h b/lldb/include/lldb/Symbol/Block.h
index c9c4d5ad767d7e..7c7a66de831998 100644
--- a/lldb/include/lldb/Symbol/Block.h
+++ b/lldb/include/lldb/Symbol/Block.h
@@ -43,11 +43,13 @@ class Block : public UserID, public SymbolContextScope {
typedef RangeVector<uint32_t, uint32_t, 1> RangeList;
typedef RangeList::Entry Range;
- /// Construct with a User ID \a uid, \a depth.
- ///
- /// Initialize this block with the specified UID \a uid. The \a depth in the
- /// \a block_list is used to represent the parent, sibling, and child block
- /// information and also allows for partial parsing at the block level.
+ // Creates a block representing the whole function. Only meant to be used from
+ // the Function class.
+ Block(Function &function, lldb::user_id_t function_uid);
+
+ ~Block() override;
+
+ /// Creates a block with the specified UID \a uid.
///
/// \param[in] uid
/// The UID for a given block. This value is given by the
@@ -56,19 +58,7 @@ class Block : public UserID, public SymbolContextScope {
/// information data that it parses for further or more in
/// depth parsing. Common values would be the index into a
/// table, or an offset into the debug information.
- ///
- /// \see BlockList
- Block(lldb::user_id_t uid);
-
- /// Destructor.
- ~Block() override;
-
- /// Add a child to this object.
- ///
- /// \param[in] child_block_sp
- /// A shared pointer to a child block that will get added to
- /// this block.
- void AddChild(const lldb::BlockSP &child_block_sp);
+ lldb::BlockSP CreateChild(lldb::user_id_t uid);
/// Add a new offset range to this block.
void AddRange(const Range &range);
@@ -317,10 +307,6 @@ class Block : public UserID, public SymbolContextScope {
const Declaration *decl_ptr,
const Declaration *call_decl_ptr);
- void SetParentScope(SymbolContextScope *parent_scope) {
- m_parent_scope = parent_scope;
- }
-
/// Set accessor for the variable list.
///
/// Called by the SymbolFile plug-ins after they have parsed the variable
@@ -364,7 +350,7 @@ class Block : public UserID, public SymbolContextScope {
protected:
typedef std::vector<lldb::BlockSP> collection;
// Member variables.
- SymbolContextScope *m_parent_scope;
+ SymbolContextScope &m_parent_scope;
collection m_children;
RangeList m_ranges;
lldb::InlineFunctionInfoSP m_inlineInfoSP; ///< Inlined function information.
@@ -382,6 +368,8 @@ class Block : public UserID, public SymbolContextScope {
private:
Block(const Block &) = delete;
const Block &operator=(const Block &) = delete;
+
+ Block(lldb::user_id_t uid, SymbolContextScope &parent_scope);
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index fadc19676609bf..df3bf157278daf 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -315,7 +315,8 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function &func) {
if (record->InlineNestLevel == 0 ||
record->InlineNestLevel <= last_added_nest_level + 1) {
last_added_nest_level = record->InlineNestLevel;
- BlockSP block_sp = std::make_shared<Block>(It.GetBookmark().offset);
+ BlockSP block_sp = blocks[record->InlineNestLevel]->CreateChild(
+ It.GetBookmark().offset);
FileSpec callsite_file;
if (record->CallSiteFileNum < m_files->size())
callsite_file = (*m_files)[record->CallSiteFileNum];
@@ -333,7 +334,6 @@ size_t SymbolFileBreakpad::ParseBlocksRecursive(Function &func) {
}
block_sp->FinalizeRanges();
- blocks[record->InlineNestLevel]->AddChild(block_sp);
if (record->InlineNestLevel + 1 >= blocks.size()) {
blocks.resize(blocks.size() + 1);
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c900f330b481bb..fe711c56958c44 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1328,9 +1328,7 @@ size_t SymbolFileDWARF::ParseBlocksRecursive(
block = parent_block;
} else {
- BlockSP block_sp(new Block(die.GetID()));
- parent_block->AddChild(block_sp);
- block = block_sp.get();
+ block = parent_block->CreateChild(die.GetID()).get();
}
DWARFRangeList ranges;
const char *name = nullptr;
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 0f77b2e28004eb..d17fedf26b4c48 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -424,7 +424,7 @@ Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) {
m_index->MakeVirtualAddress(block.Segment, block.CodeOffset);
lldb::addr_t func_base =
func->GetAddressRange().GetBaseAddress().GetFileAddress();
- BlockSP child_block = std::make_shared<Block>(opaque_block_uid);
+ BlockSP child_block = parent_block->CreateChild(opaque_block_uid);
if (block_base >= func_base)
child_block->AddRange(Block::Range(block_base - func_base, block.CodeSize));
else {
@@ -437,7 +437,6 @@ Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) {
block_id.modi, block_id.offset, block_base,
block_base + block.CodeSize, func_base);
}
- parent_block->AddChild(child_block);
ast_builder->GetOrCreateBlockDecl(block_id);
m_blocks.insert({opaque_block_uid, child_block});
break;
@@ -450,8 +449,7 @@ Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) {
Block *parent_block = GetOrCreateBlock(inline_site->parent_id);
if (!parent_block)
return nullptr;
- BlockSP child_block = std::make_shared<Block>(opaque_block_uid);
- parent_block->AddChild(child_block);
+ BlockSP child_block = parent_block->CreateChild(opaque_block_uid);
ast_builder->GetOrCreateInlinedFunctionDecl(block_id);
// Copy ranges from InlineSite to Block.
for (size_t i = 0; i < inline_site->ranges.GetSize(); ++i) {
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index c7b23aedfdccac..4935b0fbdfd877 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -421,9 +421,7 @@ static size_t ParseFunctionBlocksForPDBSymbol(
if (raw_sym.getVirtualAddress() < func_file_vm_addr)
break;
- auto block_sp = std::make_shared<Block>(pdb_symbol->getSymIndexId());
- parent_block->AddChild(block_sp);
- block = block_sp.get();
+ block = parent_block->CreateChild(pdb_symbol->getSymIndexId()).get();
} else
llvm_unreachable("Unexpected PDB symbol!");
diff --git a/lldb/source/Symbol/Block.cpp b/lldb/source/Symbol/Block.cpp
index 8746a25e3fad5a..5cc87240f552ad 100644
--- a/lldb/source/Symbol/Block.cpp
+++ b/lldb/source/Symbol/Block.cpp
@@ -21,9 +21,11 @@
using namespace lldb;
using namespace lldb_private;
-Block::Block(lldb::user_id_t uid)
- : UserID(uid), m_parent_scope(nullptr), m_children(), m_ranges(),
- m_inlineInfoSP(), m_variable_list_sp(), m_parsed_block_info(false),
+Block::Block(Function &function, user_id_t function_uid)
+ : Block(function_uid, function) {}
+
+Block::Block(lldb::user_id_t uid, SymbolContextScope &parent_scope)
+ : UserID(uid), m_parent_scope(parent_scope), m_parsed_block_info(false),
m_parsed_block_variables(false), m_parsed_child_blocks(false) {}
Block::~Block() = default;
@@ -134,27 +136,20 @@ Block *Block::FindInnermostBlockByOffset(const lldb::addr_t offset) {
}
void Block::CalculateSymbolContext(SymbolContext *sc) {
- if (m_parent_scope)
- m_parent_scope->CalculateSymbolContext(sc);
+ m_parent_scope.CalculateSymbolContext(sc);
sc->block = this;
}
lldb::ModuleSP Block::CalculateSymbolContextModule() {
- if (m_parent_scope)
- return m_parent_scope->CalculateSymbolContextModule();
- return lldb::ModuleSP();
+ return m_parent_scope.CalculateSymbolContextModule();
}
CompileUnit *Block::CalculateSymbolContextCompileUnit() {
- if (m_parent_scope)
- return m_parent_scope->CalculateSymbolContextCompileUnit();
- return nullptr;
+ return m_parent_scope.CalculateSymbolContextCompileUnit();
}
Function *Block::CalculateSymbolContextFunction() {
- if (m_parent_scope)
- return m_parent_scope->CalculateSymbolContextFunction();
- return nullptr;
+ return m_parent_scope.CalculateSymbolContextFunction();
}
Block *Block::CalculateSymbolContextBlock() { return this; }
@@ -200,9 +195,7 @@ bool Block::Contains(const Range &range) const {
}
Block *Block::GetParent() const {
- if (m_parent_scope)
- return m_parent_scope->CalculateSymbolContextBlock();
- return nullptr;
+ return m_parent_scope.CalculateSymbolContextBlock();
}
Block *Block::GetContainingInlinedBlock() {
@@ -354,8 +347,8 @@ void Block::AddRange(const Range &range) {
if (parent_block && !parent_block->Contains(range)) {
Log *log = GetLog(LLDBLog::Symbols);
if (log) {
- ModuleSP module_sp(m_parent_scope->CalculateSymbolContextModule());
- Function *function = m_parent_scope->CalculateSymbolContextFunction();
+ ModuleSP module_sp(m_parent_scope.CalculateSymbolContextModule());
+ Function *function = m_parent_scope.CalculateSymbolContextFunction();
const addr_t function_file_addr =
function->GetAddressRange().GetBaseAddress().GetFileAddress();
const addr_t block_start_addr = function_file_addr + range.GetRangeBase();
@@ -399,11 +392,9 @@ size_t Block::MemorySize() const {
return mem_size;
}
-void Block::AddChild(const BlockSP &child_block_sp) {
- if (child_block_sp) {
- child_block_sp->SetParentScope(this);
- m_children.push_back(child_block_sp);
- }
+BlockSP Block::CreateChild(user_id_t uid) {
+ m_children.push_back(std::shared_ptr<Block>(new Block(uid, *this)));
+ return m_children.back();
}
void Block::SetInlinedFunctionInfo(const char *name, const char *mangled,
@@ -520,13 +511,11 @@ void Block::SetDidParseVariables(bool b, bool set_children) {
}
Block *Block::GetSibling() const {
- if (m_parent_scope) {
- Block *parent_block = GetParent();
- if (parent_block)
- return parent_block->GetSiblingForChild(this);
- }
+ if (Block *parent_block = GetParent())
+ return parent_block->GetSiblingForChild(this);
return nullptr;
}
+
// A parent of child blocks can be asked to find a sibling block given
// one of its child blocks
Block *Block::GetSiblingForChild(const Block *child_block) const {
diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index 0186d63e72879c..b346749ca06ec3 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -278,10 +278,9 @@ Function::Function(CompileUnit *comp_unit, lldb::user_id_t func_uid,
lldb::user_id_t type_uid, const Mangled &mangled, Type *type,
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_type(type), m_mangled(mangled), m_block(*this, 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);
}
More information about the lldb-commits
mailing list