DWARF: Make DIERefs always valid

Summary:
This patch makes the DIERef class always valid by default constructor
and operator bool. This allows one to express the validity of a DIERef
in the type system. Places which are working with potentially-invalid
DIERefs have been updated to use Optional<DIERef> instead.

The constructor taking a DWARFFormValue was not needed, as all places
which were constructing a DIERef this way were immediately converting it
into a DWARFDIE or a user_id. This can be done without constructing an
intermediate DIERef.

Reviewers: JDevlieghere, clayborg, aprantl

Subscribers: arphaman, lldb-commits

Differential Revision: https://reviews.llvm.org/D63399

llvm-svn: 363767

lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp

@@ -7,22 +7,3 @@
 //===----------------------------------------------------------------------===//
 
 #include "DIERef.h"
-#include "DWARFUnit.h"
-#include "DWARFDebugInfo.h"
-#include "DWARFFormValue.h"
-#include "SymbolFileDWARF.h"
-#include "SymbolFileDWARFDebugMap.h"
-
-DIERef::DIERef(const DWARFFormValue &form_value) {
-  if (form_value.IsValid()) {
-    DWARFDIE die = form_value.Reference();
-    die_offset = die.GetOffset();
-    if (die) {
-      section = die.GetCU()->GetDebugSection();
-      if (die.GetCU()->GetBaseObjOffset() != DW_INVALID_OFFSET)
-        cu_offset = die.GetCU()->GetBaseObjOffset();
-      else
-        cu_offset = die.GetCU()->GetOffset();
-    }
-  }
-}

lldb/source/Plugins/SymbolFile/DWARF/DIERef.h

@@ -10,28 +10,17 @@
 #define SymbolFileDWARF_DIERef_h_
 
 #include "lldb/Core/dwarf.h"
-#include "lldb/lldb-defines.h"
-
-class DWARFFormValue;
-class SymbolFileDWARF;
+#include <vector>
 
 struct DIERef {
   enum Section : uint8_t { DebugInfo, DebugTypes };
 
-  DIERef() = default;
-
   DIERef(Section s, dw_offset_t c, dw_offset_t d)
       : section(s), cu_offset(c), die_offset(d) {}
 
-  explicit DIERef(const DWARFFormValue &form_value);
-
-  explicit operator bool() const {
-    return cu_offset != DW_INVALID_OFFSET || die_offset != DW_INVALID_OFFSET;
-  }
-
-  Section section = Section::DebugInfo;
-  dw_offset_t cu_offset = DW_INVALID_OFFSET;
-  dw_offset_t die_offset = DW_INVALID_OFFSET;
+  Section section;
+  dw_offset_t cu_offset;
+  dw_offset_t die_offset;
 };
 
 typedef std::vector<DIERef> DIEArray;

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

@@ -651,7 +651,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
 
     type_sp = std::make_shared<Type>(
         die.GetID(), dwarf, attrs.name, attrs.byte_size, nullptr,
-        dwarf->GetUID(DIERef(attrs.type)), encoding_data_type, &attrs.decl,
+        dwarf->GetUID(attrs.type.Reference()), encoding_data_type, &attrs.decl,
         clang_type, resolve_state);
 
     dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
