[PATCH] D83624: [DWARFYAML] Implement the .debug_rnglists section.

[Xing GUO via Phabricator via llvm-commits]

Higuoxing added inline comments.


================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:524-525
+      }
+      // Write DW_RLE_end_of_list to terminate this entry.
+      BufferOS.write_zeros(1);
+      Length += 1;
----------------
jhenderson wrote:
> It would probably be better to be more explicit about writing DW_RLE_end_of_list, e.g. `writeInteger(DW_RLE_end_of_list, ...)`.
> 
> It may make more sense to get users to explicitly specify the end-of-list marker. This would be in keeping with how `DT_NULL` works for SHT_DYNAMIC ELF sections. This means that users can deliberately write lists without trailing terminators.
The trailing terminator is removed here. Users should specify the DW_RLE_end_of_list manually.


================
Comment at: llvm/lib/ObjectYAML/DWARFYAML.cpp:242
+  IO.mapOptional("Length", RnglistsEntry.Length);
+  IO.mapRequired("Version", RnglistsEntry.Version);
+  IO.mapOptional("AddressSize", RnglistsEntry.AddrSize);
----------------
jhenderson wrote:
> `Version` should be optional, and have a value of 5 by default, since it's the only supported version of the format currently.
The version field of the .debug_addr section is required (https://reviews.llvm.org/D81541#inline-750289). Shall we make that one optional as well? Or is there any particular reason that makes these two sections different?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83624