clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere.
Herald added a subscriber: arphaman.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When fission is enabled, we were indexing the skeleton CU _and_ the .dwo CU. 
Issues arise when users enable compiler options that add extra data to the 
skeleton CU (like -fsplit-dwarf-inlining) and there can end up being types in 
the skeleton CU due to template parameters. We never want to index this 
information since the .dwo file has the real definition, and we really don't 
want function prototypes from this info since all parameters are removed. The 
index doesn't work correctly if it does index the skeleton CU as the DIE offset 
will assume it is from the .dwo file, so even if we do index the skeleton CU, 
the index entries will try and grab information from the .dwo file using the 
wrong DIE offset which can cause errors to be displayed or even worse, if the 
DIE offsets is valid in the .dwo CU, the wrong DIE will be used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131437

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -148,19 +148,36 @@
 
   const LanguageType cu_language = SymbolFileDWARF::GetLanguage(unit);
 
-  IndexUnitImpl(unit, cu_language, set);
-
-  if (SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile()) {
-    // Type units in a dwp file are indexed separately, so we just need to
-    // process the split unit here. However, if the split unit is in a dwo file,
-    // then we need to process type units here.
-    if (dwo_symbol_file == dwp) {
-      IndexUnitImpl(unit.GetNonSkeletonUnit(), cu_language, set);
-    } else {
-      DWARFDebugInfo &dwo_info = dwo_symbol_file->DebugInfo();
-      for (size_t i = 0; i < dwo_info.GetNumUnits(); ++i)
-        IndexUnitImpl(*dwo_info.GetUnitAtIndex(i), cu_language, set);
+  // First check if the unit has a DWO ID. If it does then we only want to index
+  // the .dwo file or nothing at all. If we have a compile unit where we can't
+  // locate the .dwo/.dwp file we don't want to index anything from the skeleton
+  // compile unit because it is usally has no children unless
+  // -fsplit-dwarf-inlining was used at compile time. This option will add a
+  // copy of all DW_TAG_subprogram and any contained DW_TAG_inline_subroutine
+  // DIEs so that symbolication will still work in the absence of the .dwo/.dwp
+  // file, but the functions have no return types and all arguments and locals
+  // have been removed. So we don't want to index any of these hacked up
+  // function types. Types can still exist in the skeleton compile unit DWARF
+  // though as some functions have template parameter types and other things
+  // that cause extra copies of types to be included, but we should find these
+  // types in the .dwo file only as methods could have return types removed and
+  // we don't have to index incomplete types from the skeletone compile unit.
+  if (unit.GetDWOId()) {
+    if (SymbolFileDWARFDwo *dwo_symbol_file = unit.GetDwoSymbolFile()) {
+      // Type units in a dwp file are indexed separately, so we just need to
+      // process the split unit here. However, if the split unit is in a dwo file,
+      // then we need to process type units here.
+      if (dwo_symbol_file == dwp) {
+        IndexUnitImpl(unit.GetNonSkeletonUnit(), cu_language, set);
+      } else {
+        DWARFDebugInfo &dwo_info = dwo_symbol_file->DebugInfo();
+        for (size_t i = 0; i < dwo_info.GetNumUnits(); ++i)
+          IndexUnitImpl(*dwo_info.GetUnitAtIndex(i), cu_language, set);
+      }
     }
+  } else {
+    // We either have a normal compile unit which we want to index.
+    IndexUnitImpl(unit, cu_language, set);
   }
 }
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -51,7 +51,7 @@
   uint64_t m_type_hash = 0;
   uint32_t m_type_offset = 0;
 
-  uint64_t m_dwo_id = 0;
+  llvm::Optional<uint64_t> m_dwo_id;
 
   DWARFUnitHeader() = default;
 
@@ -67,7 +67,7 @@
   }
   uint64_t GetTypeHash() const { return m_type_hash; }
   dw_offset_t GetTypeOffset() const { return m_type_offset; }
-  uint64_t GetDWOId() const { return m_dwo_id; }
+  llvm::Optional<uint64_t> GetDWOId() const { return m_dwo_id; }
   bool IsTypeUnit() const {
     return m_unit_type == llvm::dwarf::DW_UT_type ||
            m_unit_type == llvm::dwarf::DW_UT_split_type;
@@ -92,7 +92,7 @@
   virtual ~DWARFUnit();
 
   bool IsDWOUnit() { return m_is_dwo; }
-  uint64_t GetDWOId();
+  llvm::Optional<uint64_t> GetDWOId();
 
   void ExtractUnitDIEIfNeeded();
   void ExtractUnitDIENoDwoIfNeeded();
@@ -336,7 +336,7 @@
   bool m_is_dwo;
   bool m_has_parsed_non_skeleton_unit;
   /// Value of DW_AT_GNU_dwo_id (v4) or dwo_id from CU header (v5).
-  uint64_t m_dwo_id;
+  llvm::Optional<uint64_t> m_dwo_id;
 
 private:
   void ParseProducerInfo();
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -77,12 +77,15 @@
 
   m_has_parsed_non_skeleton_unit = true;
 
+  if (!m_dwo_id)
+    return; // No DWO file.
+
   std::shared_ptr<SymbolFileDWARFDwo> dwo_symbol_file =
       m_dwarf.GetDwoSymbolFileForCompileUnit(*this, m_first_die);
   if (!dwo_symbol_file)
     return;
 
-  DWARFUnit *dwo_cu = dwo_symbol_file->GetDWOCompileUnitForHash(m_dwo_id);
+  DWARFUnit *dwo_cu = dwo_symbol_file->GetDWOCompileUnitForHash(*m_dwo_id);
 
   if (!dwo_cu)
     return; // Can't fetch the compile unit from the dwo file.
@@ -345,7 +348,7 @@
   SetStrOffsetsBase(baseOffset);
 }
 
-uint64_t DWARFUnit::GetDWOId() {
+llvm::Optional<uint64_t> DWARFUnit::GetDWOId() {
   ExtractUnitDIENoDwoIfNeeded();
   return m_dwo_id;
 }
@@ -467,7 +470,7 @@
       GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
           "Failed to find location list contribution for CU with DWO Id "
           "0x%" PRIx64,
-          this->GetDWOId());
+          *GetDWOId());
       return;
     }
     offset += contribution->Offset;
@@ -889,8 +892,8 @@
         header.m_index_entry = Index->getFromHash(header.m_type_hash);
     } else {
       Index = &context.GetAsLLVM().getCUIndex();
-      if (*Index && header.m_version >= 5)
-        header.m_index_entry = Index->getFromHash(header.m_dwo_id);
+      if (*Index && header.m_version >= 5 && header.m_dwo_id)
+        header.m_index_entry = Index->getFromHash(*header.m_dwo_id);
     }
     if (!header.m_index_entry)
       header.m_index_entry = Index->getFromOffset(header.m_offset);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to