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

Reply via email to