[lldb/NativePDB] Don't create parentless blocks (#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.

Author: labath

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 &block;
     }
     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;
+  Block *func_block = block;
   while (func_block->GetParent()) {
     func_block = func_block->GetParent();
   }
+
   Address addr;
   func_block->GetStartAddress(addr);
   VariableInfo var_info =
@@ -1875,8 +1886,8 @@ VariableSP SymbolFileNativePDB::CreateLocalVariable(PdbCompilandSymId scope_id,
   bool static_member = false;
   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,
+      toOpaqueUid(var_id), name.c_str(), name.c_str(), sftype, var_scope, 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;
 }

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

@@ -226,15 +226,15 @@ 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);
   lldb::TypeSP GetOrCreateTypedef(PdbGlobalSymId id);
 
   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);