[PATCH] D78805: [llvm-readobj][test] - Stop using binaries in gnu-phdrs.test, split and refine it.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:294
+
+## Check that -section-mapping produces a mapping and not the program headers.
+# RUN: llvm-readelf --section-mapping %t64.elf \
----------------
jhenderson wrote:
> Perhaps section mapping should be split into its own test, since most of the testing here isn't needed for it? It would be clear what test to look in for that option's testing then.
> 
> Also `--section-mapping` (similar comments below).
Done.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:308
+
+## Check/document how we dump Arm specific program headers.
+# RUN: yaml2obj -DBITS=64 -DMACHINE=EM_ARM %s -o %tarm.elf
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > Arm -> ARM
> > > 
> > > Could we just fix the ARM header printing so that you don't need "document" in the comment?
> > To fix ARM we need to add/change a test. This patch introduces a proper place for it (`gnu-phdrs.test`)
> Right, what I meant was fix it in a child commit, and not bother with the "document" part of the comment. Sorry I wasn't clear.
I've tested GNU readelf 2.31.1, it prints:

```
  LOPROC+0       0x0000000000000548 0x0000000000001000 0x0000000000001000
                 0x0000000000000003 0x0000000000000003         0x1
  EXIDX          0x0000000000000548 0x0000000000001000 0x0000000000001000
                 0x0000000000000003 0x0000000000000003         0x1
  LOPROC+0x2     0x0000000000000548 0x0000000000001000 0x0000000000001000
                 0x0000000000000003 0x0000000000000003         0x1
```

We kind of follow GNU, so I think we can leave it as is for now. I am not sure if we want to follow more
and print "LOPROC+X" instead of "<unknown>: X", anyways it would be something related to tool general
behavior, not ARM specific. I've changed "Document/check" to "Check".


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

https://reviews.llvm.org/D78805