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

This patch adds the ability to precisely address debug info in
situations when a single file can have more than one debug-info-bearing
sections (as is the case with type units in DWARF v4).

The changes here can be classified into roughly three categories:

- the code which addresses a debug info by offset gets an additional argument, 
which specifies the section one should look into.
- the DIERef class also gets an additional member variable specifying the 
section. This way, code dealing with DIERefs can know which section is the 
object referring to.
- the user_id_t encoding steals one bit from the dwarf_id field to store the 
section. This means the total number of separate object files (apple .o, or 
normal .dwo) is limited to 2 billion, but that is fine as it's not possible to 
hit that number without switching to DWARF64 anyway.

This patch is functionally equivalent to (and inspired by) the two
patches (D61503 <https://reviews.llvm.org/D61503> and D61504 
<https://reviews.llvm.org/D61504>) by Jan Kratochvil, but there are differences
in the implementation:

- it uses an enum instead of a bool flag to differentiate the sections
- it increases the size of DIERef struct instead of reducing the amount of 
addressable debug info
- it sets up DWARFDebugInfo to store the units in a single vector instead of 
two. This sets us up for the future in which type units can also live in the 
debug_info section, and I believe it's cleaner because there's no need for unit 
index remapping

There are no tests with this patch as this is essentially NFC until
we start parsing type units from the debug_types section.


https://reviews.llvm.org/D61908

Files:
  lit/SymbolFile/DWARF/array-sizes.s
  lit/SymbolFile/DWARF/dwarf5_locations.s
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  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
@@ -160,5 +160,5 @@
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
   lldbassert(die_ref.cu_offset == m_base_dwarf_cu->GetOffset() ||
              die_ref.cu_offset == DW_INVALID_OFFSET);
-  return DebugInfo()->GetDIEForDIEOffset(die_ref.die_offset);
+  return DebugInfo()->GetDIEForDIEOffset(die_ref.section, die_ref.die_offset);
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -287,10 +287,13 @@
   DWARFDIE GetDIE(lldb::user_id_t uid);
 
   lldb::user_id_t GetUID(const DWARFBaseDIE &die) {
-    return GetID() | die.GetOffset();
+    return GetUID(die.GetDIERef());
   }
 
-  lldb::user_id_t GetUID(const DIERef &ref) { return GetID() | ref.die_offset; }
+  lldb::user_id_t GetUID(const DIERef &ref) {
+    return GetID() | ref.die_offset |
+           (lldb::user_id_t(ref.section == DIERef::TypesSection) << 63);
+  }
 
   virtual std::unique_ptr<SymbolFileDWARFDwo>
   GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -349,10 +349,10 @@
 }
 
 SymbolFileDWARF::SymbolFileDWARF(ObjectFile *objfile)
-    : SymbolFile(objfile), UserID(uint64_t(DW_INVALID_OFFSET)
-                                  << 32), // Used by SymbolFileDWARFDebugMap to
-                                          // when this class parses .o files to
-                                          // contain the .o file index/ID
+    : SymbolFile(objfile),
+      UserID(0x7fffffff00000000), // Used by SymbolFileDWARFDebugMap to
+                                  // when this class parses .o files to
+                                  // contain the .o file index/ID
       m_debug_map_module_wp(), m_debug_map_symfile(NULL),
       m_context(*objfile->GetModule()), m_data_debug_abbrev(),
       m_data_debug_frame(), m_data_debug_info(), m_data_debug_line(),
@@ -1257,9 +1257,11 @@
   if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
     SymbolFileDWARF *dwarf = debug_map->GetSymbolFileByOSOIndex(
         debug_map->GetOSOIndexFromUserID(uid));
-    return {dwarf, {DW_INVALID_OFFSET, dw_offset_t(uid)}};
+    return {dwarf, {DIERef::InfoSection, DW_INVALID_OFFSET, dw_offset_t(uid)}};
   }
-  uint32_t dwarf_id = uid >> 32;
+  DIERef::DebugSection section =
+      uid >> 63 ? DIERef::TypesSection : DIERef::InfoSection;
+  uint32_t dwarf_id = uid >> 32 & 0x7fffffff;
   dw_offset_t die_offset = uid;
 
   if (die_offset == DW_INVALID_OFFSET)
@@ -1272,7 +1274,7 @@
         dwarf = unit->GetDwoSymbolFile();
     }
   }
-  return {dwarf, {DW_INVALID_OFFSET, die_offset}};
+  return {dwarf, {section, DW_INVALID_OFFSET, die_offset}};
 }
 
 DWARFDIE
@@ -1765,7 +1767,8 @@
         }
       } else {
         uint32_t cu_idx = DW_INVALID_INDEX;
-        DWARFUnit *dwarf_cu = debug_info->GetUnitAtOffset(cu_offset, &cu_idx);
+        DWARFUnit *dwarf_cu = debug_info->GetUnitAtOffset(DIERef::InfoSection,
+                                                          cu_offset, &cu_idx);
         if (dwarf_cu) {
           sc.comp_unit = GetCompUnitForDWARFCompUnit(dwarf_cu, cu_idx);
           if (sc.comp_unit) {
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -245,7 +245,7 @@
       }
     }
 
-    DIERef ref(cu_offset, die.GetOffset());
+    DIERef ref(unit.GetDebugSection(), cu_offset, die.GetOffset());
     switch (tag) {
     case DW_TAG_inlined_subroutine:
     case DW_TAG_subprogram:
Index: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
+++ source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
@@ -57,7 +57,9 @@
     DIEInfo();
     DIEInfo(dw_offset_t c, dw_offset_t o, dw_tag_t t, uint32_t f, uint32_t h);
 
-    explicit operator DIERef() const { return {cu_offset, offset}; }
+    explicit operator DIERef() const {
+      return {DIERef::InfoSection, cu_offset, offset};
+    }
   };
 
   struct Atom {
Index: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -78,8 +78,7 @@
           return;
         } else {
           // Put the one true definition as the first entry so it matches first
-          die_offsets.emplace(die_offsets.begin(), die_info_array[i].cu_offset,
-                              die_info_array[i].offset);
+          die_offsets.emplace(die_offsets.begin(), die_info_array[i]);
         }
       } else {
         die_offsets.emplace_back(die_info_array[i]);
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -55,7 +55,7 @@
   if (!cu_offset)
     return DIERef();
 
-  DWARFUnit *cu = m_debug_info.GetUnitAtOffset(*cu_offset);
+  DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::InfoSection, *cu_offset);
   if (!cu)
     return DIERef();
 
@@ -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(DIERef::InfoSection, *cu_offset, die_bias + *die_offset);
 
   return DIERef();
 }
@@ -164,7 +164,8 @@
     if (!ref)
       continue;
 
-    DWARFUnit *cu = m_debug_info.GetUnitAtOffset(ref.cu_offset);
+    DWARFUnit *cu =
+        m_debug_info.GetUnitAtOffset(DIERef::InfoSection, ref.cu_offset);
     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
@@ -167,6 +167,8 @@
     return die_iterator_range(m_die_array.begin(), m_die_array.end());
   }
 
+  virtual DIERef::DebugSection GetDebugSection() const = 0;
+
 protected:
   DWARFUnit(SymbolFileDWARF *dwarf, lldb::user_id_t uid);
 
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -545,7 +545,8 @@
       // Don't specify the compile unit offset as we don't know it because the
       // DIE belongs to
       // a different compile unit in the same symbol file.
-      return m_dwarf->DebugInfo()->GetDIEForDIEOffset(die_offset);
+      return m_dwarf->DebugInfo()->GetDIEForDIEOffset(GetDebugSection(),
+                                                      die_offset);
     }
   }
   return DWARFDIE(); // Not found
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -41,10 +41,13 @@
 
   size_t GetNumUnits();
   DWARFUnit *GetUnitAtIndex(lldb::user_id_t idx);
