[PATCH] D71766: [llvm-readobj/llvm-readelf][test] - Add testing for EI_OSABI and EI_ABIVERSION fields of an ELF header.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I wouldn't go as far as to say the fields were never tested - they are partially, just not exhaustively tested in ELF/file-headers.test.



================
Comment at: llvm/test/tools/llvm-readobj/ELF/file-header-os-abi-version.test:14
+# RUN: llvm-readelf --file-headers %t.abiver.any | FileCheck %s --check-prefix=ABIVER-ANY-GNU
+## EI_ABIVERSION is set to a maximum possible value.
+# RUN: yaml2obj %s --docnum=3 -o %t.abiver.max
----------------
a maximum -> the maximum


================
Comment at: llvm/test/tools/llvm-readobj/ELF/file-header-os-abi-version.test:52
+
+## Test all supported EI_OSABI kinds.
+
----------------
I think OS/ABI testing should be split into a separate file from ABIVersion, as the two are separate fields.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/file-header-os-abi-version.test:60
+# OSABI-NONE-LLVM: OS/ABI: SystemV (0x0){{$}}
+# OSABI-NONE-GNU: OS/ABI: UNIX - System V{{$}}
+
----------------
Nit: here and throughout, it would be nice if the "OS/ABI" bit lined up:
```
# OSABI-NONE-LLVM: OS/ABI: SystemV (0x0){{$}}
# OSABI-NONE-GNU:  OS/ABI: UNIX - System V{{$}}
```


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

https://reviews.llvm.org/D71766