[Lldb-commits] [PATCH] D99401: Fix .debug_aranges parsing issues introduced with llvm error handling in LLDB

jeffrey tan via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Mar 29 15:48:49 PDT 2021

yinghuitan added inline comments.

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp:96
   const uint32_t header_size = *offset_ptr - m_offset;
   const uint32_t tuple_size = m_header.addr_size << 1;
   uint32_t first_tuple_offset = 0;
Unrelated, but I think `2 * m_header.addr_size` is more readable considering compiler would optimize it into bit shifting anyway.

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp:98-99
   uint32_t first_tuple_offset = 0;
   while (first_tuple_offset < header_size)
     first_tuple_offset += tuple_size;
Unrelated to this diff, but I find this simple round up can be shorten to:
first_tuple_offset = ((header_size + tuple_size - 1) / tuple_size) * tuple_size;

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp:134-135
+      // Some linkers will zero out the length field for some .debug_aranges
+      // entries if they were stripped. We also could watch out for multiple
+      // entries at address zero and remove those as well.
+      if (arangeDescriptor.length > 0)
Maybe add a TODO for "We also could watch out for multiple entries at address zero and remove those as well" since we did not do it here?

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h:61
+  dw_offset_t m_offset;
+  dw_offset_t m_next_offset;
   Header m_header;
Do you mind add comment to explain this field? It is not very clear that it points to the section offset after this .debug_aranges set. Also, maybe it is more meaningful rename it to `m_set_end_offset`? 

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list