Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split DWARFFormValue::getReference into four functions #98905

Merged
merged 2 commits into from
Jul 16, 2024

Conversation

labath
Copy link
Collaborator

@labath labath commented Jul 15, 2024

The result of the function cannot be correctly interpreted without knowing the precise form type (a type signature needs to be looked up very differently from a supplementary debug info reference). The function sort of worked because the two reference types (unit-relative and section-relative) that can be handled uniformly are also the most common types of references, but this setup made it easy to write code which does not support other kinds of reference (and if one tried to support them, the result didn't look pretty --
https://github.com/llvm/llvm-project/pull/97423/files#r1676217081).

The split is based on the reference type classification from DWARFv5 (Section 7.5.5 Classes and Forms), and it should enable uniform (if slightly more verbose) hadling. Note that this only affects users which want more control of how (or if) the references are resolved. Users which just want to access the referenced DIE can use the higher level API (DWARFDie::GetAttributeValueAsReferencedDie) which returns (or will return after #97423 is merged) the correct die for all reference types (except for supplementary references, which we don't support right now).

The result of the function cannot be correctly interpreted without
knowing the precise form type (a type signature needs to be looked up
very differently from a supplementary debug info reference). The
function sort of worked because the two reference types (unit-relative
and section-relative) that can be handled uniformly are also the most
common types of references, but this setup made it easy to write code
which does not support other kinds of reference (and if one tried to
support them, the result didn't look pretty --
https://github.com/llvm/llvm-project/pull/97423/files#r1676217081).

The split is based on the reference type classification from DWARFv5
(Section 7.5.5 Classes and Forms), and it should enable uniform (if
slightly more verbose) hadling. Note that this only affects users which
want more control of how (or if) the references are resolved. Users
which just want to access the referenced DIE can use the higher level
API (DWARFDie::GetAttributeValueAsReferencedDie) which returns (or will
return after llvm#97423 is merged) the correct die for all reference types
(except for supplementary references, which we don't support right now).
@llvmbot
Copy link

llvmbot commented Jul 15, 2024

@llvm/pr-subscribers-debuginfo

Author: Pavel Labath (labath)

Changes

The result of the function cannot be correctly interpreted without knowing the precise form type (a type signature needs to be looked up very differently from a supplementary debug info reference). The function sort of worked because the two reference types (unit-relative and section-relative) that can be handled uniformly are also the most common types of references, but this setup made it easy to write code which does not support other kinds of reference (and if one tried to support them, the result didn't look pretty --
https://github.com/llvm/llvm-project/pull/97423/files#r1676217081).

The split is based on the reference type classification from DWARFv5 (Section 7.5.5 Classes and Forms), and it should enable uniform (if slightly more verbose) hadling. Note that this only affects users which want more control of how (or if) the references are resolved. Users which just want to access the referenced DIE can use the higher level API (DWARFDie::GetAttributeValueAsReferencedDie) which returns (or will return after #97423 is merged) the correct die for all reference types (except for supplementary references, which we don't support right now).


Patch is 36.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/98905.diff

8 Files Affected:

  • (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h (+89-16)
  • (modified) llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp (+16-2)
  • (modified) llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp (+28-30)
  • (modified) llvm/lib/DebugInfo/DWARF/DWARFDie.cpp (+6-7)
  • (modified) llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp (+25-14)
  • (modified) llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp (+4-3)
  • (modified) llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp (+11-4)
  • (modified) llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp (+135-49)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
index dbb658940eef1..fb5712b855eed 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
@@ -107,12 +107,10 @@ class DWARFFormValue {
 
   /// getAsFoo functions below return the extracted value as Foo if only
   /// DWARFFormValue has form class is suitable for representing Foo.
-  std::optional<uint64_t> getAsReference() const;
-  struct UnitOffset {
-    DWARFUnit *Unit;
-    uint64_t Offset;
-  };
-  std::optional<UnitOffset> getAsRelativeReference() const;
+  std::optional<uint64_t> getAsRelativeReference() const;
+  std::optional<uint64_t> getAsDebugInfoReference() const;
+  std::optional<uint64_t> getAsSignatureReference() const;
+  std::optional<uint64_t> getAsSupplementaryReference() const;
   std::optional<uint64_t> getAsUnsignedConstant() const;
   std::optional<int64_t> getAsSignedConstant() const;
   Expected<const char *> getAsCString() const;
@@ -242,27 +240,102 @@ inline uint64_t toUnsigned(const std::optional<DWARFFormValue> &V,
   return toUnsigned(V).value_or(Default);
 }
 
-/// Take an optional DWARFFormValue and try to extract an reference.
+/// Take an optional DWARFFormValue and try to extract a relative offset
+/// reference.
 ///
-/// \param V and optional DWARFFormValue to attempt to extract the value from.
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
 /// \returns an optional value that contains a value if the form value
-/// was valid and has a reference form.
+/// was valid and has a relative reference form.
 inline std::optional<uint64_t>
-toReference(const std::optional<DWARFFormValue> &V) {
+toRelativeReference(const std::optional<DWARFFormValue> &V) {
   if (V)
-    return V->getAsReference();
+    return V->getAsRelativeReference();
   return std::nullopt;
 }
 
-/// Take an optional DWARFFormValue and extract a reference.
+/// Take an optional DWARFFormValue and extract a relative offset reference.
 ///
-/// \param V and optional DWARFFormValue to attempt to extract the value from.
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
+/// \param Default the default value to return in case of failure.
+/// \returns the extracted reference value or Default if the V doesn't have a
+/// value or the form value's encoding wasn't a relative offset reference form.
+inline uint64_t toRelativeReference(const std::optional<DWARFFormValue> &V,
+                            uint64_t Default) {
+  return toRelativeReference(V).value_or(Default);
+}
+
+/// Take an optional DWARFFormValue and try to extract an absolute debug info
+/// offset reference.
+///
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
+/// \returns an optional value that contains a value if the form value
+/// was valid and has an (absolute) debug info offset reference form.
+inline std::optional<uint64_t>
+toDebugInfoReference(const std::optional<DWARFFormValue> &V) {
+  if (V)
+    return V->getAsDebugInfoReference();
+  return std::nullopt;
+}
+
+/// Take an optional DWARFFormValue and extract an absolute debug info offset
+/// reference.
+///
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
+/// \param Default the default value to return in case of failure.
+/// \returns the extracted reference value or Default if the V doesn't have a
+/// value or the form value's encoding wasn't an absolute debug info offset
+/// reference form.
+inline uint64_t toDebugInfoReference(const std::optional<DWARFFormValue> &V,
+                            uint64_t Default) {
+  return toDebugInfoReference(V).value_or(Default);
+}
+
+/// Take an optional DWARFFormValue and try to extract a signature reference.
+///
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
+/// \returns an optional value that contains a value if the form value
+/// was valid and has a signature reference form.
+inline std::optional<uint64_t>
+toSignatureReference(const std::optional<DWARFFormValue> &V) {
+  if (V)
+    return V->getAsSignatureReference();
+  return std::nullopt;
+}
+
+/// Take an optional DWARFFormValue and extract a signature reference.
+///
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
+/// \param Default the default value to return in case of failure.
+/// \returns the extracted reference value or Default if the V doesn't have a
+/// value or the form value's encoding wasn't a signature reference form.
+inline uint64_t toSignatureReference(const std::optional<DWARFFormValue> &V,
+                            uint64_t Default) {
+  return toSignatureReference(V).value_or(Default);
+}
+
+/// Take an optional DWARFFormValue and try to extract a supplementary debug
+/// info reference.
+///
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
+/// \returns an optional value that contains a value if the form value
+/// was valid and has a supplementary reference form.
+inline std::optional<uint64_t>
+toSupplementaryReference(const std::optional<DWARFFormValue> &V) {
+  if (V)
+    return V->getAsSupplementaryReference();
+  return std::nullopt;
+}
+
+/// Take an optional DWARFFormValue and extract a supplementary debug info
+/// reference.
+///
+/// \param V an optional DWARFFormValue to attempt to extract the value from.
 /// \param Default the default value to return in case of failure.
 /// \returns the extracted reference value or Default if the V doesn't have a
-/// value or the form value's encoding wasn't a reference form.
-inline uint64_t toReference(const std::optional<DWARFFormValue> &V,
+/// value or the form value's encoding wasn't a supplementary reference form.
+inline uint64_t toSupplementaryReference(const std::optional<DWARFFormValue> &V,
                             uint64_t Default) {
-  return toReference(V).value_or(Default);
+  return toSupplementaryReference(V).value_or(Default);
 }
 
 /// Take an optional DWARFFormValue and try to extract an signed constant.
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
index c6312c387744a..26bce1fd7b162 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
@@ -77,7 +77,15 @@ DWARFDie DWARFLinker::resolveDIEReference(const DWARFFile &File,
                                           const DWARFDie &DIE,
                                           CompileUnit *&RefCU) {
   assert(RefValue.isFormClass(DWARFFormValue::FC_Reference));
-  uint64_t RefOffset = *RefValue.getAsReference();
+  uint64_t RefOffset;
+  if (std::optional<uint64_t> Off = RefValue.getAsRelativeReference()) {
+    RefOffset = RefValue.getUnit()->getOffset() + *Off;
+  } else if (Off = RefValue.getAsDebugInfoReference(); Off) {
+    RefOffset = *Off;
+  } else {
+    reportWarning("Unsupported reference type", File, &DIE);
+    return DWARFDie();
+  }
   if ((RefCU = getUnitForOffset(Units, RefOffset)))
     if (const auto RefDie = RefCU->getOrigUnit().getDIEForOffset(RefOffset)) {
       // In a file with broken references, an attribute might point to a NULL
@@ -1073,7 +1081,13 @@ unsigned DWARFLinker::DIECloner::cloneDieReferenceAttribute(
     unsigned AttrSize, const DWARFFormValue &Val, const DWARFFile &File,
     CompileUnit &Unit) {
   const DWARFUnit &U = Unit.getOrigUnit();
-  uint64_t Ref = *Val.getAsReference();
+  uint64_t Ref;
+  if (std::optional<uint64_t> Off = Val.getAsRelativeReference()) 
+    Ref = Val.getUnit()->getOffset() + *Off;
+  else if (Off = Val.getAsDebugInfoReference(); Off)
+    Ref = *Off;
+  else
+    return 0;
 
   DIE *NewRefDie = nullptr;
   CompileUnit *RefUnit = nullptr;
diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp
index 6f659eb8576b7..4daf781a2b53f 100644
--- a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp
+++ b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerCompileUnit.cpp
@@ -381,38 +381,36 @@ void CompileUnit::updateDieRefPatchesWithClonedOffsets() {
 std::optional<UnitEntryPairTy> CompileUnit::resolveDIEReference(
     const DWARFFormValue &RefValue,
     ResolveInterCUReferencesMode CanResolveInterCUReferences) {
-  if (std::optional<DWARFFormValue::UnitOffset> Ref =
-          *RefValue.getAsRelativeReference()) {
-    if (Ref->Unit == OrigUnit) {
-      // Referenced DIE is in current compile unit.
-      if (std::optional<uint32_t> RefDieIdx =
-              getDIEIndexForOffset(OrigUnit->getOffset() + Ref->Offset))
-        return UnitEntryPairTy{this, OrigUnit->getDebugInfoEntry(*RefDieIdx)};
-    }
-    uint64_t RefDIEOffset =
-        Ref->Unit ? Ref->Unit->getOffset() + Ref->Offset : Ref->Offset;
-    if (CompileUnit *RefCU = getUnitFromOffset(RefDIEOffset)) {
-      if (RefCU == this) {
-        // Referenced DIE is in current compile unit.
-        if (std::optional<uint32_t> RefDieIdx =
-                getDIEIndexForOffset(RefDIEOffset))
-          return UnitEntryPairTy{this, getDebugInfoEntry(*RefDieIdx)};
-      } else if (CanResolveInterCUReferences) {
-        // Referenced DIE is in other compile unit.
-
-        // Check whether DIEs are loaded for that compile unit.
-        enum Stage ReferredCUStage = RefCU->getStage();
-        if (ReferredCUStage < Stage::Loaded || ReferredCUStage > Stage::Cloned)
-          return UnitEntryPairTy{RefCU, nullptr};
-
-        if (std::optional<uint32_t> RefDieIdx =
-                RefCU->getDIEIndexForOffset(RefDIEOffset))
-          return UnitEntryPairTy{RefCU, RefCU->getDebugInfoEntry(*RefDieIdx)};
-      } else
-        return UnitEntryPairTy{RefCU, nullptr};
-    }
+  CompileUnit *RefCU;
+  uint64_t RefDIEOffset;
+  if (std::optional<uint64_t> Offset = RefValue.getAsRelativeReference()) {
+    RefCU = this;
+    RefDIEOffset = RefValue.getUnit()->getOffset() + *Offset;
+  } else if (Offset = RefValue.getAsDebugInfoReference(); Offset) {
+    RefCU = getUnitFromOffset(*Offset);
+    RefDIEOffset = *Offset;
+  } else {
+    return std::nullopt;
   }
 
+  if (RefCU == this) {
+    // Referenced DIE is in current compile unit.
+    if (std::optional<uint32_t> RefDieIdx = getDIEIndexForOffset(RefDIEOffset))
+      return UnitEntryPairTy{this, getDebugInfoEntry(*RefDieIdx)};
+  } else if (RefCU && CanResolveInterCUReferences) {
+    // Referenced DIE is in other compile unit.
+
+    // Check whether DIEs are loaded for that compile unit.
+    enum Stage ReferredCUStage = RefCU->getStage();
+    if (ReferredCUStage < Stage::Loaded || ReferredCUStage > Stage::Cloned)
+      return UnitEntryPairTy{RefCU, nullptr};
+
+    if (std::optional<uint32_t> RefDieIdx =
+            RefCU->getDIEIndexForOffset(RefDIEOffset))
+      return UnitEntryPairTy{RefCU, RefCU->getDebugInfoEntry(*RefDieIdx)};
+  } else {
+    return UnitEntryPairTy{RefCU, nullptr};
+  }
   return std::nullopt;
 }
 
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
index 410842a80b015..72e7464b68971 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
@@ -313,13 +313,12 @@ DWARFDie::getAttributeValueAsReferencedDie(dwarf::Attribute Attr) const {
 DWARFDie
 DWARFDie::getAttributeValueAsReferencedDie(const DWARFFormValue &V) const {
   DWARFDie Result;
-  if (auto SpecRef = V.getAsRelativeReference()) {
-    if (SpecRef->Unit)
-      Result = SpecRef->Unit->getDIEForOffset(SpecRef->Unit->getOffset() +
-                                              SpecRef->Offset);
-    else if (auto SpecUnit =
-                 U->getUnitVector().getUnitForOffset(SpecRef->Offset))
-      Result = SpecUnit->getDIEForOffset(SpecRef->Offset);
+  if (std::optional<uint64_t> Offset = V.getAsRelativeReference()) {
+    Result = const_cast<DWARFUnit *>(V.getUnit())
+                 ->getDIEForOffset(V.getUnit()->getOffset() + *Offset);
+  } else if (Offset = V.getAsDebugInfoReference(); Offset) {
+    if (DWARFUnit *SpecUnit = U->getUnitVector().getUnitForOffset(*Offset))
+      Result = SpecUnit->getDIEForOffset(*Offset);
   }
   return Result;
 }
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
index b9cf7d22c80d4..e4d2d0c7e0423 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
@@ -665,16 +665,7 @@ DWARFFormValue::getAsSectionedAddress() const {
   return getAsSectionedAddress(Value, Form, U);
 }
 
-std::optional<uint64_t> DWARFFormValue::getAsReference() const {
-  if (auto R = getAsRelativeReference())
-    return R->Unit ? R->Unit->getOffset() + R->Offset : R->Offset;
-  return std::nullopt;
-}
-
-std::optional<DWARFFormValue::UnitOffset>
-DWARFFormValue::getAsRelativeReference() const {
-  if (!isFormClass(FC_Reference))
-    return std::nullopt;
+std::optional<uint64_t> DWARFFormValue::getAsRelativeReference() const {
   switch (Form) {
   case DW_FORM_ref1:
   case DW_FORM_ref2:
@@ -683,11 +674,31 @@ DWARFFormValue::getAsRelativeReference() const {
   case DW_FORM_ref_udata:
     if (!U)
       return std::nullopt;
-    return UnitOffset{const_cast<DWARFUnit*>(U), Value.uval};
-  case DW_FORM_ref_addr:
-  case DW_FORM_ref_sig8:
+    return Value.uval;
+  default:
+    return std::nullopt;
+  }
+}
+
+std::optional<uint64_t> DWARFFormValue::getAsDebugInfoReference() const {
+  if (Form == DW_FORM_ref_addr)
+    return Value.uval;
+  return std::nullopt;
+}
+
+
+std::optional<uint64_t> DWARFFormValue::getAsSignatureReference() const {
+  if (Form == DW_FORM_ref_sig8)
+    return Value.uval;
+  return std::nullopt;
+}
+
+std::optional<uint64_t> DWARFFormValue::getAsSupplementaryReference() const {
+  switch (Form) {
   case DW_FORM_GNU_ref_alt:
-    return UnitOffset{nullptr, Value.uval};
+  case DW_FORM_ref_sup4:
+  case DW_FORM_ref_sup8:
+    return Value.uval;
   default:
     return std::nullopt;
   }
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 4ef6c80ed0289..a804deb446186 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -836,7 +836,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
   case DW_FORM_ref8:
   case DW_FORM_ref_udata: {
     // Verify all CU relative references are valid CU offsets.
-    std::optional<uint64_t> RefVal = AttrValue.Value.getAsReference();
+    std::optional<uint64_t> RefVal = AttrValue.Value.getAsRelativeReference();
     assert(RefVal);
     if (RefVal) {
       auto CUSize = DieCU->getNextUnitOffset() - DieCU->getOffset();
@@ -854,7 +854,8 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
       } else {
         // Valid reference, but we will verify it points to an actual
         // DIE later.
-        LocalReferences[*RefVal].insert(Die.getOffset());
+        LocalReferences[AttrValue.Value.getUnit()->getOffset() + *RefVal]
+            .insert(Die.getOffset());
       }
     }
     break;
@@ -862,7 +863,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
   case DW_FORM_ref_addr: {
     // Verify all absolute DIE references have valid offsets in the
     // .debug_info section.
-    std::optional<uint64_t> RefVal = AttrValue.Value.getAsReference();
+    std::optional<uint64_t> RefVal = AttrValue.Value.getAsDebugInfoReference();
     assert(RefVal);
     if (RefVal) {
       if (*RefVal >= DieCU->getInfoSection().Data.size()) {
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
index 6a97bed9e3a83..68a14b8b0ad33 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
@@ -1082,10 +1082,17 @@ void LVDWARFReader::updateReference(dwarf::Attribute Attr,
   // 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();
+  uint64_t Offset;
+  if (std::optional<uint64_t> Off = FormValue.getAsRelativeReference())
+    Offset = FormValue.getUnit()->getOffset() + *Off;
+  else if (Off = FormValue.getAsDebugInfoReference(); Off)
+    Offset = *Off;
+  else
+    llvm_unreachable("Unsupported reference type");
+
   // Get target for the given reference, if already created.
   LVElement *Target = getElementForOffset(
-      Reference, CurrentElement,
+      Offset, 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) {
@@ -1093,10 +1100,10 @@ void LVDWARFReader::updateReference(dwarf::Attribute Attr,
       // The global reference is ready. Mark it as global.
       Target->setIsGlobalReference();
       // Remove global reference from the unseen list.
-      removeGlobalOffset(Reference);
+      removeGlobalOffset(Offset);
     } else
       // Record the unseen cross CU reference.
-      addGlobalOffset(Reference);
+      addGlobalOffset(Offset);
   }
 
   // At this point, 'Target' can be null, in the case of the target element
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
index 5cb0310c0ad09..32ab022380f02 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -384,15 +384,15 @@ void TestAllForms() {
   //----------------------------------------------------------------------
   // Test reference forms
   //----------------------------------------------------------------------
-  EXPECT_EQ(RefAddr, toReference(DieDG.find(Attr_DW_FORM_ref_addr), 0));
-  EXPECT_EQ(Data1, toReference(DieDG.find(Attr_DW_FORM_ref1), 0));
-  EXPECT_EQ(Data2, toReference(DieDG.find(Attr_DW_FORM_ref2), 0));
-  EXPECT_EQ(Data4, toReference(DieDG.find(Attr_DW_FORM_ref4), 0));
-  EXPECT_EQ(Data8, toReference(DieDG.find(Attr_DW_FORM_ref8), 0));
+  EXPECT_EQ(RefAddr, toDebugInfoReference(DieDG.find(Attr_DW_FORM_ref_addr), 0));
+  EXPECT_EQ(Data1, toRelativeReference(DieDG.find(Attr_DW_FORM_ref1), 0));
+  EXPECT_EQ(Data2, toRelativeReference(DieDG.find(Attr_DW_FORM_ref2), 0));
+  EXPECT_EQ(Data4, toRelativeReference(DieDG.find(Attr_DW_FORM_ref4), 0));
+  EXPECT_EQ(Data8, toRelativeReference(DieDG.find(Attr_DW_FORM_ref8), 0));
   if (Version >= 4) {
-    EXPECT_EQ(Data8_2, toReference(DieDG.find(Attr_DW_FORM_ref_sig8), 0));
+    EXPECT_EQ(Data8_2, toSignatureReference(DieDG.find(Attr_DW_FORM_ref_sig8), 0));
   }
-  EXPECT_EQ(UData[0], toReference(DieDG.find(Attr_DW_FORM_ref_udata), 0));
+  EXPECT_EQ(UData[0], toRelativeReference(DieDG.find(Attr_DW_FORM_ref_udata), 0));
 
   //----------------------------------------------------------------------
   // Test flag forms
@@ -420,7 +420,7 @@ void TestAllForms() {
   // Test DWARF32/DWARF64 forms
   //----------------------------------------------------------------------
   EXPECT_EQ(Dwarf32Values[0],
-            toReference(DieDG.find(Attr_DW_FORM_GNU_ref_alt), 0));
+            toSupplementaryReference(DieDG.find(Attr_DW_FORM_GNU_ref_alt), 0));
   if (Version >= 4) {
     EXPECT_EQ(Dwarf32Values[1],
               toSectionOffset(DieDG.find(Attr_DW_FORM_sec_offset), 0));
@@ -761,14 +761,14 @@ template <uint16_t Version, class AddrType> void TestReferences() {
   EXPECT_TRUE(CU1Ref1DieDG.isValid());
   EXPECT_EQ(CU1Ref1DieDG.getTag(), DW_TAG_variable);
   EXPECT_EQ(CU1TypeDieDG.getOffset(),
-            toReference(CU1Ref1DieDG.find(DW_AT_type), -1ULL));
+            toRelativeReference(CU1Ref1DieDG.find(DW_AT_type), -1ULL));
   // Verify the sibling is our Ref2 DIE and that its DW_AT_type points to our
   // base type DIE in CU1.
   auto CU1Ref2DieDG = CU1Ref1DieDG.getSibling();
   EXPECT_TRUE(CU1Ref2DieDG.isValid());
   EXPECT_EQ(CU1Ref2DieDG.getTag(), DW_TAG_variable);
   EXPECT_EQ(CU1TypeDieDG.getOffset(),
-            toReference(CU1Ref2DieDG.find(DW_AT_type), -1ULL));
+            toRelativeReference(CU1Ref2DieDG.find(DW_AT_type), -1ULL));
 
   // Verify the sibling is our Ref4 DIE and that its DW_AT_type points to our
...
[truncated]

Copy link

github-actions bot commented Jul 15, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome - thanks!

Copy link
Contributor

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@labath labath merged commit d0d61a7 into llvm:main Jul 16, 2024
5 of 7 checks passed
@labath labath deleted the reference branch July 16, 2024 10:55
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 16, 2024

LLVM Buildbot has detected a new failure on builder bolt-x86_64-ubuntu-nfc running on bolt-worker while building llvm at step 7 "build-bolt".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/92/builds/1768

Here is the relevant piece of the build log for the reference:

Step 7 (build-bolt) failure: build (failure)
...
14.724 [75/18/50] Linking CXX static library lib/libLLVMProfileData.a
14.868 [74/18/51] Linking CXX static library lib/libLLVMDWARFLinker.a
14.996 [73/18/52] Linking CXX static library lib/libLLVMDWARFLinkerClassic.a
15.320 [72/18/53] Linking CXX static library lib/libLLVMVectorize.a
15.734 [71/18/54] Linking CXX static library lib/libLLVMAArch64CodeGen.a
16.658 [70/18/55] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinarySection.cpp.o
16.700 [69/18/56] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/AsmDump.cpp.o
17.299 [68/18/57] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryFunctionCallGraph.cpp.o
17.824 [67/18/58] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryFunctionProfile.cpp.o
18.021 [66/18/59] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o
FAILED: tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o 
ccache /usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/build/tools/bolt/lib/Core -I/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/bolt/lib/Core -I/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/build/include -I/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/llvm/include -I/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/bolt/include -I/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/build/tools/bolt/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o -MF tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o.d -o tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o -c /home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/bolt/lib/Core/DIEBuilder.cpp
/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/bolt/lib/Core/DIEBuilder.cpp: In member function ‘llvm::DWARFDie llvm::bolt::DIEBuilder::resolveDIEReference(const llvm::DWARFFormValue&, llvm::DWARFAbbreviationDeclaration::AttributeSpec, llvm::DWARFUnit*&, llvm::DWARFDebugInfoEntry&)’:
/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/bolt/lib/Core/DIEBuilder.cpp:559:34: error: ‘const class llvm::DWARFFormValue’ has no member named ‘getAsReference’; did you mean ‘getAsReferenceUVal’?
  559 |   uint64_t RefOffset = *RefValue.getAsReference();
      |                                  ^~~~~~~~~~~~~~
      |                                  getAsReferenceUVal
/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/bolt/lib/Core/DIEBuilder.cpp: In member function ‘void llvm::bolt::DIEBuilder::cloneDieReferenceAttribute(llvm::DIE&, const llvm::DWARFUnit&, const llvm::DWARFDie&, llvm::DWARFAbbreviationDeclaration::AttributeSpec, const llvm::DWARFFormValue&)’:
/home/worker/bolt-worker2/bolt-x86_64-ubuntu-nfc/llvm-project/bolt/lib/Core/DIEBuilder.cpp:610:29: error: ‘const class llvm::DWARFFormValue’ has no member named ‘getAsReference’; did you mean ‘getAsReferenceUVal’?
  610 |   const uint64_t Ref = *Val.getAsReference();
      |                             ^~~~~~~~~~~~~~
      |                             getAsReferenceUVal
18.506 [66/17/60] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DebugData.cpp.o
19.003 [66/16/61] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/GDBIndex.cpp.o
19.270 [66/15/62] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryEmitter.cpp.o
20.126 [66/14/63] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DebugNames.cpp.o
20.260 [66/13/64] Building CXX object tools/bolt/lib/Rewrite/CMakeFiles/LLVMBOLTRewrite.dir/DWARFRewriter.cpp.o
20.549 [66/12/65] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/JumpTable.cpp.o
20.735 [66/11/66] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DynoStats.cpp.o
21.149 [66/10/67] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryContext.cpp.o
21.491 [66/9/68] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/ParallelUtilities.cpp.o
21.583 [66/8/69] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/HashUtilities.cpp.o
22.016 [66/7/70] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/Exceptions.cpp.o
22.312 [66/6/71] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/Aligner.cpp.o
23.116 [66/5/72] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/CacheMetrics.cpp.o
23.432 [66/4/73] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/CMOVConversion.cpp.o
23.500 [66/3/74] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/DataflowAnalysis.cpp.o
25.243 [66/2/75] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryFunction.cpp.o
26.682 [66/1/76] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/BinaryPasses.cpp.o
ninja: build stopped: subcommand failed.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 16, 2024

LLVM Buildbot has detected a new failure on builder premerge-monolithic-linux running on premerge-linux-1 while building llvm at step 6 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/3089

Here is the relevant piece of the build log for the reference:

Step 6 (build-unified-tree) failure: build (failure)
...
9.097 [3238/58/57] Building CXX object lib/DWARFLinker/Parallel/CMakeFiles/LLVMDWARFLinkerParallel.dir/DWARFLinkerUnit.cpp.o
9.278 [3237/58/58] Building CXX object lib/DWARFLinker/Classic/CMakeFiles/LLVMDWARFLinkerClassic.dir/DWARFStreamer.cpp.o
9.438 [3236/58/59] Building CXX object lib/ExecutionEngine/Orc/Debugging/CMakeFiles/LLVMOrcDebugging.dir/DebuggerSupportPlugin.cpp.o
11.296 [3235/58/60] Building CXX object lib/DWARFLinker/Parallel/CMakeFiles/LLVMDWARFLinkerParallel.dir/DWARFLinkerCompileUnit.cpp.o
11.329 [3234/58/61] Building CXX object lib/DWARFLinker/Parallel/CMakeFiles/LLVMDWARFLinkerParallel.dir/DWARFLinkerTypeUnit.cpp.o
11.584 [3233/58/62] Building CXX object lib/DebugInfo/LogicalView/CMakeFiles/LLVMDebugInfoLogicalView.dir/Readers/LVDWARFReader.cpp.o
11.726 [3232/58/63] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinarySection.cpp.o
11.775 [3231/58/64] Building CXX object lib/DebugInfo/LogicalView/CMakeFiles/LLVMDebugInfoLogicalView.dir/LVReaderHandler.cpp.o
11.798 [3230/58/65] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/GDBIndex.cpp.o
11.801 [3229/58/66] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o
FAILED: tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o 
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/build/buildbot/premerge-monolithic-linux/build/tools/bolt/lib/Core -I/build/buildbot/premerge-monolithic-linux/llvm-project/bolt/lib/Core -I/build/buildbot/premerge-monolithic-linux/build/include -I/build/buildbot/premerge-monolithic-linux/llvm-project/llvm/include -I/build/buildbot/premerge-monolithic-linux/llvm-project/bolt/include -I/build/buildbot/premerge-monolithic-linux/build/tools/bolt/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o -MF tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o.d -o tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DIEBuilder.cpp.o -c /build/buildbot/premerge-monolithic-linux/llvm-project/bolt/lib/Core/DIEBuilder.cpp
/build/buildbot/premerge-monolithic-linux/llvm-project/bolt/lib/Core/DIEBuilder.cpp:559:34: error: no member named 'getAsReference' in 'llvm::DWARFFormValue'; did you mean 'getAsReferenceUVal'?
  uint64_t RefOffset = *RefValue.getAsReference();
                                 ^~~~~~~~~~~~~~
                                 getAsReferenceUVal
/build/buildbot/premerge-monolithic-linux/llvm-project/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h:122:27: note: 'getAsReferenceUVal' declared here
  std::optional<uint64_t> getAsReferenceUVal() const;
                          ^
/build/buildbot/premerge-monolithic-linux/llvm-project/bolt/lib/Core/DIEBuilder.cpp:610:29: error: no member named 'getAsReference' in 'llvm::DWARFFormValue'; did you mean 'getAsReferenceUVal'?
  const uint64_t Ref = *Val.getAsReference();
                            ^~~~~~~~~~~~~~
                            getAsReferenceUVal
/build/buildbot/premerge-monolithic-linux/llvm-project/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h:122:27: note: 'getAsReferenceUVal' declared here
  std::optional<uint64_t> getAsReferenceUVal() const;
                          ^
2 errors generated.
12.146 [3229/57/67] Building CXX object lib/ProfileData/CMakeFiles/LLVMProfileData.dir/MemProfReader.cpp.o
12.215 [3229/56/68] Building CXX object lib/DebugInfo/DWARF/CMakeFiles/LLVMDebugInfoDWARF.dir/DWARFContext.cpp.o
13.243 [3229/55/69] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/AddressMap.cpp.o
13.494 [3229/54/70] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryFunctionProfile.cpp.o
13.631 [3229/53/71] Building CXX object lib/DWARFLinker/Parallel/CMakeFiles/LLVMDWARFLinkerParallel.dir/DWARFLinkerImpl.cpp.o
13.734 [3229/52/72] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DebugNames.cpp.o
13.906 [3229/51/73] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DebugData.cpp.o
13.999 [3229/50/74] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/DynoStats.cpp.o
14.177 [3229/49/75] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/JumpTable.cpp.o
14.259 [3229/48/76] Building CXX object lib/DebugInfo/DWARF/CMakeFiles/LLVMDebugInfoDWARF.dir/DWARFVerifier.cpp.o
14.282 [3229/47/77] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryFunctionCallGraph.cpp.o
14.356 [3229/46/78] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/BinaryBasicBlock.cpp.o
14.490 [3229/45/79] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/HashUtilities.cpp.o
14.649 [3229/44/80] Building CXX object tools/bolt/tools/merge-fdata/CMakeFiles/merge-fdata.dir/merge-fdata.cpp.o
14.841 [3229/43/81] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/ADRRelaxationPass.cpp.o
14.849 [3229/42/82] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/Aligner.cpp.o
15.231 [3229/41/83] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/DataflowAnalysis.cpp.o
15.277 [3229/40/84] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/CMOVConversion.cpp.o
15.461 [3229/39/85] Building CXX object tools/bolt/lib/Core/CMakeFiles/LLVMBOLTCore.dir/ParallelUtilities.cpp.o
15.565 [3229/38/86] Building CXX object lib/DWARFLinker/Classic/CMakeFiles/LLVMDWARFLinkerClassic.dir/DWARFLinker.cpp.o
15.645 [3229/37/87] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/AllocCombiner.cpp.o
15.923 [3229/36/88] Building CXX object tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/CacheMetrics.cpp.o

@akuegel
Copy link
Member

akuegel commented Jul 16, 2024

@labath seems that bolt needs some adaptation

@labath
Copy link
Collaborator Author

labath commented Jul 16, 2024

I see the problem. I'm testing a fix now.

labath added a commit that referenced this pull request Jul 16, 2024
sayhaan pushed a commit to sayhaan/llvm-project that referenced this pull request Jul 16, 2024
Summary:
The result of the function cannot be correctly interpreted without
knowing the precise form type (a type signature needs to be looked up
very differently from a supplementary debug info reference). The
function sort of worked because the two reference types (unit-relative
and section-relative) that can be handled uniformly are also the most
common types of references, but this setup made it easy to write code
which does not support other kinds of reference (and if one tried to
support them, the result didn't look pretty --
https://github.com/llvm/llvm-project/pull/97423/files#r1676217081).

The split is based on the reference type classification from DWARFv5
(Section 7.5.5 Classes and Forms), and it should enable uniform (if
slightly more verbose) hadling. Note that this only affects users which
want more control of how (or if) the references are resolved. Users
which just want to access the referenced DIE can use the higher level
API (DWARFDie::GetAttributeValueAsReferencedDie) which returns (or will
return after llvm#97423 is merged) the correct die for all reference types
(except for supplementary references, which we don't support right now).

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D59822392
sayhaan pushed a commit to sayhaan/llvm-project that referenced this pull request Jul 16, 2024
Summary: 

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D59822372
labath added a commit to labath/llvm-project that referenced this pull request Jul 17, 2024
The caller (cloneAttribute) already switches on the reference type. By
aligning the cases with the retrieval functions, we can avoid branching
twice.
labath added a commit that referenced this pull request Jul 18, 2024
 (#99324)

The caller (cloneAttribute) already switches on the reference type. By
aligning the cases with the retrieval functions, we can avoid branching
twice.
Harini0924 pushed a commit to Harini0924/llvm-project that referenced this pull request Jul 22, 2024
…m#98905 (llvm#99324)

The caller (cloneAttribute) already switches on the reference type. By
aligning the cases with the retrieval functions, we can avoid branching
twice.
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
…m#98905 (llvm#99324)

The caller (cloneAttribute) already switches on the reference type. By
aligning the cases with the retrieval functions, we can avoid branching
twice.
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
The result of the function cannot be correctly interpreted without
knowing the precise form type (a type signature needs to be looked up
very differently from a supplementary debug info reference). The
function sort of worked because the two reference types (unit-relative
and section-relative) that can be handled uniformly are also the most
common types of references, but this setup made it easy to write code
which does not support other kinds of reference (and if one tried to
support them, the result didn't look pretty --
https://github.com/llvm/llvm-project/pull/97423/files#r1676217081).

The split is based on the reference type classification from DWARFv5
(Section 7.5.5 Classes and Forms), and it should enable uniform (if
slightly more verbose) hadling. Note that this only affects users which
want more control of how (or if) the references are resolved. Users
which just want to access the referenced DIE can use the higher level
API (DWARFDie::GetAttributeValueAsReferencedDie) which returns (or will
return after #97423 is merged) the correct die for all reference types
(except for supplementary references, which we don't support right now).

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251742
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: 

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251577
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
 (#99324)

Summary:
The caller (cloneAttribute) already switches on the reference type. By
aligning the cases with the retrieval functions, we can avoid branching
twice.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250831
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants