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–5 | 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. | |
14 | Maybe a separate change, but we should have a big-endian ELF tested in here too. | |
16–17 | 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). | |
21 | Non-zero entry point? | |
24 | Non-zero flags? | |
44–45 | For the purpose of this test, you don't need to have any sections in the segment. | |
47 | Similar comments to above apply to this and the MIPS case, but you don't need to necessarily test everything. | |
89 | Any particular reason you felt the need to test EM_MIPS? | |
llvm/test/tools/llvm-readobj/ELF/gnu-section-mapping-no-phdrs.test | ||
1–18 | 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? | |
24–25 | These two RUN lines need an associated comment to say what they're testing. | |
25–30 | 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–5 | Yes, in a few tests. | |
llvm/test/tools/llvm-readobj/ELF/gnu-file-headers.test | ||
14 | I am trying just to get rid of precompiled object for now. Such improvements should be done in a new separate test. | |
16–17 | I agree that these fields should be tested and that we should do this separatelly in a more specific tests. | |
24 | 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 | |
89 | 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. | |
24–25 | 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 | ||
---|---|---|
16–17 | 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. | |
45 | Can't you just delete the line entirely? | |
llvm/test/tools/llvm-readobj/ELF/hex-dump.test | ||
25–30 | 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 | ||
---|---|---|
16–17 |
Is the EM_386 ELF still used anywhere?