-  DWARFUnit *GetUnitAtOffset(dw_offset_t cu_offset, uint32_t *idx_ptr = NULL);
-  DWARFUnit *GetUnitContainingDIEOffset(dw_offset_t die_offset);
+  DWARFUnit *GetUnitAtOffset(DIERef::DebugSection section,
+                             dw_offset_t cu_offset, uint32_t *idx_ptr = NULL);
+  DWARFUnit *GetUnitContainingDIEOffset(DIERef::DebugSection section,
+                                        dw_offset_t die_offset);
   DWARFUnit *GetUnit(const DIERef &die_ref);
-  DWARFDIE GetDIEForDIEOffset(dw_offset_t die_offset);
+  DWARFDIE GetDIEForDIEOffset(DIERef::DebugSection section,
+                              dw_offset_t die_offset);
   DWARFDIE GetDIE(const DIERef &die_ref);
 
   enum {
@@ -57,9 +60,6 @@
   llvm::Expected<DWARFDebugAranges &> GetCompileUnitAranges();
 
 protected:
-  static bool OffsetLessThanUnitOffset(dw_offset_t offset,
-                                       const DWARFUnitSP &cu_sp);
-
   typedef std::vector<DWARFUnitSP> UnitColl;
 
   // Member variables
@@ -74,7 +74,7 @@
   // accessors are called.
   void ParseUnitHeadersIfNeeded();
 
-  uint32_t FindUnitIndex(dw_offset_t offset);
+  uint32_t FindUnitIndex(DIERef::DebugSection section, dw_offset_t offset);
 
   DISALLOW_COPY_AND_ASSIGN(DWARFDebugInfo);
 };
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -116,26 +116,28 @@
   return cu;
 }
 
