This rewrites a few tests to stop using the
trivial.obj.elf-x86-64 precompiled object
and removes it.
Details
Diff Detail
Event Timeline
| llvm/test/tools/llvm-readobj/ELF/Inputs/trivial.ll | ||
|---|---|---|
| 4 | Is the EM_386 ELF still used anywhere? | |
| llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test | ||
| 1 | Delete "a" | |
| 6–7 | 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. | |
| 12 | Maybe a separate change, but we should have a big-endian ELF tested in here too. | |
| 14–15 | 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). | |
| 19 | Non-zero entry point? | |
| 22 | Non-zero flags? | |
| 42–43 | For the purpose of this test, you don't need to have any sections in the segment. | |
| 45 | Similar comments to above apply to this and the MIPS case, but you don't need to necessarily test everything. | |
| 87 | Any particular reason you felt the need to test EM_MIPS? | |
| llvm/test/tools/llvm-readobj/ELF/gnu-section-mapping-no-phdrs.test | ||
| 1 | in an object | |
| 5 | I think this line is also important to this test. | |
| 7 | This needs a {{$}} or --match-full-lines to prevent there being any other data on the same line. | |
| llvm/test/tools/llvm-readobj/ELF/hex-dump.test | ||
| 4 | Don't we need a check somewhere that shows that dumping .strtab produces expected output? | |
| 26–27 | These two RUN lines need an associated comment to say what they're testing. | |
| 29 | This should be moved up to its associated RUN line and a comment added explaining what its purpose is. | |
| llvm/test/tools/llvm-readobj/ELF/thin-archive-paths.test | ||
| 1 | (Aside: I'm not sure this test should be in the ELF directory. It is testing generic functionality) | |
- Addressed review comments.
| llvm/test/tools/llvm-readobj/ELF/Inputs/trivial.ll | ||
|---|---|---|
| 4 | Yes, in a few tests. | |
| llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test | ||
| 12 | I am trying just to get rid of precompiled object for now. Such improvements should be done in a new separate test. | |
| 14–15 | I agree that these fields should be tested and that we should do this separatelly in a more specific tests. | |
| 22 | There are no specific flags for x64 and i386 and yaml2obj does not allow to use an arbitrary value. (funny that current logic of obj2yaml crashes with llvm-unreachable if the description has EM_386 | |
| 87 | I just tried to convert the original test and for some reason it had a test for MIPS. | |
| llvm/test/tools/llvm-readobj/ELF/hex-dump.test | ||
| 4 | I've changed .strtab to .shstrtab, which is dumped below. | |
| 26–27 | It had the original comment saying "Test we dump the section only once.". I've expanded it. | |
| llvm/test/tools/llvm-readobj/ELF/thin-archive-paths.test | ||
| 1 | I'll move it out. | |
| llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test | ||
|---|---|---|
| 14–15 | Okay, that's fine, but I do want to make sure it doesn't get overlooked, because at least in my most common usage, we use non-zero values for these two fields. | |
| 43 | Can't you just delete the line entirely? | |
| llvm/test/tools/llvm-readobj/ELF/hex-dump.test | ||
| 29 | The "Both 2..." original comment is probably a bit redundant now that you have the sanity check comment above. | |
- Addressed review comments.
| llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test | ||
|---|---|---|
| 14–15 | ||
Is the EM_386 ELF still used anywhere?