[PATCH] D71662: [llvm-readobj][test] - Stop using Inputs/trivial.obj.elf-x86-64.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 01:25:15 PST 2019
jhenderson added inline comments.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/Inputs/trivial.ll:4
; llc -mtriple=x86_64-pc-win32 trivial.ll -filetype=obj -o trivial.obj.coff-x86-64
; llc -mtriple=i386-linux-gnu trivial.ll -filetype=obj -o trivial.obj.elf-i386 -relocation-model=pic
; llc -mtriple=i386-apple-darwin10 trivial.ll -filetype=obj -o trivial.obj.macho-i386 -relocation-model=pic
----------------
Is the EM_386 ELF still used anywhere?
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:1
-RUN: llvm-readelf -h %p/Inputs/trivial.obj.elf-i386 \
-RUN: | FileCheck %s -check-prefix ELF32
-RUN: llvm-readelf --file-header %p/Inputs/trivial.obj.elf-i386 \
-RUN: | FileCheck %s -check-prefix ELF32
-RUN: llvm-readelf --file-headers %p/Inputs/trivial.obj.elf-i386 \
-RUN: | FileCheck %s -check-prefix ELF32
-RUN: llvm-readelf -h %p/Inputs/trivial.obj.elf-x86-64 \
-RUN: | FileCheck %s -check-prefix ELF64
-RUN: llvm-readelf --file-header %p/Inputs/trivial.obj.elf-x86-64 \
-RUN: | FileCheck %s -check-prefix ELF64
-RUN: llvm-readelf --file-headers %p/Inputs/trivial.obj.elf-x86-64 \
-RUN: | FileCheck %s -check-prefix ELF64
-RUN: llvm-readelf -h %p/Inputs/trivial.obj.elf-mipsel \
-RUN: | FileCheck %s -check-prefix MIPSEL
-RUN: llvm-readelf --file-header %p/Inputs/trivial.obj.elf-mipsel \
-RUN: | FileCheck %s -check-prefix MIPSEL
-RUN: llvm-readelf --file-headers %p/Inputs/trivial.obj.elf-mipsel \
-RUN: | FileCheck %s -check-prefix MIPSEL
+## In this test we test how llvm-readelf prints a file headers.
----------------
Delete "a"
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:6-7
+# RUN: llvm-readelf -h %t1 | FileCheck %s --check-prefix=I386
+# RUN: llvm-readelf --file-header %t1 | FileCheck %s --strict-whitespace --check-prefix=I386
+# RUN: llvm-readelf --file-headers %t1 | FileCheck %s --strict-whitespace --check-prefix=I386
----------------
You should probably either add `{{$}}` to the end of each line or use `--match-full-lines` to show there's no more data after that being checked on each line. If there were, it might not get caught by this test.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:12
+# I386-NEXT: Class: ELF32
+# I386-NEXT: Data: 2's complement, little endian
+# I386-NEXT: Version: 1 (current)
----------------
Maybe a separate change, but we should have a big-endian ELF tested in here too.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:14-15
+# I386-NEXT: Version: 1 (current)
+# I386-NEXT: OS/ABI: UNIX - System V
+# I386-NEXT: ABI Version: 0x0
+# I386-NEXT: Type: REL (Relocatable file)
----------------
We should have at least one test case with non-zero OS/ABI and ABI Version fields. Possibly we could test those separately (we should for example probably enumerate the OS/ABI values in a separate test to show that they are all GNU compatible).
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:19
+# I386-NEXT: Version: 0x1
+# I386-NEXT: Entry point address: 0x0
+# I386-NEXT: Start of program headers: 52 (bytes into file)
----------------
Non-zero entry point?
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:22
+# I386-NEXT: Start of section headers: 112 (bytes into file)
+# I386-NEXT: Flags: 0x0
+# I386-NEXT: Size of this header: 52 (bytes)
----------------
Non-zero flags?
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:42-43
+ Flags: [ PF_R ]
+ Sections:
+ - Section: .foo
+
----------------
For the purpose of this test, you don't need to have any sections in the segment.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:45
+
+## Case 2: Test EM_X86_64 object.
+# RUN: yaml2obj %s --docnum=2 -o %t2
----------------
Similar comments to above apply to this and the MIPS case, but you don't need to necessarily test everything.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test:87
+
+## Case 3: Test EM_MIPS object.
+# RUN: yaml2obj %s --docnum=3 -o %t3
----------------
Any particular reason you felt the need to test EM_MIPS?
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-section-mapping-no-phdrs.test:1
-RUN: llvm-readelf --section-mapping %p/Inputs/trivial.obj.elf-x86-64 | FileCheck %s
-CHECK: Section to Segment mapping:
-CHECK-NEXT: Segment Sections...
-CHECK-NEXT: None .text .rela.text .data .bss .rodata.str1.1 .note.GNU-stack .shstrtab .symtab .strtab {{$}}
-CHECK-NOT: {{.}}
+## Test the behaviour of --section-mapping when there are no section headers in a object.
+# RUN: yaml2obj %s -o %t
----------------
in an object
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-section-mapping-no-phdrs.test:5
-CHECK-NEXT: None .text .rela.text .data .bss .rodata.str1.1 .note.GNU-stack .shstrtab .symtab .strtab {{$}}
-CHECK-NOT: {{.}}
----------------
I think this line is also important to this test.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-section-mapping-no-phdrs.test:7
+# CHECK-NEXT: Segment Sections...
+# CHECK-NEXT: None .foo .strtab .shstrtab
+
----------------
This needs a {{$}} or --match-full-lines to prevent there being any other data on the same line.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/hex-dump.test:4
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readelf --file-header --hex-dump=.strtab %t > %t.hexdump.out
+# RUN: llvm-readelf -h --hex-dump .strtab %t > %t.hexdump.1
----------------
Don't we need a check somewhere that shows that dumping .strtab produces expected output?
================
Comment at: llvm/test/tools/llvm-readobj/ELF/hex-dump.test:26-27
+# RUN: | FileCheck %s --check-prefix=ELF
+# RUN: llvm-readobj -x 2 -x .shstrtab -x 3 -x not_exist %t 2>&1 \
+# RUN: | FileCheck %s -DFILE=%t --check-prefixes=ELF-WARN,ELF
----------------
These two RUN lines need an associated comment to say what they're testing.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/hex-dump.test:29
-# ELF-SEC: [ 9] .strtab
+# ELF-SEC: [ 2] .shstrtab
----------------
This should be moved up to its associated RUN line and a comment added explaining what its purpose is.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/thin-archive-paths.test:1
# RUN: rm -rf %t
# RUN: mkdir -p %t/a/b
----------------
(Aside: I'm not sure this test should be in the ELF directory. It is testing generic functionality)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71662/new/
https://reviews.llvm.org/D71662
More information about the llvm-commits
mailing list