[DWARF] Reimplement/simplify DWARFUnit::GetID

[labath]

Summary

The implementation of GetID used a relatively complicated algorithm,
which returned some kind of an offset of the unit in some file
(depending on the debug info flavour). The only thing this ID was used
for was to enable subseqent retrieval of the unit from the SymbolFile.

This can be made simpler if we just make the "ID" of the unit an index
into the list of the units belonging to the symbol file. We already
support indexed access to the units, so each unit already has a well
"index" -- this just makes it accessible from within the unit.

To make the distincion between "id" and "offset" clearer (and help catch
any misuses), I also rename DWARFDebugInfo::GetCompileUnit (which
accesses by offset) into DWARFDebugInfo::GetCompileUnitAtOffset.

On its own, this only brings a minor simplification, but it enables
further simplifications in the DIERef class (coming in a follow-up
patch).

[JDevlieghere]

LGTM with one inline comment.

source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h

42–43

This should probably take a lldb::user_id_t then?

[labath]

source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h

42–43

Yep, I guess it should.

source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h

 #define SymbolFileDWARF_DWARFCompileUnit_h_
 
 #include "DWARFUnit.h"
 #include "llvm/Support/Error.h"
 
 class DWARFCompileUnit : public DWARFUnit {
 public:
   static llvm::Expected<DWARFUnitSP>
-  extract(SymbolFileDWARF *dwarf2Data,
+  extract(SymbolFileDWARF *dwarf2Data, lldb::user_id_t uid,
           const lldb_private::DWARFDataExtractor &debug_info,
           lldb::offset_t *offset_ptr);
   void Dump(lldb_private::Stream *s) const override;
 
   /// Get the data that contains the DIE information for this unit.
   ///
   /// \return
   ///   The correct data (.debug_types for DWARF 4 and earlier, and
   ///   .debug_info for DWARF 5 and later) for the DIE information in
   ///   this unit.
   const lldb_private::DWARFDataExtractor &GetData() const override;
 
   /// Get the size in bytes of the header.
   ///
   /// \return
   ///     Byte size of the compile unit header
   uint32_t GetHeaderByteSize() const override;
 
 private:
-  DWARFCompileUnit(SymbolFileDWARF *dwarf2Data);
+  DWARFCompileUnit(SymbolFileDWARF *dwarf2Data, lldb::user_id_t uid);
   DISALLOW_COPY_AND_ASSIGN(DWARFCompileUnit);
 };
 
 #endif // SymbolFileDWARF_DWARFCompileUnit_h_

source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp

 
 #include "SymbolFileDWARF.h"
 #include "lldb/Utility/Stream.h"
 #include "llvm/Object/Error.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
-DWARFCompileUnit::DWARFCompileUnit(SymbolFileDWARF *dwarf2Data)
-    : DWARFUnit(dwarf2Data) {}
+DWARFCompileUnit::DWARFCompileUnit(SymbolFileDWARF *dwarf2Data,
+                                   lldb::user_id_t uid)
+    : DWARFUnit(dwarf2Data, uid) {}
 
-llvm::Expected<DWARFUnitSP>
-DWARFCompileUnit::extract(SymbolFileDWARF *dwarf2Data,
-                          const DWARFDataExtractor &debug_info,
-                          lldb::offset_t *offset_ptr) {
+
+llvm::Expected<DWARFUnitSP> DWARFCompileUnit::extract(
+    SymbolFileDWARF *dwarf2Data, user_id_t uid,
+    const DWARFDataExtractor &debug_info, lldb::offset_t *offset_ptr) {
   assert(debug_info.ValidOffset(*offset_ptr));
 
   // std::make_shared would require the ctor to be public.
-  std::shared_ptr<DWARFCompileUnit> cu_sp(new DWARFCompileUnit(dwarf2Data));
+  std::shared_ptr<DWARFCompileUnit> cu_sp(
+      new DWARFCompileUnit(dwarf2Data, uid));
 
   cu_sp->m_offset = *offset_ptr;
 
   dw_offset_t abbr_offset;
   const DWARFDebugAbbrev *abbr = dwarf2Data->DebugAbbrev();
   if (!abbr)
     return llvm::make_error<llvm::object::GenericBinaryError>(
         "No debug_abbrev data");

source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h

typedef dw_offset_t (*Callback)(SymbolFileDWARF *dwarf2Data,
                                   DWARFUnit *cu,
                                   DWARFDebugInfoEntry *die,
                                   const dw_offset_t next_offset,
                                   const uint32_t depth, void *userData);
 
   explicit DWARFDebugInfo(lldb_private::DWARFContext &context);
   void SetDwarfData(SymbolFileDWARF *dwarf2Data);
 
   size_t GetNumCompileUnits();
-  DWARFUnit *GetCompileUnitAtIndex(uint32_t idx);
-  DWARFUnit *GetCompileUnit(dw_offset_t cu_offset, uint32_t *idx_ptr = NULL);
+  DWARFUnit *GetCompileUnitAtIndex(lldb::user_id_t idx);
+  DWARFUnit *GetCompileUnitAtOffset(dw_offset_t cu_offset,
+                                    uint32_t *idx_ptr = NULL);
   DWARFUnit *GetCompileUnitContainingDIEOffset(dw_offset_t die_offset);
   DWARFUnit *GetCompileUnit(const DIERef &die_ref);
   DWARFDIE GetDIEForDIEOffset(dw_offset_t die_offset);
   DWARFDIE GetDIE(const DIERef &die_ref);
 
   enum {
     eDumpFlag_Verbose = (1 << 0),  // Verbose dumping
     eDumpFlag_ShowForm = (1 << 1), // Show the DW_form type

source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp

if (!m_compile_units.empty())
     return;
   if (!m_dwarf2Data)
     return;
 
   lldb::offset_t offset = 0;
   const auto &debug_info_data = m_dwarf2Data->get_debug_info_data();
 
   while (debug_info_data.ValidOffset(offset)) {
-    llvm::Expected<DWARFUnitSP> cu_sp =
-        DWARFCompileUnit::extract(m_dwarf2Data, debug_info_data, &offset);
+    llvm::Expected<DWARFUnitSP> cu_sp = DWARFCompileUnit::extract(
+        m_dwarf2Data, m_compile_units.size(), debug_info_data, &offset);
 
     if (!cu_sp) {
       // FIXME: Propagate this error up.
       llvm::consumeError(cu_sp.takeError());
       return;
     }
 
     // If it didn't return an error, then it should be returning a valid
     // CompileUnit.
     assert(*cu_sp);
 
     m_compile_units.push_back(*cu_sp);
 
     offset = (*cu_sp)->GetNextCompileUnitOffset();
   }
 }
 
 size_t DWARFDebugInfo::GetNumCompileUnits() {
   ParseCompileUnitHeadersIfNeeded();
   return m_compile_units.size();
 }
 
-DWARFUnit *DWARFDebugInfo::GetCompileUnitAtIndex(uint32_t idx) {
+DWARFUnit *DWARFDebugInfo::GetCompileUnitAtIndex(user_id_t idx) {
   DWARFUnit *cu = NULL;
   if (idx < GetNumCompileUnits())
     cu = m_compile_units[idx].get();
   return cu;
 }
 
 bool DWARFDebugInfo::OffsetLessThanCompileUnitOffset(
     dw_offset_t offset, const DWARFUnitSP &cu_sp) {
   return offset < cu_sp->GetOffset();
 }
 
-DWARFUnit *DWARFDebugInfo::GetCompileUnit(dw_offset_t cu_offset,
+DWARFUnit *DWARFDebugInfo::GetCompileUnitAtOffset(dw_offset_t cu_offset,
                                                   uint32_t *idx_ptr) {
   DWARFUnitSP cu_sp;
   uint32_t cu_idx = DW_INVALID_INDEX;
   if (cu_offset != DW_INVALID_OFFSET) {
     ParseCompileUnitHeadersIfNeeded();
 
     // Watch out for single compile unit executable as they are pretty common
     const size_t num_cus = m_compile_units.size();
     if (num_cus == 1) {
if (idx_ptr)
     *idx_ptr = cu_idx;
   return cu_sp.get();
 }
 
 DWARFUnit *DWARFDebugInfo::GetCompileUnit(const DIERef &die_ref) {
   if (die_ref.cu_offset == DW_INVALID_OFFSET)
     return GetCompileUnitContainingDIEOffset(die_ref.die_offset);
   else
-    return GetCompileUnit(die_ref.cu_offset);
+    return GetCompileUnitAtOffset(die_ref.cu_offset);
 }
 
 DWARFUnit *
 DWARFDebugInfo::GetCompileUnitContainingDIEOffset(dw_offset_t die_offset) {
   ParseCompileUnitHeadersIfNeeded();
 
   DWARFUnitSP cu_sp;
 

source/Plugins/SymbolFile/DWARF/DWARFUnit.h

 enum DWARFProducer {
   eProducerInvalid = 0,
   eProducerClang,
   eProducerGCC,
   eProducerLLVMGCC,
   eProcucerOther
 };
 
-class DWARFUnit {
+class DWARFUnit : public lldb_private::UserID {
   using die_iterator_range =
       llvm::iterator_range<DWARFDebugInfoEntry::collection::iterator>;
 
 public:
   virtual ~DWARFUnit();
 
   void ExtractUnitDIEIfNeeded();
   void ExtractDIEsIfNeeded();
public:
   virtual const lldb_private::DWARFDataExtractor &GetData() const = 0;
   /// Get the size in bytes of the compile unit header.
   ///
   /// \return
   ///     Byte size of the compile unit header
   virtual uint32_t GetHeaderByteSize() const = 0;
   // Offset of the initial length field.
   dw_offset_t GetOffset() const { return m_offset; }
-  lldb::user_id_t GetID() const;
   /// Get the size in bytes of the length field in the header.
   ///
   /// In DWARF32 this is just 4 bytes
   ///
   /// \return
   ///     Byte size of the compile unit header length field
   size_t GetLengthByteSize() const { return 4; }
 
public:
   dw_offset_t GetBaseObjOffset() const;
 
   die_iterator_range dies() {
     ExtractDIEsIfNeeded();
     return die_iterator_range(m_die_array.begin(), m_die_array.end());
   }
 
 protected:
-  DWARFUnit(SymbolFileDWARF *dwarf);
+  DWARFUnit(SymbolFileDWARF *dwarf, lldb::user_id_t uid);
 
   SymbolFileDWARF *m_dwarf = nullptr;
   std::unique_ptr<SymbolFileDWARFDwo> m_dwo_symbol_file;
   const DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
   void *m_user_data = nullptr;
   // The compile unit debug information entry item
   DWARFDebugInfoEntry::collection m_die_array;
   mutable llvm::sys::RWMutex m_die_array_mutex;

source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

 #include "SymbolFileDWARFDwo.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace std;
 
 extern int g_verbose;
 
-DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf)
-    : m_dwarf(dwarf), m_cancel_scopes(false) {}
+DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf, user_id_t uid)
+    : UserID(uid), m_dwarf(dwarf), m_cancel_scopes(false) {}
 
 DWARFUnit::~DWARFUnit() {}
 
 // Parses first DIE of a compile unit.
 void DWARFUnit::ExtractUnitDIEIfNeeded() {
   {
     llvm::sys::ScopedReader lock(m_first_die_mutex);
     if (m_first_die)
for (pos = m_die_array.begin(); pos != end; ++pos) {
         dies.emplace_back(this, &(*pos));
     }
   }
 
   // Return the number of DIEs added to the collection
   return dies.size() - old_size;
 }
 
-lldb::user_id_t DWARFUnit::GetID() const {
-  dw_offset_t local_id =
-      m_base_obj_offset != DW_INVALID_OFFSET ? m_base_obj_offset : m_offset;
-  if (m_dwarf)
-    return DIERef(local_id, local_id).GetUID(m_dwarf);
-  else
-    return local_id;
-}
-
 dw_offset_t DWARFUnit::GetNextCompileUnitOffset() const {
   return m_offset + GetLengthByteSize() + GetLength();
 }
 
 size_t DWARFUnit::GetDebugInfoSize() const {
   return GetLengthByteSize() + GetLength() - GetHeaderByteSize();
 }
 

source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   return result;
 }
 
 DIERef DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) {
   llvm::Optional<uint64_t> cu_offset = entry.getCUOffset();
   if (!cu_offset)
     return DIERef();
 
-  DWARFUnit *cu = m_debug_info.GetCompileUnit(*cu_offset);
+  DWARFUnit *cu = m_debug_info.GetCompileUnitAtOffset(*cu_offset);
   if (!cu)
     return DIERef();
 
   // This initializes the DWO symbol file. It's not possible for
   // GetDwoSymbolFile to call this automatically because of mutual recursion
   // between this and DWARFDebugInfoEntry::GetAttributeValue.
   cu->ExtractUnitDIEIfNeeded();
   uint64_t die_bias = cu->GetDwoSymbolFile() ? 0 : *cu_offset;
for (const DebugNames::Entry &entry :
     if (entry.tag() != DW_TAG_structure_type &&
         entry.tag() != DW_TAG_class_type)
       continue;
 
     DIERef ref = ToDIERef(entry);
     if (!ref)
       continue;
 
-    DWARFUnit *cu = m_debug_info.GetCompileUnit(ref.cu_offset);
+    DWARFUnit *cu = m_debug_info.GetCompileUnitAtOffset(ref.cu_offset);
     if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) {
       incomplete_types.push_back(ref);
       continue;
     }
 
     // FIXME: We should return DWARFDIEs so we don't have to resolve it twice.
     DWARFDIE die = m_debug_info.GetDIE(ref);
     if (!die)

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

 
 DWARFUnit *
 SymbolFileDWARF::GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) {
   if (!comp_unit)
     return nullptr;
 
   DWARFDebugInfo *info = DebugInfo();
   if (info) {
-    // Just a normal DWARF file whose user ID for the compile unit is the DWARF
-    // offset itself
-
-    DWARFUnit *dwarf_cu =
-        info->GetCompileUnit((dw_offset_t)comp_unit->GetID());
+    // The compile unit ID is the index of the DWARF unit.
+    DWARFUnit *dwarf_cu = info->GetCompileUnitAtIndex(comp_unit->GetID());
     if (dwarf_cu && dwarf_cu->GetUserData() == NULL)
       dwarf_cu->SetUserData(comp_unit);
     return dwarf_cu;
   }
   return NULL;
 }
 
 DWARFDebugRangesBase *SymbolFileDWARF::DebugRanges() {
if (comp_unit) {
                   cu_sp->GetSupportFiles().Replace(0, cu_file_spec);
                 }
               }
 
               dwarf_cu->SetUserData(cu_sp.get());
 
               // Figure out the compile unit index if we weren't given one
               if (cu_idx == UINT32_MAX)
-                DebugInfo()->GetCompileUnit(dwarf_cu->GetOffset(), &cu_idx);
+                cu_idx = dwarf_cu->GetID();
 
               m_obj_file->GetModule()->GetSymbolVendor()->SetCompileUnitAtIndex(
                   cu_idx, cu_sp);
             }
           }
         }
       }
     }
if (debug_info) {
               sc.variable = variable;
             }
             return sc.GetResolvedMask();
           }
         }
       } else {
         uint32_t cu_idx = DW_INVALID_INDEX;
         DWARFUnit *dwarf_cu =
-            debug_info->GetCompileUnit(cu_offset, &cu_idx);
+            debug_info->GetCompileUnitAtOffset(cu_offset, &cu_idx);
         if (dwarf_cu) {
           sc.comp_unit = GetCompUnitForDWARFCompUnit(dwarf_cu, cu_idx);
           if (sc.comp_unit) {
             resolved |= eSymbolContextCompUnit;
 
             bool force_check_line_table = false;
             if (resolve_scope &
                 (eSymbolContextFunction | eSymbolContextBlock)) {
if (sc.function) {
         const size_t num_variables = ParseVariables(
             sc, function_die.GetFirstChild(), func_lo_pc, true, true);
 
         // Let all blocks know they have parse all their variables
         sc.function->GetBlock(false).SetDidParseVariables(true, true);
         return num_variables;
       }
     } else if (sc.comp_unit) {
-      DWARFUnit *dwarf_cu = info->GetCompileUnit(sc.comp_unit->GetID());
+      DWARFUnit *dwarf_cu = info->GetCompileUnitAtIndex(sc.comp_unit->GetID());
 
       if (dwarf_cu == NULL)
         return 0;
 
       uint32_t vars_added = 0;
       VariableListSP variables(sc.comp_unit->GetVariableList(false));
 
       if (variables.get() == NULL) {