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); } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits