[PATCH] D87601: [obj2yaml] Add support for dumping the .debug_addr(v5) section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-addr.yaml:7
+# RUN: yaml2obj --docnum=1 %s -DADDRESS=0xFFFFFFFFFFFFFFFF \
+# RUN:   -DADDRSIZE1=4 -DADDRSIZE2=4 | obj2yaml | \
+# RUN:   FileCheck %s --check-prefix=BASIC --implicit-check-not=Sections \
----------------
Any reason you have separate variables for the two address sizes? As far as I can see, they are identical always.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-addr.yaml:8
+# RUN:   -DADDRSIZE1=4 -DADDRSIZE2=4 | obj2yaml | \
+# RUN:   FileCheck %s --check-prefix=BASIC --implicit-check-not=Sections \
+# RUN:     -DLENGTH12=0x0000000000000014 \
----------------
This is more robust against other data being emitted in the future causing failures, in my opinion.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-addr.yaml:9-13
+# RUN:     -DLENGTH12=0x0000000000000014 \
+# RUN:     -DADDRSIZE12=0x08             \
+# RUN:     -DADDR=0xFFFFFFFFFFFFFFFF     \
+# RUN:     -DLENGTH34=0x000000000000000C \
+# RUN:     -DADDRSIZE34=0x04
----------------
I found the naming scheme of these variables a bit confusing, as I was trying to match up "12" with the value "14", when really you meant "length one-two". I'd just go with `LENGTH1` and `LENGTH2` etc, i.e. the number of times the value is reused is irrelevant to the naming in this contaxt, at least.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-addr.yaml:82
+  debug_addr:
+    ## a DWARF32 address table.
+    - Version:     5
----------------
Nit: here and below.


================
Comment at: llvm/test/tools/obj2yaml/ELF/DWARF/debug-addr.yaml:183
+## c) Test dumping a address table whose version isn't 5.
+## This makes the DWARF parser fail to parse it and we will dump it as a raw
+## content section.
----------------
I think this is a little cleaner English.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87601