[PATCH] D70541: [DWARF][RISCV] Test resolving of RISC-V relocations

[Luís Marques via Phabricator via llvm-commits]

luismarques marked an inline comment as done.
luismarques added inline comments.


================
Comment at: llvm/test/tools/llvm-dwarfdump/RISCV/riscv-relocs.yaml:19
+    # DW_AT_high_pc and DW_AT_stmt_list.
+    Content:         280000000400000000000801000000000C000000002F00000000000000000042000000000000000100000000280000000400000000000801000000000C000000002F00000000000000000042000000000000000200000000280000000400000000000801000000000C000000002F00000000000000000042000000000000000300000000280000000400000000000801000000000C000000002F00000000000000000042000000000000000400000000280000000400000000000801000000000C000000002F00000000000000000042000000000000000500000000280000000400000000000801000000000C000000002F00000000000000000042000000000000000600000000280000000400000000000801000000000C000000002F00000000000000000042000000000000000700000000280000000400000000000801000000000C000000002F00000000000000000042420000000000000800000000280000000400000000000801000000000C000000002F00000000000000000042420000000000000900000000280000000400000000000801000000000C000000002F00000000000000000042424242000000000A00000000280000000400000000000801000000000C000000002F00000000000000000042424242000000000B00000000280000000400000000000801000000000C000000002F00000000000000000042424242424242000C00000000280000000400000000000801000000000C000000002F00000000000000000042424242424242000D00000000280000000400000000000801000000000C000000002F00000000000000000042000000000000000e00000000
+  - Name:            .rela.debug_info
----------------
MaskRay wrote:
> luismarques wrote:
> > MaskRay wrote:
> > > For this test, I feel that the opaque content makes the YAML test worse than an assembly test. You can use llvm-readelf -r or llvm-readobj -r to check the presence of intended relocations.
> > It's not ideal but I don't think we can have an assembly test file we can process with all of the relocations we can resolve. (The goal is to check that they are correctly resolved, not that they are present).
> The opaque and length `Content` makes the test very difficult to modify. Neither does it explain what each octet does.
> 
> Assembly augmented with comments will be easier to understand and modify, e.g.
> 
> ```
> .byte 8  # Address size
> .byte 0  # Segment selector size
> .long 0  # Offset entry count
> ```
Changing the test to be an assembly file proved incompatible with adding all of the required relocations (if you know of a way to do it please say so). So instead I added an assembly file as a comment, which documents the binary blobs and allows people to easily recreate and extend the individual relocation tests below. Does that sufficiently address your concerns @MaskRay ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70541