https://github.com/labath created
https://github.com/llvm/llvm-project/pull/117581
In case of an error GetBlock would return a reference to a Block without adding
it to a parent. This doesn't seem like a good idea, and none of the other
plugins do that.
This patch fixes that by propagating errors (well, null pointers...) up the
stack.
I don't know of any specific problem that this solves, but given that this
occurs only when something goes very wrong (e.g. a corrupted PDB file), it's
quite possible noone has run into this situation, so we can't say the code is
correct either. It also gets in the way of a refactor I'm contemplating.
>From 704eaf250480e0f74e4f135d61b7e66c3356eb97 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Mon, 25 Nov 2024 18:05:54 +0100
Subject: [PATCH] [lldb/NativePDB] Don't create parentless blocks
In case of an error GetBlock would return a reference to a Block
without adding it to a parent. This doesn't seem like a good idea, and
none of the other plugins do that.
This patch fixes that by propagating errors (well, null pointers...) up
the stack.
I don't know of any specific problem that this solves, but given that
this occurs only when something goes very wrong (e.g. a corrupted PDB
file), it's quite possible noone has run into this situation, so we
can't say the code is correct either. It also gets in the way of a
refactor I'm contemplating.
---
.../NativePDB/SymbolFileNativePDB.cpp | 59 +++++++++++--------
.../NativePDB/SymbolFileNativePDB.h | 4 +-
2 files changed, 38 insertions(+), 25 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index c0416b4d06815d..8b02e6369c59c7 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -379,18 +379,17 @@ uint32_t SymbolFileNativePDB::CalculateNumCompileUnits() {
return count;
}
-Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) {
+Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) {
CompilandIndexItem *cii = m_index->compilands().GetCompiland(block_id.modi);
CVSymbol sym = cii->m_debug_stream.readSymbolAtOffset(block_id.offset);
CompUnitSP comp_unit = GetOrCreateCompileUnit(*cii);
lldb::user_id_t opaque_block_uid = toOpaqueUid(block_id);
- BlockSP child_block = std::make_shared<Block>(opaque_block_uid);
auto ts_or_err = GetTypeSystemForLanguage(comp_unit->GetLanguage());
if (auto err = ts_or_err.takeError())
- return *child_block;
+ return nullptr;
auto ts = *ts_or_err;
if (!ts)
- return *child_block;
+ return nullptr;
PdbAstBuilder* ast_builder = ts->GetNativePDBParser();
switch (sym.kind()) {
@@ -403,7 +402,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId
block_id) {
Block &block = func->GetBlock(false);
if (block.GetNumRanges() == 0)
block.AddRange(Block::Range(0, func->GetAddressRange().GetByteSize()));
- return block;
+ return █
}
break;
}
@@ -416,13 +415,16 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId
block_id) {
cantFail(SymbolDeserializer::deserializeAs<BlockSym>(sym, block));
lldbassert(block.Parent != 0);
PdbCompilandSymId parent_id(block_id.modi, block.Parent);
- Block &parent_block = GetOrCreateBlock(parent_id);
- Function *func = parent_block.CalculateSymbolContextFunction();
+ Block *parent_block = GetOrCreateBlock(parent_id);
+ if (!parent_block)
+ return nullptr;
+ Function *func = parent_block->CalculateSymbolContextFunction();
lldbassert(func);
lldb::addr_t block_base =
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);
if (block_base >= func_base)
child_block->AddRange(Block::Range(block_base - func_base,
block.CodeSize));
else {
@@ -435,7 +437,7 @@ 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);
+ parent_block->AddChild(child_block);
ast_builder->GetOrCreateBlockDecl(block_id);
m_blocks.insert({opaque_block_uid, child_block});
break;
@@ -445,8 +447,11 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId
block_id) {
comp_unit->GetLineTable();
std::shared_ptr<InlineSite> inline_site = m_inline_sites[opaque_block_uid];
- Block &parent_block = GetOrCreateBlock(inline_site->parent_id);
- parent_block.AddChild(child_block);
+ 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);
ast_builder->GetOrCreateInlinedFunctionDecl(block_id);
// Copy ranges from InlineSite to Block.
for (size_t i = 0; i < inline_site->ranges.GetSize(); ++i) {
@@ -469,7 +474,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId
block_id) {
lldbassert(false && "Symbol is not a block!");
}
- return *child_block;
+ return nullptr;
}
lldb::FunctionSP SymbolFileNativePDB::CreateFunction(PdbCompilandSymId func_id,
@@ -997,10 +1002,10 @@ SymbolFileNativePDB::GetOrCreateCompileUnit(const
CompilandIndexItem &cci) {
return emplace_result.first->second;
}
-Block &SymbolFileNativePDB::GetOrCreateBlock(PdbCompilandSymId block_id) {
+Block *SymbolFileNativePDB::GetOrCreateBlock(PdbCompilandSymId block_id) {
auto iter = m_blocks.find(toOpaqueUid(block_id));
if (iter != m_blocks.end())
- return *iter->second;
+ return iter->second.get();
return CreateBlock(block_id);
}
@@ -1124,14 +1129,16 @@ uint32_t SymbolFileNativePDB::ResolveSymbolContext(
}
if (type == PDB_SymType::Block) {
- Block &block = GetOrCreateBlock(csid);
- sc.function = block.CalculateSymbolContextFunction();
+ Block *block = GetOrCreateBlock(csid);
+ if (!block)
+ continue;
+ sc.function = block->CalculateSymbolContextFunction();
if (sc.function) {
sc.function->GetBlock(true);
addr_t func_base =
sc.function->GetAddressRange().GetBaseAddress().GetFileAddress();
addr_t offset = file_addr - func_base;
- sc.block = block.FindInnermostBlockByOffset(offset);
+ sc.block = block->FindInnermostBlockByOffset(offset);
}
}
if (sc.function)
@@ -1837,12 +1844,16 @@ VariableSP
SymbolFileNativePDB::CreateLocalVariable(PdbCompilandSymId scope_id,
PdbCompilandSymId var_id,
bool is_param) {
ModuleSP module = GetObjectFile()->GetModule();
- Block &block = GetOrCreateBlock(scope_id);
+ Block *block = GetOrCreateBlock(scope_id);
+ if (!block)
+ return nullptr;
+
// Get function block.
- Block *func_block = █
+ Block *func_block = block;
while (func_block->GetParent()) {
func_block = func_block->GetParent();
}
+
Address addr;
func_block->GetStartAddress(addr);
VariableInfo var_info =
@@ -1876,7 +1887,7 @@ VariableSP
SymbolFileNativePDB::CreateLocalVariable(PdbCompilandSymId scope_id,
Variable::RangeList scope_ranges;
VariableSP var_sp = std::make_shared<Variable>(
toOpaqueUid(var_id), name.c_str(), name.c_str(), sftype, var_scope,
- &block, scope_ranges, &decl, var_info.location, external, artificial,
+ block, scope_ranges, &decl, var_info.location, external, artificial,
location_is_constant_data, static_member);
if (!is_param) {
auto ts_or_err = GetTypeSystemForLanguage(comp_unit_sp->GetLanguage());
@@ -1935,7 +1946,9 @@ TypeSP
SymbolFileNativePDB::GetOrCreateTypedef(PdbGlobalSymId id) {
}
size_t SymbolFileNativePDB::ParseVariablesForBlock(PdbCompilandSymId block_id)
{
- Block &block = GetOrCreateBlock(block_id);
+ Block *block = GetOrCreateBlock(block_id);
+ if (!block)
+ return 0;
size_t count = 0;
@@ -1977,10 +1990,10 @@ size_t
SymbolFileNativePDB::ParseVariablesForBlock(PdbCompilandSymId block_id) {
return 0;
}
- VariableListSP variables = block.GetBlockVariableList(false);
+ VariableListSP variables = block->GetBlockVariableList(false);
if (!variables) {
variables = std::make_shared<VariableList>();
- block.SetVariableList(variables);
+ block->SetVariableList(variables);
}
CVSymbolArray syms = limitSymbolArrayToScope(
@@ -2027,7 +2040,7 @@ size_t
SymbolFileNativePDB::ParseVariablesForBlock(PdbCompilandSymId block_id) {
// Pass false for set_children, since we call this recursively so that the
// children will call this for themselves.
- block.SetDidParseVariables(true, false);
+ block->SetDidParseVariables(true, false);
return count;
}
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
index 669c44aa131edc..b0e78a243a3c24 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -226,7 +226,7 @@ class SymbolFileNativePDB : public SymbolFileCommon {
lldb::TypeSP GetOrCreateType(PdbTypeSymId type_id);
lldb::TypeSP GetOrCreateType(llvm::codeview::TypeIndex ti);
lldb::VariableSP GetOrCreateGlobalVariable(PdbGlobalSymId var_id);
- Block &GetOrCreateBlock(PdbCompilandSymId block_id);
+ Block *GetOrCreateBlock(PdbCompilandSymId block_id);
lldb::VariableSP GetOrCreateLocalVariable(PdbCompilandSymId scope_id,
PdbCompilandSymId var_id,
bool is_param);
@@ -234,7 +234,7 @@ class SymbolFileNativePDB : public SymbolFileCommon {
lldb::FunctionSP CreateFunction(PdbCompilandSymId func_id,
CompileUnit &comp_unit);
- Block &CreateBlock(PdbCompilandSymId block_id);
+ Block *CreateBlock(PdbCompilandSymId block_id);
lldb::VariableSP CreateLocalVariable(PdbCompilandSymId scope_id,
PdbCompilandSymId var_id, bool
is_param);
lldb::TypeSP CreateTypedef(PdbGlobalSymId id);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
