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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h:19
   struct Header {
-    uint32_t length;    // The total length of the entries for that set, not
-                        // including the length field itself.
-    uint16_t version;   // The DWARF version number
-    uint32_t cu_offset; // The offset from the beginning of the .debug_info
-                        // section of the compilation unit entry referenced by
-                        // the table.
-    uint8_t addr_size;  // The size in bytes of an address on the target
-                        // architecture. For segmented addressing, this is the
-                        // size of the offset portion of the address
-    uint8_t seg_size; // The size in bytes of a segment descriptor on the target
-                      // architecture. If the target system uses a flat address
-                      // space, this value is 0.
+    uint32_t length = 0;    // The total length of the entries for that set,
+                            // not including the length field itself.
----------------
Nit: Can you put these above the members (because of the 80 col limit) and make them Doxygen comments (`///`)?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h:47
   const Header &GetHeader() const { return m_header; }
-
+  lldb::offset_t GetNextOffset() const { return m_next_offset; }
   const Descriptor &GetDescriptorRef(uint32_t i) const {
----------------
`dw_offset_t`? 


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp:63-67
+    // with the current DWARFDebugArangeSet. Previously if there was an error,
+    // like finding two zero terminator addresses early, the previous
+    // DWARFDebugArangeSet would stop parsing the DWARFDebugArangeSet data and
+    // then the next DWARFDebugArangeSet would end up parsing left over
+    // addresses as a new DWARFDebugArangeSet header and it would error out.
----------------
Nit: I would phrase this as an example rather than in terms of what the old code did.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99401/new/

https://reviews.llvm.org/D99401