clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Nice patch! A few minor fixes, see inlined comments.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:78
+uint32_t DWARFCompileUnit::GetDWARF5HeaderSize(uint8_t unitType) {
+ switch (unitType) {
----------------
Rename to GetHeaderByteSize() and do:
```
uint32_t DWARFCompileUnit::GetHeaderByteSize() {
if (m_version < 5)
return m_is_dwarf64 ? 23 : 11;
switch (m_unit_type) {
case llvm::dwarf::DW_UT_compile:
case llvm::dwarf::DW_UT_partial:
return 12;
case llvm::dwarf::DW_UT_skeleton:
case llvm::dwarf::DW_UT_split_compile:
return 20;
case llvm::dwarf::DW_UT_type:
case llvm::dwarf::DW_UT_split_type:
return 24;
}
llvm_unreachable("invalid UnitType.");
}
```
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:38-42
uint32_t GetHeaderByteSize() const override {
+ if (m_version >= 5)
+ return GetDWARF5HeaderSize(m_unit_type);
return m_is_dwarf64 ? 23 : 11;
}
----------------
Move this to .cpp file and inline contents of GetDWARF5HeaderSize() into the
the body of the function.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:45
private:
+ static uint32_t GetDWARF5HeaderSize(uint8_t unitType);
+
----------------
Remove this
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp:442
+ ReadDescriptors(debug_line_data, offset_ptr);
+ uint8_t dirCount = debug_line_data.GetULEB128(offset_ptr);
+ for (int i = 0; i < dirCount; ++i) {
----------------
"dirCount" is documented in the DWARF 5 spec to be a "ubyte" not a ULEB128
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp:557-558
-const char *DWARFFormValue::AsCString() const {
- SymbolFileDWARF *symbol_file = m_cu->GetSymbolFileDWARF();
-
+const char *DWARFFormValue::AsCString(SymbolFileDWARF *symbol_file,
+ bool is64Bit) const {
if (m_form == DW_FORM_string) {
----------------
Revert signature and get info form m_cu. There are no other calls to
AsCString() that manually specify a different symbol file and 64 bit value.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp:613-616
+const char *DWARFFormValue::AsCString() const {
+ return AsCString(m_cu->GetSymbolFileDWARF(), m_cu->IsDWARF64());
+}
+
----------------
Remove this as there are no other calls to AsCString() that manually specify a
different symbol file and 64 bit value.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFFormValue.h:77
const char *AsCString() const;
+ const char *AsCString(SymbolFileDWARF *Dwarf, bool Is64Bit) const;
dw_addr_t Address() const;
----------------
This doesn't seem to be needed in this patch? The SymbolFileDWARF and Is64Bit
can be grabbed from the m_cu
https://reviews.llvm.org/D51935
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
