clayborg added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:489 const llvm::Optional<llvm::DWARFDebugRnglistTable> &DWARFUnit::GetRnglist() { + if (GetVersion() >= 5 && !m_rnglist_table_done) { + m_rnglist_table_done = true; ---------------- jankratochvil wrote: > `if (GetVersion() < 5) return llvm::None;` is no longer possible with the > returned reference. Do we actually need "m_rnglist_table_done" here? Can't we just check if m_rnglist_table has a value since it is an optional? ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:490 + if (GetVersion() >= 5 && !m_rnglist_table_done) { + m_rnglist_table_done = true; + if (auto table_or_error = ---------------- Should we check m_ranges_base here to make sure it has a value? ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:491-494 + if (auto table_or_error = + ParseListTableHeader<llvm::DWARFDebugRnglistTable>( + m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(), + m_ranges_base, DWARF32)) ---------------- Will this function call return an error if "m_ranges_base" doesn't have a value? ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:508-513 + if (!m_ranges_base) { + GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError( + "%8.8x: DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base", + GetOffset()); + return llvm::None; + } ---------------- Do we need this check? We should probably be checking "m_ranges_base" in the DWARFUnit::GetRnglist() and report the error there? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98289/new/ https://reviews.llvm.org/D98289 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits