[PATCH] D81217: [ObjectYAML][DWARF] Support emitting .debug_ranges section in ELFYAML.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thanks. A number of relatively minor test comments only from me.



================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-ranges.yaml:11-22
+#         DWARF-HEADER: Index: 1
+#    DWARF-HEADER-NEXT: Name: .debug_ranges (1)
+#    DWARF-HEADER-NEXT: Type: SHT_PROGBITS (0x1)
+#    DWARF-HEADER-NEXT: Flags [ (0x0)
+#    DWARF-HEADER-NEXT: ]
+#    DWARF-HEADER-NEXT: Address: 0x0
+#    DWARF-HEADER-NEXT: Offset: 0x40
----------------
Nit: you need an extra space to make these line up with the CONTENT checks.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-ranges.yaml:23
+#    DWARF-HEADER-NEXT: EntrySize: 0
+#      DWARF-LE-CONTENT: SectionData (
+# DWARF-LE-CONTENT-NEXT:   0000: 00000000 10000000 00000000 20000000
----------------
You can make this `DWARF-LE-CONTENT-NEXT` since it should follow the `EntrySize` (or you can add extra directives to make it do so). Same goes for the big endian version.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-ranges.yaml:162
+    - Offset:   0
+      AddrSize: 0x04
+      Entries:
----------------
Nit: It doesn't matter much, but I'd make `AddrSize: 0x8` from here on, since it is the natural address size of X86_64


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-ranges.yaml:164-165
+      Entries:
+        - LowOffset:  0x00000000
+          HighOffset: 0x00000000
+
----------------
I'd avoid using 0 for both high and low, since that is a special meaning for .debug_ranges. Perhaps 1 and 2 would be better. Also, perhaps worth an extra case showing that a) 0, 0 can be used explicitly, and that b) 0xffffffffffffffff, <something> can be used too, since those are both special kinds of entries in the table. You could put them in the original pair of tests if you want, or in a new case, whichever you prefer.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-ranges.yaml:214-215
+    AddressAlign: 2                  # 0 by default.
+    Address:      0x0000000000002020 # 0x00 by default.
+    Offset:       0x00000050         # 0x40 for the first section.
+    Size:         0x10               # Set the "Size" so that we can reuse the check tag "OVERRIDDEN".
----------------
Nit: don't pad your numbers with leading zeros unless necessary (I don't think it is necessary here). Same goes throughout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81217