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 <pa...@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); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits