labath created this revision.
labath added reviewers: JDevlieghere, clayborg, aprantl.
Herald added a subscriber: arphaman.

The offset of a compile unit is enough to identify in a section, but
things start to get a lot more complicated when you have multiple object
files, each with potentially several debug-info-carrying sections
floating around.

We were already pushing this concept to its limits split-dwarf, which
required juggling offsets of several units to get things to work.
However, this really become a problem when we tried to introduce
debug_types support, which resulted in a long series of attempts to
"concatenate" various debug info sections so that the unit offset
remains a valid identifier.

Instead of attempting to assign unique offsets to all units that we care
about, this patch does something more fundamental. We just admit that
using offsets as unique identifiers does not scale to all debug info
formats we want to support, and uses a different ID instead.

Decoupling the ID from the offset means the offset can be free to point
to the actual offset of the compile unit in the relevant section,
without any changes to low-level parsing code needed. And using a
surrogate ID means its much easier to map the compile units into a
single address space. At least two strategies are possible after this
patch:
a) map all relevant units into a single continuous sequence of integers
(as if the were all stored in one vector)
b) steal some bits from the cu_idx field and use it as some sort of a
discriminator (essentially creating multiple address spaces)

Additionally, this makes us better prepared for the future, where the
total size of debug info exceeds 4GB. It also simplifies code, as we can
treat different debug info flavours more uniformly.

This patch:

- renames the cu_offset field of DIERef to cu_idx and fixes all uses to treat 
it as such
- removes DWARFUnit::GetBaseObjOffset as it no longer serves any useful 
purpose. Instead the DWO unit is made to share the ID of the "base" unit, as 
they logically represent a single compilation.
- changes SymbolFileDWARFDwo to use the index of the contained unit as its ID 
(this is the same approach taken by SymbolFileDWARFDebugMap).
- has a longer commit message than the code it changes. :)


https://reviews.llvm.org/D61482

Files:
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -23,7 +23,7 @@
                                        DWARFUnit *dwarf_cu)
     : SymbolFileDWARF(objfile.get()), m_obj_file_sp(objfile),
       m_base_dwarf_cu(dwarf_cu) {
-  SetID(((lldb::user_id_t)dwarf_cu->GetOffset()) << 32);
+  SetID(((lldb::user_id_t)dwarf_cu->GetID()) << 32);
 }
 
 void SymbolFileDWARFDwo::LoadSectionData(lldb::SectionType sect_type,
@@ -158,6 +158,6 @@
 
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
-  lldbassert(m_base_dwarf_cu->GetOffset() == die_ref.cu_offset);
+  assert(m_base_dwarf_cu->GetID() == die_ref.cu_idx);
   return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset);
 }
Index: source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
@@ -38,13 +38,13 @@
   return m_map.GetValues(regex, info_array);
 }
 
-size_t NameToDIE::FindAllEntriesForCompileUnit(dw_offset_t cu_offset,
+size_t NameToDIE::FindAllEntriesForCompileUnit(dw_offset_t cu_idx,
                                                DIEArray &info_array) const {
   const size_t initial_size = info_array.size();
   const uint32_t size = m_map.GetSize();
   for (uint32_t i = 0; i < size; ++i) {
     const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-    if (cu_offset == die_ref.cu_offset)
+    if (cu_idx == die_ref.cu_idx)
       info_array.push_back(die_ref);
   }
   return info_array.size() - initial_size;
@@ -56,7 +56,7 @@
     ConstString cstr = m_map.GetCStringAtIndex(i);
     const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
     s->Printf("%p: {0x%8.8x/0x%8.8x} \"%s\"\n", (const void *)cstr.GetCString(),
-              die_ref.cu_offset, die_ref.die_offset, cstr.GetCString());
+              die_ref.cu_idx, die_ref.die_offset, cstr.GetCString());
   }
 }
 
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -60,7 +60,7 @@
   static void
   IndexUnitImpl(DWARFUnit &unit, const lldb::LanguageType cu_language,
                 const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
-                const dw_offset_t cu_offset, IndexSet &set);
+                IndexSet &set);
 
   /// Non-null value means we haven't built the index yet.
   DWARFDebugInfo *m_debug_info;
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -102,19 +102,19 @@
   const LanguageType cu_language = unit.GetLanguageType();
   DWARFFormValue::FixedFormSizes fixed_form_sizes = unit.GetFixedFormSizes();
 
-  IndexUnitImpl(unit, cu_language, fixed_form_sizes, unit.GetOffset(), set);
+  IndexUnitImpl(unit, cu_language, fixed_form_sizes, set);
 
   SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile();
   if (dwo_symbol_file && dwo_symbol_file->GetCompileUnit()) {
     IndexUnitImpl(*dwo_symbol_file->GetCompileUnit(), cu_language,
-                  fixed_form_sizes, unit.GetOffset(), set);
+                  fixed_form_sizes, set);
   }
 }
 
 void ManualDWARFIndex::IndexUnitImpl(
     DWARFUnit &unit, const LanguageType cu_language,
-    const DWARFFormValue::FixedFormSizes &fixed_form_sizes,
-    const dw_offset_t cu_offset, IndexSet &set) {
+    const DWARFFormValue::FixedFormSizes &fixed_form_sizes, IndexSet &set) {
+  user_id_t cu_offset = unit.GetID();
   for (const DWARFDebugInfoEntry &die : unit.dies()) {
     const dw_tag_t tag = die.Tag();
 
@@ -371,7 +371,7 @@
 void ManualDWARFIndex::GetGlobalVariables(const DWARFUnit &cu,
                                           DIEArray &offsets) {
   Index();
-  m_set.globals.FindAllEntriesForCompileUnit(cu.GetOffset(), offsets);
+  m_set.globals.FindAllEntriesForCompileUnit(cu.GetID(), offsets);
 }
 
 void ManualDWARFIndex::GetObjCMethods(ConstString class_name,
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -66,7 +66,7 @@
   uint64_t die_bias = cu->GetDwoSymbolFile() ? 0 : *cu_offset;
 
   if (llvm::Optional<uint64_t> die_offset = entry.getDIEUnitOffset())
-    return DIERef(*cu_offset, die_bias + *die_offset);
+    return DIERef(cu->GetID(), die_bias + *die_offset);
 
   return DIERef();
 }
@@ -164,7 +164,7 @@
     if (!ref)
       continue;
 
-    DWARFUnit *cu = m_debug_info.GetCompileUnitAtOffset(ref.cu_offset);
+    DWARFUnit *cu = m_debug_info.GetCompileUnitAtIndex(ref.cu_idx);
     if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) {
       incomplete_types.push_back(ref);
       continue;
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -104,7 +104,6 @@
   dw_addr_t GetStrOffsetsBase() const { return m_str_offsets_base; }
   void SetAddrBase(dw_addr_t addr_base);
   void SetRangesBase(dw_addr_t ranges_base);
-  void SetBaseObjOffset(dw_offset_t base_obj_offset);
   void SetStrOffsetsBase(dw_offset_t str_offsets_base);
   void BuildAddressRangeTable(SymbolFileDWARF *dwarf,
                               DWARFDebugAranges *debug_aranges);
@@ -160,8 +159,6 @@
 
   SymbolFileDWARFDwo *GetDwoSymbolFile() const;
 
-  dw_offset_t GetBaseObjOffset() const;
-
   die_iterator_range dies() {
     ExtractDIEsIfNeeded();
     return die_iterator_range(m_die_array.begin(), m_die_array.end());
@@ -207,7 +204,6 @@
   dw_addr_t m_ranges_base = 0; // Value of DW_AT_ranges_base
   // If this is a dwo compile unit this is the offset of the base compile unit
   // in the main object file
-  dw_offset_t m_base_obj_offset = DW_INVALID_OFFSET;
   dw_offset_t m_str_offsets_base = 0; // Value of DW_AT_str_offsets_base.
   // Offset of the initial length field.
   dw_offset_t m_offset;
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -332,7 +332,7 @@
                                                      DW_AT_GNU_ranges_base, 0);
   dwo_cu->SetRangesBase(ranges_base);
 
-  dwo_cu->SetBaseObjOffset(m_offset);
+  dwo_cu->SetID(GetID());
 
   SetDwoStrOffsetsBase(dwo_cu);
 }
@@ -388,10 +388,6 @@
   m_ranges_base = ranges_base;
 }
 
-void DWARFUnit::SetBaseObjOffset(dw_offset_t base_obj_offset) {
-  m_base_obj_offset = base_obj_offset;
-}
-
 void DWARFUnit::SetStrOffsetsBase(dw_offset_t str_offsets_base) {
   m_str_offsets_base = str_offsets_base;
 }
@@ -768,8 +764,6 @@
   return m_dwo_symbol_file.get();
 }
 
