clayborg added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:95-96
const DWARFDataExtractor &DWARFContext::getOrLoadMacroData() {
- return LoadOrGetSection(eSectionTypeDWARFDebugMacro, llvm::None,
- m_data_debug_macro);
+ return LoadOrGetSection(eSectionTypeDWARFDebugMacro,
+ eSectionTypeDWARFDebugMacro, m_data_debug_macro);
}
----------------
This enum should be different otherwise this code change does nothing.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp:22
+ // LLDB doesn't support DWARF64, so we always have item size of 4.
+ uint64_t offset = cu_offset + 4 * index;
+ return data->GetU32(&offset);
----------------
I would make this DWARF64 compatible.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h:28
+struct DWARFStrOffsetsInfo {
+ lldb::offset_t cu_offset;
+ const lldb_private::DWARFDataExtractor *data;
----------------
"cu_offset" sounds like it is actually the offset of the compile unit. This is
actually the compile unit offset for the string data for the compile unit in
the string index section right? Maybe a name like "cu_str_offset" or
"cu_strx_offset" would be a better name?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h:29
+ lldb::offset_t cu_offset;
+ const lldb_private::DWARFDataExtractor *data;
+
----------------
Can "data" ever be NULL? If not, then change this to a reference and add a
constructor that inits the reference?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h:31
+
+ bool IsValid() const { return cu_offset && cu_offset != DW_INVALID_OFFSET; }
+ uint64_t GetOffset(uint64_t index) const;
----------------
if you are going to keep "data" as a pointer, shouldn't this function also
check "data" against nullptr?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1219
- comp_unit.SetDebugMacros(ParseDebugMacros(§_offset));
+ DWARFStrOffsetsInfo str_offsets_info = {};
+ str_offsets_info.cu_offset = dwarf_cu->GetStrOffsetsBase();
----------------
If we want to change "data" to be a reference, we will need a constructor:
```
DWARFStrOffsetsInfo str_offsets_info(dwarf_cu->GetStrOffsetsBase(),
symfile.GetDWARFContext().getOrLoadStrOffsetsData());
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130062/new/
https://reviews.llvm.org/D130062
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
