[DebugInfo][DWARF] Add doesFormBelongToClass function.

[avl]

Summary

The result of DWARFFormValue::isFormClass depends on DWARF version in some cases.
The current implementation takes DWARF version from the stored DWARFUnit.
If there is no stored DWARFUnit then the current behavior is to assume
DwarfVersion <= 3. This patch adds new function which has a DWARF version as a
parameter so it is possible to check form class for various DWARF versions.

[dblaikie]

Sounds OK, though might be nice to avoid having these different ways of using (with/without DWARFUnit) and find some way to unify them (have some more general abstraction over DWARFUnit that can be provided by all users of this API or something)

[avl]

In D145499#4175487, @dblaikie wrote:

Sounds OK, though might be nice to avoid having these different ways of using (with/without DWARFUnit) and find some way to unify them (have some more general abstraction over DWARFUnit that can be provided by all users of this API or something)

something like this?

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
index fa47621265f7..b13f5ffdf7f4 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
@@ -77,10 +77,7 @@ public:
   dwarf::Form getForm() const { return Form; }
   uint64_t getRawUValue() const { return Value.uval; }
 
-  /// Returns true if current value is of \p FC form class.
-  /// If current value does not have reference to original DWARFUnit
-  /// then \p DwarfVersion might be used to check form class of value.
-  bool isFormClass(FormClass FC, uint16_t DwarfVersion = 3) const;
+  bool isFormClass(FormClass FC) const;
   const DWARFUnit *getUnit() const { return U; }
   void dump(raw_ostream &OS, DIDumpOptions DumpOpts = DIDumpOptions()) const;
   void dumpSectionedAddress(raw_ostream &OS, DIDumpOptions DumpOpts,
@@ -352,6 +349,9 @@ toBlock(const std::optional<DWARFFormValue> &V) {
   return std::nullopt;
 }
 
+bool doesFormBelongToClass(dwarf::Form Form, DWARFFormValue::FormClass FC,
+                                 uint16_t DwarfVersion);
+
 } // end namespace dwarf
 
 } // end namespace llvm
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
index fd94c8a7ffbb..5ea0aa63c218 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
@@ -213,41 +213,8 @@ bool DWARFFormValue::skipValue(dwarf::Form Form, DataExtractor DebugInfoData,
   return true;
 }
 
-bool DWARFFormValue::isFormClass(DWARFFormValue::FormClass FC,
-                                 uint16_t DwarfVersion) const {
-  // First, check DWARF5 form classes.
-  if (Form < ArrayRef(DWARF5FormClasses).size() &&
-      DWARF5FormClasses[Form] == FC)
-    return true;
-  // Check more forms from extensions and proposals.
-  switch (Form) {
-  case DW_FORM_GNU_ref_alt:
-    return (FC == FC_Reference);
-  case DW_FORM_GNU_addr_index:
-    return (FC == FC_Address);
-  case DW_FORM_GNU_str_index:
-  case DW_FORM_GNU_strp_alt:
-    return (FC == FC_String);
-  case DW_FORM_LLVM_addrx_offset:
-    return (FC == FC_Address);
-  default:
-    break;
-  }
-
-  if (FC == FC_SectionOffset) {
-    if (Form == DW_FORM_strp || Form == DW_FORM_line_strp)
-      return true;
-    // In DWARF3 DW_FORM_data4 and DW_FORM_data8 served also as a section
-    // offset. If we don't have a DWARFUnit, default to the old behavior.
-    if (Form == DW_FORM_data4 || Form == DW_FORM_data8) {
-      if (U)
-        return U->getVersion() <= 3;
-
-      return DwarfVersion <= 3;
-    }
-  }
-
-  return false;
+bool DWARFFormValue::isFormClass(DWARFFormValue::FormClass FC) const {
+  return doesFormBelongToClass(Form, FC, U ? U->getVersion() : 3);
 }
 
 bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
@@ -783,3 +750,36 @@ DWARFFormValue::getAsFile(DILineInfoSpecifier::FileLineInfoKind Kind) const {
   }
   return std::nullopt;
 }
