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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:3
+
+## Check that -l,--program-headers and --segments is the same option.
+# RUN: yaml2obj -DBITS=32 -DMACHINE=EM_386 %s -o %t32.elf
----------------
Space after "-l,".

Also "is the same" -> "are the same"


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:5
+# RUN: yaml2obj -DBITS=32 -DMACHINE=EM_386 %s -o %t32.elf
+# RUN: llvm-readelf -l %t32.elf 2>&1 > %t.readobj-l.txt
+# RUN: llvm-readelf --program-headers %t32.elf 2>&1 > %t.readobj-pheaders.txt
----------------
readobj -> readelf here and below?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:19-20
+
+#      ELF32:Elf file type is EXEC (Executable file)
+# ELF32-NEXT:Entry point 0x12345678
+# ELF32-NEXT:There are 23 program headers, starting at offset 52
----------------
I think these two lines are not needed - they aren't part of the program header dumping, right? Same goes for ELF64.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:152
+ProgramHeaders:
+## Case 1: an arbitraty segment with sections..
+  - Type:  PT_PHDR
----------------
arbitraty -> arbitrary

Too many full stops.


================
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 \
----------------
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).


================
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
----------------
Arm -> ARM

Could we just fix the ARM header printing so that you don't need "document" in the comment?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-phdrs.test:325
+
+## Check the output when an object has no section headers.
+## RUN: llvm-objcopy --strip-sections %t64.elf %tno-shdrs.o
----------------
What's the reasoning from removing this from its separate test? I don't think that gives us anything.


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

https://reviews.llvm.org/D78805