@@ -1077,7 +1077,8 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
         &m_ast, dwarf->GetForwardDeclDieToClangType().lookup(die.GetDIE()));
     if (!clang_type) {
       if (attrs.type.IsValid()) {
-        Type *enumerator_type = dwarf->ResolveTypeUID(DIERef(attrs.type));
+        Type *enumerator_type =
+            dwarf->ResolveTypeUID(attrs.type.Reference(), true);
         if (enumerator_type)
           enumerator_clang_type = enumerator_type->GetFullCompilerType();
       }
@@ -1106,8 +1107,8 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
 
     type_sp = std::make_shared<Type>(
         die.GetID(), dwarf, attrs.name, attrs.byte_size, nullptr,
-        dwarf->GetUID(DIERef(attrs.type)), Type::eEncodingIsUID, &attrs.decl,
-        clang_type, Type::eResolveStateForward);
+        dwarf->GetUID(attrs.type.Reference()), Type::eEncodingIsUID,
+        &attrs.decl, clang_type, Type::eResolveStateForward);
 
     if (ClangASTContext::StartTagDeclarationDefinition(clang_type)) {
       if (die.HasChildren()) {
@@ -1149,7 +1150,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
     Type *func_type = NULL;
 
     if (attrs.type.IsValid())
-      func_type = dwarf->ResolveTypeUID(DIERef(attrs.type));
+      func_type = dwarf->ResolveTypeUID(attrs.type.Reference(), true);
 
     if (func_type)
       return_clang_type = func_type->GetForwardCompilerType();
@@ -1293,8 +1294,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
               // If we have a specification, then the function type should
               // have been made with the specification and not with this
               // die.
-              DWARFDIE spec_die =
-                  dwarf->DebugInfo()->GetDIE(DIERef(attrs.specification));
+              DWARFDIE spec_die = attrs.specification.Reference();
               clang::DeclContext *spec_clang_decl_ctx =
                   GetClangDeclContextForDIE(spec_die);
               if (spec_clang_decl_ctx) {
@@ -1303,7 +1303,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
                 dwarf->GetObjectFile()->GetModule()->ReportWarning(
                     "0x%8.8" PRIx64 ": DW_AT_specification(0x%8.8x"
                     ") has no decl\n",
-                    die.GetID(), attrs.specification.Reference().GetOffset());
+                    die.GetID(), spec_die.GetOffset());
               }
               type_handled = true;
             } else if (attrs.abstract_origin.IsValid()) {
@@ -1313,8 +1313,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
               // the abstract origin has a valid clang decl context.
               class_type->GetForwardCompilerType();
 
-              DWARFDIE abs_die =
-                  dwarf->DebugInfo()->GetDIE(DIERef(attrs.abstract_origin));
+              DWARFDIE abs_die = attrs.abstract_origin.Reference();
               clang::DeclContext *abs_clang_decl_ctx =
                   GetClangDeclContextForDIE(abs_die);
               if (abs_clang_decl_ctx) {
@@ -1323,7 +1322,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
                 dwarf->GetObjectFile()->GetModule()->ReportWarning(
                     "0x%8.8" PRIx64 ": DW_AT_abstract_origin(0x%8.8x"
                     ") has no decl\n",
-                    die.GetID(), attrs.abstract_origin.Reference().GetOffset());
+                    die.GetID(), abs_die.GetOffset());
               }
               type_handled = true;
             } else {
@@ -1542,8 +1541,8 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
     DEBUG_PRINTF("0x%8.8" PRIx64 ": %s (\"%s\")\n", die.GetID(),
                  DW_TAG_value_to_name(tag), type_name_cstr);
 
-    DIERef type_die_ref(attrs.type);
-    Type *element_type = dwarf->ResolveTypeUID(type_die_ref);
+    DWARFDIE type_die = attrs.type.Reference();
+    Type *element_type = dwarf->ResolveTypeUID(type_die, true);
 
     if (element_type) {
       auto array_info = ParseChildArrayInfo(die);
@@ -1566,15 +1565,15 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
                 "forward declaration, not a complete definition.\nTry "
                 "compiling the source file with -fstandalone-debug or "
                 "disable -gmodules",
-                die.GetOffset(), type_die_ref.die_offset);
+                die.GetOffset(), type_die.GetOffset());
           else
             module_sp->ReportError(
                 "DWARF DW_TAG_array_type DIE at 0x%8.8x has a "
                 "class/union/struct element type DIE 0x%8.8x that is a "
                 "forward declaration, not a complete definition.\nPlease "
                 "file a bug against the compiler and include the "
                 "preprocessed output for %s",
-                die.GetOffset(), type_die_ref.die_offset,
+                die.GetOffset(), type_die.GetOffset(),
                 GetUnitName(die).c_str());
         }
 
@@ -1591,7 +1590,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
                                  "start its definition.\nPlease file a "
                                  "bug and attach the file at the start "
                                  "of this error message",
-                                 type_die_ref.die_offset);
+                                 type_die.GetOffset());
         }
       }
 
@@ -1616,16 +1615,17 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const SymbolContext &sc,
       ConstString empty_name;
       type_sp = std::make_shared<Type>(
           die.GetID(), dwarf, empty_name, array_element_bit_stride / 8, nullptr,
-          dwarf->GetUID(type_die_ref), Type::eEncodingIsUID, &attrs.decl,
+          dwarf->GetUID(type_die), Type::eEncodingIsUID, &attrs.decl,
           clang_type, Type::eResolveStateFull);
       type_sp->SetEncodingType(element_type);
       m_ast.SetMetadataAsUserID(clang_type.GetOpaqueQualType(), die.GetID());
     }
   } break;
 
   case DW_TAG_ptr_to_member_type: {
-    Type *pointee_type = dwarf->ResolveTypeUID(DIERef(attrs.type));
-    Type *class_type = dwarf->ResolveTypeUID(DIERef(attrs.containing_type));
+    Type *pointee_type = dwarf->ResolveTypeUID(attrs.type.Reference(), true);
+    Type *class_type =
+        dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
     CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
     CompilerType class_clang_type = class_type->GetLayoutCompilerType();
@@ -1792,7 +1792,7 @@ bool DWARFASTParserClang::ParseTemplateDIE(
 
         case DW_AT_type:
           if (attributes.ExtractFormValueAtIndex(i, form_value)) {
-            Type *lldb_type = die.ResolveTypeUID(DIERef(form_value));
+            Type *lldb_type = die.ResolveTypeUID(form_value.Reference());
             if (lldb_type)
               clang_type = lldb_type->GetForwardCompilerType();
           }
@@ -2634,7 +2634,7 @@ bool DWARFASTParserClang::ParseChildMembers(
 
         // Handle static members
         if (is_external && member_byte_offset == UINT32_MAX) {
-          Type *var_type = die.ResolveTypeUID(DIERef(encoding_form));
+          Type *var_type = die.ResolveTypeUID(encoding_form.Reference());
 
           if (var_type) {
             if (accessibility == eAccessNone)
@@ -2647,7 +2647,7 @@ bool DWARFASTParserClang::ParseChildMembers(
         }
 
         if (!is_artificial) {
-          Type *member_type = die.ResolveTypeUID(DIERef(encoding_form));
+          Type *member_type = die.ResolveTypeUID(encoding_form.Reference());
 
           clang::FieldDecl *field_decl = nullptr;
           if (tag == DW_TAG_member) {
@@ -2997,7 +2997,7 @@ bool DWARFASTParserClang::ParseChildMembers(
           }
         }
 
-        Type *base_class_type = die.ResolveTypeUID(DIERef(encoding_form));
+        Type *base_class_type = die.ResolveTypeUID(encoding_form.Reference());
         if (base_class_type == nullptr) {
           module_sp->ReportError("0x%8.8x: DW_TAG_inheritance failed to "
                                  "resolve the base class at 0x%8.8x"
@@ -3121,7 +3121,8 @@ size_t DWARFASTParserClang::ParseChildParameters(
               // specification DIEs, so we can't rely upon the name being in
               // the formal parameter DIE...
               (name == nullptr || ::strcmp(name, "this") == 0)) {
-            Type *this_type = die.ResolveTypeUID(DIERef(param_type_die_form));
+            Type *this_type =
+                die.ResolveTypeUID(param_type_die_form.Reference());
             if (this_type) {
               uint32_t encoding_mask = this_type->GetEncodingMask();
               if (encoding_mask & Type::eEncodingIsPointerUID) {
@@ -3138,7 +3139,7 @@ size_t DWARFASTParserClang::ParseChildParameters(
         }
 
         if (!skip) {
-          Type *type = die.ResolveTypeUID(DIERef(param_type_die_form));
+          Type *type = die.ResolveTypeUID(param_type_die_form.Reference());
           if (type) {
             function_param_types.push_back(type->GetForwardCompilerType());
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp

@@ -17,9 +17,9 @@
 
 using namespace lldb_private;
 
-DIERef DWARFBaseDIE::GetDIERef() const {
+llvm::Optional<DIERef> DWARFBaseDIE::GetDIERef() const {
   if (!IsValid())
-    return DIERef();
+    return llvm::None;
 
   dw_offset_t cu_offset = m_cu->GetOffset();
   if (m_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)

lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h

@@ -54,7 +54,7 @@ class DWARFBaseDIE {
 
   DWARFDebugInfoEntry *GetDIE() const { return m_die; }
 
-  DIERef GetDIERef() const;
+  llvm::Optional<DIERef> GetDIERef() const;
 
   lldb_private::TypeSystem *GetTypeSystem() const;
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp

@@ -313,12 +313,10 @@ lldb_private::Type *DWARFDIE::ResolveType() const {
     return nullptr;
 }
 
-lldb_private::Type *DWARFDIE::ResolveTypeUID(const DIERef &die_ref) const {
-  SymbolFileDWARF *dwarf = GetDWARF();
-  if (dwarf)
-    return dwarf->ResolveTypeUID(dwarf->GetDIE(die_ref), true);
-  else
-    return nullptr;
+lldb_private::Type *DWARFDIE::ResolveTypeUID(const DWARFDIE &die) const {
+  if (SymbolFileDWARF *dwarf = GetDWARF())
+    return dwarf->ResolveTypeUID(die, true);
+  return nullptr;
 }
 
 std::vector<DWARFDIE> DWARFDIE::GetDeclContextDIEs() const {

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h

@@ -42,7 +42,7 @@ class DWARFDIE : public DWARFBaseDIE {
   lldb_private::Type *ResolveType() const;
 
   // Resolve a type by UID using this DIE's DWARF file
-  lldb_private::Type *ResolveTypeUID(const DIERef &die_ref) const;
+  lldb_private::Type *ResolveTypeUID(const DWARFDIE &die) const;
 
   // Functions for obtaining DIE relations and references
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

@@ -234,7 +234,7 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
 
   dw_addr_t lo_pc = LLDB_INVALID_ADDRESS;
   dw_addr_t hi_pc = LLDB_INVALID_ADDRESS;
-  std::vector<DIERef> die_refs;
+  std::vector<DWARFDIE> dies;
   bool set_frame_base_loclist_addr = false;
 
   auto abbrevDecl = GetAbbreviationDeclarationPtr(cu);
@@ -302,11 +302,11 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
           break;
 
         case DW_AT_abstract_origin:
-          die_refs.emplace_back(form_value);
+          dies.push_back(form_value.Reference());
           break;
 
         case DW_AT_specification:
-          die_refs.emplace_back(form_value);
+          dies.push_back(form_value.Reference());
           break;
 
         case DW_AT_decl_file:
@@ -392,13 +392,11 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
   }
 
   if (ranges.IsEmpty() || name == nullptr || mangled == nullptr) {
-    for (const DIERef &die_ref : die_refs) {
-      if (die_ref.die_offset != DW_INVALID_OFFSET) {
-        DWARFDIE die = dwarf.GetDIE(die_ref);
-        if (die)
-          die.GetDIE()->GetDIENamesAndRanges(die.GetCU(), name, mangled, ranges,
-                                             decl_file, decl_line, decl_column,
-                                             call_file, call_line, call_column);
+    for (const DWARFDIE &die : dies) {
+      if (die) {
+        die.GetDIE()->GetDIENamesAndRanges(die.GetCU(), name, mangled, ranges,
+                                           decl_file, decl_line, decl_column,
+                                           call_file, call_line, call_column);
       }
     }
   }