+
+bool doesFormBelongToClass(dwarf::Form Form, DWARFFormValue::FormClass FC,
+                                 uint16_t DwarfVersion) {
+    // First, check DWARF5 form classes.
+    if (Form < ArrayRef(DWARF5FormClasses).size() &&
+        DWARF5FormClasses[Form] == FC)
+      return true;
+    // Check more forms from extensions and proposals.
+    switch (Form) {
+    case DW_FORM_GNU_ref_alt:
+      return (FC == DWARFFormValue::FC_Reference);
+    case DW_FORM_GNU_addr_index:
+      return (FC == DWARFFormValue::FC_Address);
+    case DW_FORM_GNU_str_index:
+    case DW_FORM_GNU_strp_alt:
+      return (FC == DWARFFormValue::FC_String);
+    case DW_FORM_LLVM_addrx_offset:
+      return (FC == DWARFFormValue::FC_Address);
+    default:
+      break;
+    }
+
+    if (FC == DWARFFormValue::FC_SectionOffset) {
+      if (Form == DW_FORM_strp || Form == DW_FORM_line_strp)
+        return true;
+      // In DWARF3 DW_FORM_data4 and DW_FORM_data8 served also as a section
+      // offset.
+      if (Form == DW_FORM_data4 || Form == DW_FORM_data8)
+        return DwarfVersion <= 3;
+    }
+
+    return false;
+}

[dblaikie]

In D145499#4175662, @avl wrote:

In D145499#4175487, @dblaikie wrote:

Sounds OK, though might be nice to avoid having these different ways of using (with/without DWARFUnit) and find some way to unify them (have some more general abstraction over DWARFUnit that can be provided by all users of this API or something)

something like this?

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
index fa47621265f7..b13f5ffdf7f4 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
@@ -77,10 +77,7 @@ public:
   dwarf::Form getForm() const { return Form; }
   uint64_t getRawUValue() const { return Value.uval; }
 
-  /// Returns true if current value is of \p FC form class.
-  /// If current value does not have reference to original DWARFUnit
-  /// then \p DwarfVersion might be used to check form class of value.
-  bool isFormClass(FormClass FC, uint16_t DwarfVersion = 3) const;
+  bool isFormClass(FormClass FC) const;
   const DWARFUnit *getUnit() const { return U; }
   void dump(raw_ostream &OS, DIDumpOptions DumpOpts = DIDumpOptions()) const;
   void dumpSectionedAddress(raw_ostream &OS, DIDumpOptions DumpOpts,
@@ -352,6 +349,9 @@ toBlock(const std::optional<DWARFFormValue> &V) {
   return std::nullopt;
 }
 
+bool doesFormBelongToClass(dwarf::Form Form, DWARFFormValue::FormClass FC,
+                                 uint16_t DwarfVersion);
+
 } // end namespace dwarf
 
 } // end namespace llvm
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
index fd94c8a7ffbb..5ea0aa63c218 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
@@ -213,41 +213,8 @@ bool DWARFFormValue::skipValue(dwarf::Form Form, DataExtractor DebugInfoData,
   return true;
 }
 
-bool DWARFFormValue::isFormClass(DWARFFormValue::FormClass FC,
-                                 uint16_t DwarfVersion) const {
-  // First, check DWARF5 form classes.
-  if (Form < ArrayRef(DWARF5FormClasses).size() &&
-      DWARF5FormClasses[Form] == FC)
-    return true;
-  // Check more forms from extensions and proposals.
-  switch (Form) {
-  case DW_FORM_GNU_ref_alt:
-    return (FC == FC_Reference);
-  case DW_FORM_GNU_addr_index:
-    return (FC == FC_Address);
-  case DW_FORM_GNU_str_index:
-  case DW_FORM_GNU_strp_alt:
-    return (FC == FC_String);
-  case DW_FORM_LLVM_addrx_offset:
-    return (FC == FC_Address);
-  default:
-    break;
-  }
-
-  if (FC == FC_SectionOffset) {
-    if (Form == DW_FORM_strp || Form == DW_FORM_line_strp)
-      return true;
-    // In DWARF3 DW_FORM_data4 and DW_FORM_data8 served also as a section
-    // offset. If we don't have a DWARFUnit, default to the old behavior.
-    if (Form == DW_FORM_data4 || Form == DW_FORM_data8) {
-      if (U)
-        return U->getVersion() <= 3;
-
-      return DwarfVersion <= 3;
-    }
-  }
-
-  return false;
+bool DWARFFormValue::isFormClass(DWARFFormValue::FormClass FC) const {
+  return doesFormBelongToClass(Form, FC, U ? U->getVersion() : 3);
 }
 
 bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
@@ -783,3 +750,36 @@ DWARFFormValue::getAsFile(DILineInfoSpecifier::FileLineInfoKind Kind) const {
   }
   return std::nullopt;
 }
+
+bool doesFormBelongToClass(dwarf::Form Form, DWARFFormValue::FormClass FC,
+                                 uint16_t DwarfVersion) {
+    // First, check DWARF5 form classes.
+    if (Form < ArrayRef(DWARF5FormClasses).size() &&
+        DWARF5FormClasses[Form] == FC)
+      return true;
+    // Check more forms from extensions and proposals.
+    switch (Form) {
+    case DW_FORM_GNU_ref_alt:
+      return (FC == DWARFFormValue::FC_Reference);
+    case DW_FORM_GNU_addr_index:
+      return (FC == DWARFFormValue::FC_Address);
+    case DW_FORM_GNU_str_index:
+    case DW_FORM_GNU_strp_alt:
+      return (FC == DWARFFormValue::FC_String);
+    case DW_FORM_LLVM_addrx_offset:
+      return (FC == DWARFFormValue::FC_Address);
+    default:
+      break;
+    }
+
+    if (FC == DWARFFormValue::FC_SectionOffset) {
+      if (Form == DW_FORM_strp || Form == DW_FORM_line_strp)
+        return true;
+      // In DWARF3 DW_FORM_data4 and DW_FORM_data8 served also as a section
+      // offset.
+      if (Form == DW_FORM_data4 || Form == DW_FORM_data8)
+        return DwarfVersion <= 3;
+    }
+
+    return false;
+}

Sure, yeah, that seems OK!

[avl]

addressed comment(added new function doesFormBelongToClass to not depend on DWARFUnit existance)

[probinson]

LGTM too

[avl]

Thanks!

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp

786–790

https://llvm.org/docs/CodingStandards.html#id37 suggests implementing this like this:

[avl]

llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp

786–790

Thanks, will do that way.

[ayermolo]

Ah nice. This actually helps with something I am working on for BOLT. There was an internal change that did similar thing.

llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h

public:
   static DWARFFormValue createFromBlockValue(dwarf::Form F,
                                              ArrayRef<uint8_t> D);
   static DWARFFormValue createFromUnit(dwarf::Form F, const DWARFUnit *Unit,
                                        uint64_t *OffsetPtr);
 
   dwarf::Form getForm() const { return Form; }
   uint64_t getRawUValue() const { return Value.uval; }
 
-  bool isFormClass(FormClass FC) const;
+  /// Returns true if current value is of \p FC form class.
+  /// If current value does not have reference to original DWARFUnit
+  /// then \p DwarfVersion might be used to check form class of value.
+  bool isFormClass(FormClass FC, uint16_t DwarfVersion = 3) const;
   const DWARFUnit *getUnit() const { return U; }
   void dump(raw_ostream &OS, DIDumpOptions DumpOpts = DIDumpOptions()) const;
   void dumpSectionedAddress(raw_ostream &OS, DIDumpOptions DumpOpts,
                             object::SectionedAddress SA) const;
   void dumpAddress(raw_ostream &OS, uint64_t Address) const;
   static void dumpAddress(raw_ostream &OS, uint8_t AddressSize,
                           uint64_t Address);
   static void dumpAddressSection(const DWARFObject &Obj, raw_ostream &OS,

llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp

do {
 
     default:
       return false;
     }
   } while (Indirect);
   return true;
 }
 