-bool DWARFDebugInfo::OffsetLessThanUnitOffset(dw_offset_t offset,
-                                              const DWARFUnitSP &cu_sp) {
-  return offset < cu_sp->GetOffset();
-}
-
-uint32_t DWARFDebugInfo::FindUnitIndex(dw_offset_t offset) {
+uint32_t DWARFDebugInfo::FindUnitIndex(DIERef::DebugSection section,
+                                       dw_offset_t offset) {
   ParseUnitHeadersIfNeeded();
 
   // llvm::lower_bound is not used as for DIE offsets it would still return
   // index +1 and GetOffset() returning index itself would be a special case.
-  auto pos = llvm::upper_bound(m_units, offset, OffsetLessThanUnitOffset);
+  auto pos = llvm::upper_bound(
+      m_units, std::make_pair(section, offset),
+      [](const std::pair<DIERef::DebugSection, dw_offset_t> &lhs,
+         const DWARFUnitSP &rhs) {
+        return lhs < std::make_pair(rhs->GetDebugSection(), rhs->GetOffset());
+      });
   uint32_t idx = std::distance(m_units.begin(), pos);
   if (idx == 0)
     return DW_INVALID_OFFSET;
   return idx - 1;
 }
 
-DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(dw_offset_t cu_offset,
+DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::DebugSection section,
+                                           dw_offset_t cu_offset,
                                            uint32_t *idx_ptr) {
-  uint32_t idx = FindUnitIndex(cu_offset);
+  uint32_t idx = FindUnitIndex(section, cu_offset);
   DWARFUnit *result = GetUnitAtIndex(idx);
   if (result && result->GetOffset() != cu_offset) {
     result = nullptr;
@@ -148,13 +150,15 @@
 
 DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
   if (die_ref.cu_offset == DW_INVALID_OFFSET)
-    return GetUnitContainingDIEOffset(die_ref.die_offset);
+    return GetUnitContainingDIEOffset(die_ref.section, die_ref.die_offset);
   else
-    return GetUnitAtOffset(die_ref.cu_offset);
+    return GetUnitAtOffset(die_ref.section, die_ref.cu_offset);
 }
 
-DWARFUnit *DWARFDebugInfo::GetUnitContainingDIEOffset(dw_offset_t die_offset) {
-  uint32_t idx = FindUnitIndex(die_offset);
+DWARFUnit *
+DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::DebugSection section,
+                                           dw_offset_t die_offset) {
+  uint32_t idx = FindUnitIndex(section, die_offset);
   DWARFUnit *result = GetUnitAtIndex(idx);
   if (result && !result->ContainsDIEOffset(die_offset))
     return nullptr;
@@ -162,8 +166,9 @@
 }
 
 DWARFDIE
-DWARFDebugInfo::GetDIEForDIEOffset(dw_offset_t die_offset) {
-  DWARFUnit *cu = GetUnitContainingDIEOffset(die_offset);
+DWARFDebugInfo::GetDIEForDIEOffset(DIERef::DebugSection section,
+                                   dw_offset_t die_offset) {
+  DWARFUnit *cu = GetUnitContainingDIEOffset(section, die_offset);
   if (cu)
     return cu->GetDIE(die_offset);
   return DWARFDIE();
Index: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -155,7 +155,8 @@
           return DWARFDIE(cu, block_die);
         else
           return DWARFDIE(dwarf->DebugInfo()->GetUnit(
-                              DIERef(cu->GetOffset(), block_die->GetOffset())),
+                              DIERef(cu->GetDebugSection(), cu->GetOffset(),
+                                     block_die->GetOffset())),
                           block_die);
       }
     }
Index: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
@@ -34,6 +34,10 @@
   ///     Byte size of the compile unit header
   uint32_t GetHeaderByteSize() const override;
 
+  DIERef::DebugSection GetDebugSection() const override {
+    return DIERef::InfoSection;
+  }
+
 private:
   DWARFCompileUnit(SymbolFileDWARF *dwarf2Data, lldb::user_id_t uid);
   DISALLOW_COPY_AND_ASSIGN(DWARFCompileUnit);
Index: source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
@@ -24,7 +24,7 @@
   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->GetDebugSection(), cu_offset, 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
@@ -16,9 +16,12 @@
 class SymbolFileDWARF;
 
 struct DIERef {
+  enum DebugSection : uint8_t { InfoSection, TypesSection };
+
   DIERef() = default;
 
-  DIERef(dw_offset_t c, dw_offset_t d) : cu_offset(c), die_offset(d) {}
+  DIERef(DebugSection s, dw_offset_t c, dw_offset_t d)
+      : section(s), cu_offset(c), die_offset(d) {}
 
   explicit DIERef(const DWARFFormValue &form_value);
 
@@ -30,6 +33,7 @@
     return cu_offset != DW_INVALID_OFFSET || die_offset != DW_INVALID_OFFSET;
   }
 
+  DebugSection section = InfoSection;
   dw_offset_t cu_offset = 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
@@ -13,11 +13,11 @@
 #include "SymbolFileDWARF.h"
 #include "SymbolFileDWARFDebugMap.h"
 
-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) {
+      section = dwarf_cu->GetDebugSection();
       if (dwarf_cu->GetBaseObjOffset() != DW_INVALID_OFFSET)
         cu_offset = dwarf_cu->GetBaseObjOffset();
       else
Index: lit/SymbolFile/DWARF/dwarf5_locations.s
===================================================================
--- lit/SymbolFile/DWARF/dwarf5_locations.s
+++ lit/SymbolFile/DWARF/dwarf5_locations.s
@@ -4,7 +4,7 @@
 # RUN: ld.lld -m elf_x86_64 %t.o -o %t 
 # RUN: lldb-test symbols %t | FileCheck %s
 
-# CHECK: Variable{0xffffffff00000011}, name = "color"
+# CHECK: Variable{0x7fffffff00000011}, name = "color"
 # CHECK-SAME: location = DW_OP_addrx(0x0)
 
         .text
Index: lit/SymbolFile/DWARF/array-sizes.s
===================================================================
--- lit/SymbolFile/DWARF/array-sizes.s
+++ lit/SymbolFile/DWARF/array-sizes.s
@@ -9,8 +9,8 @@
 # RUN: ld.lld %t.o -o %t
 # RUN: lldb-test symbols %t | FileCheck %s
 
-# CHECK: Variable{0xffffffff0000001e}, name = "X"
-# CHECK-SAME: type = {ffffffff00000033} 0x{{[0-9a-f]*}} (char [56])
+# CHECK: Variable{0x7fffffff0000001e}, name = "X"
+# CHECK-SAME: type = {7fffffff00000033} 0x{{[0-9a-f]*}} (char [56])
 
 
 # Generated from "char X[47];"
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to