[Lldb-commits] [lldb] 2ec334d - [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

Alex Langford via lldb-commits lldb-commits at lists.llvm.org
Wed May 10 11:24:15 PDT 2023


Author: Alex Langford
Date: 2023-05-10T11:17:30-07:00
New Revision: 2ec334dc7b7329c6b71faa037a0d926e8e130c20

URL: https://github.com/llvm/llvm-project/commit/2ec334dc7b7329c6b71faa037a0d926e8e130c20
DIFF: https://github.com/llvm/llvm-project/commit/2ec334dc7b7329c6b71faa037a0d926e8e130c20.diff

LOG: [lldb][NFCI] Replace dw_form_t with llvm::dwarf::Form

LLDB currently defines `dw_form_t` as a `uint16_t` which makes sense.
However, LLVM also defines a similar type `llvm::dwarf::Form` which is
an enum backed by a `uint16_t`. Switching to the llvm implementation
means that we can more easily interoperate with the LLVM DWARF code.

Additionally, we get some type checking out of this: I found that
DWARFAttribute had a method called `FormAtIndex` that returned a
`dw_attr_t`. Although `dw_attr_t` is also a `uint16_t` under the hood,
the type checking benefits here are undeniable: If this had returned a
something of different signedness/width, we could have had some bad
bugs.

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

Added: 
    

Modified: 
    lldb/include/lldb/Core/dwarf.h
    lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
    lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
    lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
    lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/dwarf.h b/lldb/include/lldb/Core/dwarf.h
index e930200393c27..6fcf2f8cc0c78 100644
--- a/lldb/include/lldb/Core/dwarf.h
+++ b/lldb/include/lldb/Core/dwarf.h
@@ -22,7 +22,7 @@ namespace dwarf {
 }
 
 typedef uint16_t dw_attr_t;
-typedef uint16_t dw_form_t;
+typedef llvm::dwarf::Form dw_form_t;
 typedef llvm::dwarf::Tag dw_tag_t;
 typedef uint64_t dw_addr_t; // Dwarf address define that must be big enough for
                             // any addresses in the compile units that get

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
index 62d75c69afa86..5bd3b23dc95fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
@@ -41,7 +41,7 @@ DWARFAbbreviationDeclaration::extract(const DWARFDataExtractor &data,
 
   while (data.ValidOffset(*offset_ptr)) {
     dw_attr_t attr = data.GetULEB128(offset_ptr);
-    dw_form_t form = data.GetULEB128(offset_ptr);
+    auto form = static_cast<dw_form_t>(data.GetULEB128(offset_ptr));
 
     // This is the last attribute for this abbrev decl, but there may still be
     // more abbrev decls, so return MoreItems to indicate to the caller that

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
index 7922a14b33f18..1c69894f82eca 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.h
@@ -28,7 +28,8 @@ class DWARFAbbreviationDeclaration {
   bool HasChildren() const { return m_has_children; }
   size_t NumAttributes() const { return m_attributes.size(); }
   dw_form_t GetFormByIndex(uint32_t idx) const {
-    return m_attributes.size() > idx ? m_attributes[idx].get_form() : 0;
+    return m_attributes.size() > idx ? m_attributes[idx].get_form()
+                                     : dw_form_t(0);
   }
 
   // idx is assumed to be valid when calling GetAttrAndFormByIndex()

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
index a31ee861179c2..faba5f5e9f6e1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h
@@ -55,7 +55,7 @@ class DWARFAttributes {
   dw_attr_t AttributeAtIndex(uint32_t i) const {
     return m_infos[i].attr.get_attr();
   }
-  dw_attr_t FormAtIndex(uint32_t i) const { return m_infos[i].attr.get_form(); }
+  dw_form_t FormAtIndex(uint32_t i) const { return m_infos[i].attr.get_form(); }
   DWARFFormValue::ValueType ValueAtIndex(uint32_t i) const {
     return m_infos[i].attr.get_value();
   }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index bd2bb3d839c6a..1db71c0eccdf5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -177,7 +177,7 @@ bool DWARFDebugInfoEntry::Extract(const DWARFDataExtractor &data,
 
         case DW_FORM_indirect:
           form_is_indirect = true;
-          form = data.GetULEB128(&offset);
+          form = static_cast<dw_form_t>(data.GetULEB128(&offset));
           break;
 
         case DW_FORM_strp:

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
index 2b1d3b37a95a5..6ca17dcf47ff7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
@@ -25,7 +25,7 @@ using namespace lldb_private::dwarf;
 
 void DWARFFormValue::Clear() {
   m_unit = nullptr;
-  m_form = 0;
+  m_form = dw_form_t(0);
   m_value = ValueTypeTag();
 }
 
@@ -127,7 +127,7 @@ bool DWARFFormValue::ExtractValue(const DWARFDataExtractor &data,
       m_value.value.uval = data.GetMaxU64(offset_ptr, ref_addr_size);
       break;
     case DW_FORM_indirect:
-      m_form = data.GetULEB128(offset_ptr);
+      m_form = static_cast<dw_form_t>(data.GetULEB128(offset_ptr));
       indirect = true;
       break;
     case DW_FORM_flag_present:
@@ -321,9 +321,10 @@ bool DWARFFormValue::SkipValue(dw_form_t form,
       return true;
 
   case DW_FORM_indirect: {
-    dw_form_t indirect_form = debug_info_data.GetULEB128(offset_ptr);
-    return DWARFFormValue::SkipValue(indirect_form, debug_info_data, offset_ptr,
-                                     unit);
+      auto indirect_form =
+          static_cast<dw_form_t>(debug_info_data.GetULEB128(offset_ptr));
+      return DWARFFormValue::SkipValue(indirect_form, debug_info_data,
+                                       offset_ptr, unit);
   }
 
   default:
@@ -573,8 +574,10 @@ bool DWARFFormValue::IsBlockForm(const dw_form_t form) {
   case DW_FORM_block2:
   case DW_FORM_block4:
     return true;
+  default:
+    return false;
   }
-  return false;
+  llvm_unreachable("All cases handled above!");
 }
 
 bool DWARFFormValue::IsDataForm(const dw_form_t form) {
@@ -586,8 +589,10 @@ bool DWARFFormValue::IsDataForm(const dw_form_t form) {
   case DW_FORM_data4:
   case DW_FORM_data8:
     return true;
+  default:
+    return false;
   }
-  return false;
+  llvm_unreachable("All cases handled above!");
 }
 
 bool DWARFFormValue::FormIsSupported(dw_form_t form) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
index 90e07ea67f537..2a8843c1a0d45 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h
@@ -45,7 +45,7 @@ class DWARFFormValue {
   const DWARFUnit *GetUnit() const { return m_unit; }
   void SetUnit(const DWARFUnit *unit) { m_unit = unit; }
   dw_form_t Form() const { return m_form; }
-  dw_form_t& FormRef() { return m_form; }
+  dw_form_t &FormRef() { return m_form; }
   void SetForm(dw_form_t form) { m_form = form; }
   const ValueType &Value() const { return m_value; }
   ValueType &ValueRef() { return m_value; }
@@ -83,7 +83,7 @@ class DWARFFormValue {
   // Compile unit where m_value was located.
   // It may be 
diff erent from compile unit where m_value refers to.
   const DWARFUnit *m_unit = nullptr; // Unit for this form
-  dw_form_t m_form = 0;              // Form for this value
+  dw_form_t m_form = dw_form_t(0);   // Form for this value
   ValueType m_value;            // Contains all data for the form
 };
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
index c31a3d8ce913a..f530993381a93 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -158,6 +158,7 @@ void DWARFMappedHash::Prologue::AppendAtom(AtomType type, dw_form_t form) {
   atoms.push_back({type, form});
   atom_mask |= 1u << type;
   switch (form) {
+  default:
   case DW_FORM_indirect:
   case DW_FORM_exprloc:
   case DW_FORM_flag_present:
@@ -227,7 +228,7 @@ DWARFMappedHash::Prologue::Read(const lldb_private::DataExtractor &data,
   } else {
     for (uint32_t i = 0; i < atom_count; ++i) {
       AtomType type = (AtomType)data.GetU16(&offset);
-      dw_form_t form = (dw_form_t)data.GetU16(&offset);
+      auto form = static_cast<dw_form_t>(data.GetU16(&offset));
       AppendAtom(type, form);
     }
   }


        


More information about the lldb-commits mailing list