-bool DWARFFormValue::isFormClass(DWARFFormValue::FormClass FC) const {
+bool DWARFFormValue::isFormClass(DWARFFormValue::FormClass FC, uint16_t DwarfVersion) const {
   // First, check DWARF5 form classes.
   if (Form < ArrayRef(DWARF5FormClasses).size() &&
       DWARF5FormClasses[Form] == FC)
     return true;
   // Check more forms from extensions and proposals.
   switch (Form) {
   case DW_FORM_GNU_ref_alt:
     return (FC == FC_Reference);
   case DW_FORM_GNU_addr_index:
     return (FC == FC_Address);
   case DW_FORM_GNU_str_index:
   case DW_FORM_GNU_strp_alt:
     return (FC == FC_String);
   case DW_FORM_LLVM_addrx_offset:
     return (FC == FC_Address);
   default:
     break;
   }
 
   if (FC == FC_SectionOffset) {
     if (Form == DW_FORM_strp || Form == DW_FORM_line_strp)
       return true;
     // In DWARF3 DW_FORM_data4 and DW_FORM_data8 served also as a section
     // offset. If we don't have a DWARFUnit, default to the old behavior.
-    if (Form == DW_FORM_data4 || Form == DW_FORM_data8)
-      return !U || U->getVersion() <= 3;
+    if (Form == DW_FORM_data4 || Form == DW_FORM_data8) {
+      if (U)
+        return U->getVersion() <= 3;
+
+      return DwarfVersion <= 3;
+    }
   }
 
   return false;
 }
 
 bool DWARFFormValue::extractValue(const DWARFDataExtractor &Data,
                                   uint64_t *OffsetPtr, dwarf::FormParams FP,
                                   const DWARFContext *Ctx,
DWARFFormValue::getAsFile(DILineInfoSpecifier::FileLineInfoKind Kind) const {
   DWARFUnit *DLU = const_cast<DWARFUnit *>(U)->getLinkedUnit();
   if (auto *LT = DLU->getContext().getLineTableForUnit(DLU)) {
     std::string FileName;
     if (LT->getFileNameByIndex(Value.uval, DLU->getCompilationDir(), Kind,
                                FileName))
       return FileName;
   }
   return std::nullopt;
 }

llvm/unittests/DebugInfo/DWARF/DWARFFormValueTest.cpp

 using namespace dwarf;
 
 namespace {
 
 bool isFormClass(dwarf::Form Form, DWARFFormValue::FormClass FC) {
   return DWARFFormValue(Form).isFormClass(FC);
 }
 
+bool isFormClass(dwarf::Form Form, DWARFFormValue::FormClass FC, uint16_t DwarfVersion) {
+  return DWARFFormValue(Form).isFormClass(FC, DwarfVersion);
+}
+
 TEST(DWARFFormValue, FormClass) {
   EXPECT_TRUE(isFormClass(DW_FORM_addr, DWARFFormValue::FC_Address));
   EXPECT_FALSE(isFormClass(DW_FORM_data8, DWARFFormValue::FC_Address));
   EXPECT_TRUE(isFormClass(DW_FORM_data8, DWARFFormValue::FC_Constant));
   EXPECT_TRUE(isFormClass(DW_FORM_data8, DWARFFormValue::FC_SectionOffset));
+  EXPECT_TRUE(isFormClass(DW_FORM_data8, DWARFFormValue::FC_SectionOffset, 3));
+  EXPECT_FALSE(isFormClass(DW_FORM_data8, DWARFFormValue::FC_SectionOffset, 5));
   EXPECT_TRUE(
       isFormClass(DW_FORM_sec_offset, DWARFFormValue::FC_SectionOffset));
   EXPECT_TRUE(isFormClass(DW_FORM_GNU_str_index, DWARFFormValue::FC_String));
   EXPECT_TRUE(isFormClass(DW_FORM_GNU_addr_index, DWARFFormValue::FC_Address));
   EXPECT_FALSE(isFormClass(DW_FORM_ref_addr, DWARFFormValue::FC_Address));
   EXPECT_TRUE(isFormClass(DW_FORM_ref_addr, DWARFFormValue::FC_Reference));
   EXPECT_TRUE(isFormClass(DW_FORM_ref_sig8, DWARFFormValue::FC_Reference));
 }