[PATCH] D84234: [DWARFYAML] Implement the .debug_loclists section.

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

I don't see anything out of the ordinary here, but I'll leave it others to have the final say.



================
Comment at: llvm/include/llvm/ObjectYAML/DWARFYAML.h:190
+  dwarf::LocationAtom Operator;
+  std::vector<yaml::Hex64> Values;
+};
----------------
Higuoxing wrote:
> Higuoxing wrote:
> > There are some operators that take a size and a block of that size. I'm not sure if it's ok to use a `std::vector<>` here.
> I think we can add a new field called `BlockSize` which is used to overwrite the size of blocks.
> 
> ```
> struct DWARFOperation {
>   dwarf::LocationAtom Operator;
>   Optional<yaml::Hex64> BlockSize;
>   std::vector<yaml::Hex64> Values;
> };
> ```
> 
> e.g., `DW_OP_implicit_value` takes two operands, the first operand is an ULEB128 size, the second operand is a block of that size. We can use `std::vector<yaml::Hex64>` to hold a series of bytes, and the size of this block can be overwritten by `BlockSize`.
Supporting DW_OP_entry_value is going to be amusing, as that takes an entire nested dwarf expression as an operand. But I think it would be fine even if the nested expression is simply printed in hex (in fact, I would be fine if initially _all_ dwarf expressions are simply printed in hex).


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-loclists.yaml:22-92
+# DWARF32-LE-NEXT:   0000: 3D000000 05000800 03000000 0C000000  |=...............|
+##                         ^-------                             unit_length (4-byte)
+##                                  ^---                        version (2-byte)
+##                                      ^-                      address_size (1-byte)
+##                                        ^-                    segment_selector_size (1-byte)
+##                                           ^-------           offset_entry_count (4-byte)
+##                                                    ^-------  offsets[0] (4-byte)
----------------
Would it be better to verify this structurally by checking llvm-dwarfdump output ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84234