-dw_offset_t DWARFUnit::GetBaseObjOffset() const { return m_base_obj_offset; }
-
 const DWARFDebugAranges &DWARFUnit::GetFunctionAranges() {
   if (m_func_aranges_up == NULL) {
     m_func_aranges_up.reset(new DWARFDebugAranges());
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -157,10 +157,10 @@
 }
 
 DWARFUnit *DWARFDebugInfo::GetCompileUnit(const DIERef &die_ref) {
-  if (die_ref.cu_offset == DW_INVALID_OFFSET)
+  if (die_ref.cu_idx == DW_INVALID_OFFSET)
     return GetCompileUnitContainingDIEOffset(die_ref.die_offset);
   else
-    return GetCompileUnitAtOffset(die_ref.cu_offset);
+    return GetCompileUnitAtIndex(die_ref.cu_idx);
 }
 
 DWARFUnit *
Index: source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -21,10 +21,7 @@
   if (!IsValid())
     return DIERef();
 
-  dw_offset_t cu_offset = m_cu->GetOffset();
-  if (m_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
-    cu_offset = m_cu->GetBaseObjOffset();
-  return DIERef(cu_offset, m_die->GetOffset());
+  return DIERef(m_cu->GetID(), m_die->GetOffset());
 }
 
 dw_tag_t DWARFBaseDIE::Tag() const {
Index: source/Plugins/SymbolFile/DWARF/DIERef.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DIERef.h
+++ source/Plugins/SymbolFile/DWARF/DIERef.h
@@ -18,7 +18,7 @@
 struct DIERef {
   DIERef() = default;
 
-  DIERef(dw_offset_t c, dw_offset_t d) : cu_offset(c), die_offset(d) {}
+  DIERef(dw_offset_t c, dw_offset_t d) : cu_idx(c), die_offset(d) {}
 
   // In order to properly decode a lldb::user_id_t back into a DIERef we
   // need the DWARF file since it knows if DWARF in .o files is being used
@@ -41,10 +41,10 @@
   bool operator<(const DIERef &ref) { return die_offset < ref.die_offset; }
 
   explicit operator bool() const {
-    return cu_offset != DW_INVALID_OFFSET || die_offset != DW_INVALID_OFFSET;
+    return cu_idx != DW_INVALID_OFFSET || die_offset != DW_INVALID_OFFSET;
   }
 
-  dw_offset_t cu_offset = DW_INVALID_OFFSET;
+  dw_offset_t cu_idx = DW_INVALID_OFFSET;
   dw_offset_t die_offset = DW_INVALID_OFFSET;
 };
 
Index: source/Plugins/SymbolFile/DWARF/DIERef.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DIERef.cpp
+++ source/Plugins/SymbolFile/DWARF/DIERef.cpp
@@ -14,7 +14,7 @@
 #include "SymbolFileDWARFDebugMap.h"
 
 DIERef::DIERef(lldb::user_id_t uid, SymbolFileDWARF *dwarf)
-    : cu_offset(DW_INVALID_OFFSET), die_offset(uid & 0xffffffff) {
+    : die_offset(uid & 0xffffffff) {
   SymbolFileDWARFDebugMap *debug_map = dwarf->GetDebugMapSymfile();
   if (debug_map) {
     const uint32_t oso_idx = debug_map->GetOSOIndexFromUserID(uid);
@@ -25,27 +25,22 @@
         DWARFUnit *dwarf_cu =
             debug_info->GetCompileUnitContainingDIEOffset(die_offset);
         if (dwarf_cu) {
-          cu_offset = dwarf_cu->GetOffset();
+          cu_idx = dwarf_cu->GetOffset();
           return;
         }
       }
     }
     die_offset = DW_INVALID_OFFSET;
   } else {
-    cu_offset = uid >> 32;
+    cu_idx = uid >> 32;
   }
 }
 
-DIERef::DIERef(const DWARFFormValue &form_value)
-    : cu_offset(DW_INVALID_OFFSET), die_offset(DW_INVALID_OFFSET) {
+DIERef::DIERef(const DWARFFormValue &form_value) {
   if (form_value.IsValid()) {
     const DWARFUnit *dwarf_cu = form_value.GetCompileUnit();
-    if (dwarf_cu) {
-      if (dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
-        cu_offset = dwarf_cu->GetBaseObjOffset();
-      else
-        cu_offset = dwarf_cu->GetOffset();
-    }
+    if (dwarf_cu)
+      cu_idx = dwarf_cu->GetID();
     die_offset = form_value.Reference();
   }
 }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to