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

[Xing GUO via Phabricator via llvm-commits]

Higuoxing marked 2 inline comments as done.
Higuoxing added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-loclists.yaml:808
+  debug_loclists:
+    - Offsets: []
+      Lists:
----------------
jhenderson wrote:
> This is fine, but I think an expectation might be that if `OffsetEntryCount: 0` is specified, (and not `Offsets: []`), it would have the same effect.
> 
> I think you want two more cases:
> 
> 1) `OffsetEntryCount: 0` and no `Offsets` key at all (will result in no offsets).
> 2) `OffsetEntryCount: 0` and a non-empty `Offsets` key (will result in 0 in offset_entry_count, but with offsets).
> 
> Does that make sense to you?
Done. But we should land D85006 first.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-loclists.yaml:757-758
+
+## t) Test that yaml2obj emits an error message if we use an unimplemented DWARF expression
+## operator.
+
----------------
jhenderson wrote:
> Higuoxing wrote:
> > jhenderson wrote:
> > > Rather than a known value that we one day will likely support, causing this test to be updated, perhaps we should pick a value that isn't supported by the DWARF spec?
> > I'm not sure if I'm doing right here. If we pick a value that isn't supported by the DWARF sepc, the error message
> > 
> > ```
> > yaml2obj: error: DWARF expression: DW_OP_blah is not supported
> >                                    ^~~~~~~~~~
> > ```
> > 
> > will be left untested.
> Ah, I see. Perhaps we should have both then. (One with arbitrary unrecognised hex, and the other using a recognised value which isn't supported).
I've added one test that use a recognized value `DW_OP_entry_value` since it might not be implemented recently, so that we don't have to update this test very frequently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84234