[llvm-debuginfo-analyzer] Support both Reference and Type attrs in si…

…ngle DIE

Relax the assumption that at most one Reference-or-Type-like attribute is
present on a DWARF DIE.

Add support for at most one Type attribute (i.e. DW_AT_import xor
DW_AT_type) and separately at most one Reference attribute (i.e.
DW_AT_specification xor DW_AT_abstract_origin xor ...).

Update comment describing old assumption and tag it as a "FIXME" to
reflect the fact that this is perhaps still not general enough.

Add a test based on the case which led me to encounter the bug in the
wild.

Reviewed By: CarlosAlbertoEnciso

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

llvm/include/llvm/DebugInfo/LogicalView/Readers/LVELFReader.h

@@ -69,7 +69,12 @@ class LVELFReader final : public LVBinaryReader {
 
   // Cross references (Elements).
   using LVElementSet = std::unordered_set<LVElement *>;
-  using LVElementEntry = std::pair<LVElement *, LVElementSet>;
+  struct LVElementEntry {
+    LVElement *Element;
+    LVElementSet References;
+    LVElementSet Types;
+    LVElementEntry(LVElement *Element = nullptr) : Element(Element) {}
+  };
   using LVElementReference = std::unordered_map<LVOffset, LVElementEntry>;
   LVElementReference ElementTable;
 
@@ -114,7 +119,8 @@ class LVELFReader final : public LVBinaryReader {
   void updateReference(dwarf::Attribute Attr, const DWARFFormValue &FormValue);
 
   // Get an element given the DIE offset.
-  LVElement *getElementForOffset(LVOffset offset, LVElement *Element);
+  LVElement *getElementForOffset(LVOffset offset, LVElement *Element,
+                                 bool IsType);
 
 protected:
   Error createScopes() override;

llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp

@@ -548,22 +548,22 @@ LVScope *LVELFReader::processOneDie(const DWARFDie &InputDIE, LVScope *Parent,
     // referencing this element.
     if (ElementTable.find(Offset) == ElementTable.end()) {
       // No previous references to this offset.
-      ElementTable.emplace(
-          std::piecewise_construct, std::forward_as_tuple(Offset),
-          std::forward_as_tuple(CurrentElement, LVElementSet()));
+      ElementTable.emplace(std::piecewise_construct,
+                           std::forward_as_tuple(Offset),
+                           std::forward_as_tuple(CurrentElement));
     } else {
       // There are previous references to this element. We need to update the
       // element and all the references pointing to this element.
       LVElementEntry &Reference = ElementTable[Offset];
-      Reference.first = CurrentElement;
+      Reference.Element = CurrentElement;
       // Traverse the element set and update the elements (backtracking).
-      // Using the bit associated with 'type' or 'reference' allows us to set
-      // the correct target.
-      for (LVElement *Target : Reference.second)
-        Target->getHasReference() ? Target->setReference(CurrentElement)
-                                  : Target->setType(CurrentElement);
+      for (LVElement *Target : Reference.References)
+        Target->setReference(CurrentElement);
+      for (LVElement *Target : Reference.Types)
+        Target->setType(CurrentElement);
       // Clear the pending elements.
-      Reference.second.clear();
+      Reference.References.clear();
+      Reference.Types.clear();
     }
 
     // Add the current element to its parent as there are attributes
@@ -1075,12 +1075,14 @@ void LVELFReader::processLocationMember(dwarf::Attribute Attr,
 // Update the current element with the reference.
 void LVELFReader::updateReference(dwarf::Attribute Attr,
                                   const DWARFFormValue &FormValue) {
-  // We are assuming that DW_AT_specification, DW_AT_abstract_origin,
-  // DW_AT_type and DW_AT_extension do not appear at the same time
-  // in the same DIE.
+  // FIXME: We are assuming that at most one Reference (DW_AT_specification,
+  // DW_AT_abstract_origin, ...) and at most one Type (DW_AT_import, DW_AT_type)
+  // appear in any single DIE, but this may not be true.
   uint64_t Reference = *FormValue.getAsReference();
   // Get target for the given reference, if already created.
-  LVElement *Target = getElementForOffset(Reference, CurrentElement);
+  LVElement *Target = getElementForOffset(
+      Reference, CurrentElement,
+      /*IsType=*/Attr == dwarf::DW_AT_import || Attr == dwarf::DW_AT_type);
   // Check if we are dealing with cross CU references.
   if (FormValue.getForm() == dwarf::DW_FORM_ref_addr) {
     if (Target) {
@@ -1124,26 +1126,18 @@ void LVELFReader::updateReference(dwarf::Attribute Attr,
 }
 
 // Get an element given the DIE offset.
-LVElement *LVELFReader::getElementForOffset(LVOffset Offset,
-                                            LVElement *Element) {
-  LVElement *Target = nullptr;
-  // Search offset in the cross references.
-  LVElementReference::iterator Iter = ElementTable.find(Offset);
-  if (Iter == ElementTable.end())
-    // Reference to an unseen element.
-    ElementTable.emplace(std::piecewise_construct,
-                         std::forward_as_tuple(Offset),
-                         std::forward_as_tuple(nullptr, LVElementSet{Element}));
-  else {
-    // There are previous references to this element. We need to update the
-    // element and all the references pointing to this element.
-    LVElementEntry &Reference = Iter->second;
-    Target = Reference.first;
-    if (!Target)
-      // Add the element to the set.
-      Reference.second.insert(Element);
+LVElement *LVELFReader::getElementForOffset(LVOffset Offset, LVElement *Element,
+                                            bool IsType) {
+  auto Iter = ElementTable.try_emplace(Offset).first;
+  // Update the element and all the references pointing to this element.
+  LVElementEntry &Entry = Iter->second;
+  if (!Entry.Element) {
+    if (IsType)
+      Entry.Types.insert(Element);
+    else
+      Entry.References.insert(Element);
   }
-  return Target;
+  return Entry.Element;
 }
 
 Error LVELFReader::loadTargetInfo(const ObjectFile &Obj) {

llvm/test/tools/llvm-debuginfo-analyzer/DWARF/dw-at-specification.test

@@ -0,0 +1,29 @@
+; REQUIRES: x86-registered-target
+
+; dw-at-specification.cpp
+; 1  struct S {
+; 2      static const int Arr[];
+; 3  };
+; 4  const int S::Arr[] = {
+; 5      0, 1, 2
+; 6  };
+
+; The above test compiled with clang++ produces both a DW_AT_type and
+; DW_AT_specification on the definition die for S::Arr, which previously caused
+; an assert in the LVELFReader:
+; $ clang++ -g -c dw-at-specification.cpp -o dw-at-specification.o
+
+; RUN: llvm-debuginfo-analyzer --attribute=level,format,producer \
+; RUN:                         --output-sort=name \
+; RUN:                         --print=symbols \
+; RUN:                         %p/Inputs/dw-at-specification.o 2>&1 | \
+; RUN: FileCheck --strict-whitespace -check-prefix=CHECK %s
+
+; CHECK: Logical View:
+; CHECK: [000]           {File} 'dw-at-specification.o' -> elf64-x86-64
+; CHECK-EMPTY:
+; CHECK: [001]             {CompileUnit} 'a.cpp'
+; CHECK: [002]               {Producer} 'clang version 11.0.0 ({{.*}})'
+; CHECK: [002]               {Variable} 'Arr' -> 'const int [3]'
+; CHECK: [002]     1         {Struct} 'S'
+; CHECK: [003]     2           {Member} extern public 'Arr' -> 'const int [1]'