This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj][test] - Refactor mips-st-other.test
ClosedPublic

Authored by grimar on Dec 18 2019, 1:48 AM.

Details

Summary

This removes 2 precompiled binaries, adds testing
for STO_* flags missing, refines and renames the test.

Diff Detail

Event Timeline

grimar created this revision.Dec 18 2019, 1:48 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar updated this revision to Diff 234486.Dec 18 2019, 1:49 AM
  • Fix a comment.
jhenderson added inline comments.Dec 18 2019, 2:07 AM
llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test
4

-o

5–6

Is --strict-whitespace important for this test? Do we anticipate spacing differences from other non-MIPS other bits?

37–38

Change this to: "Use a different symbol for STO_MIPS_MIPS16 (0xf0) as it interferes..."

grimar marked an inline comment as done.Dec 18 2019, 2:16 AM
grimar added inline comments.
llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test
5–6

I think it is important for GNU. As output shows, the Ndx column is misplaced. Here I document this behavior.
We might want to fix it. As far I can see we do not have a test that has sh_other > 0xF
and seems that is why this issue was never noticed. It is only a valid case for MIPS, hence I think it should be here.

For LLVM style perhaps we do not need --strict-whitespace, I've added it just in case and for consistency.

jhenderson added inline comments.Dec 18 2019, 2:18 AM
llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test
5–6

Okay.

grimar updated this revision to Diff 234508.Dec 18 2019, 4:25 AM
  • Addressed review comments.
This revision is now accepted and ready to land.Dec 18 2019, 5:24 AM
MaskRay accepted this revision.Dec 18 2019, 1:06 PM
MaskRay added inline comments.
llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test
23

UND looks misaligned.

MaskRay added inline comments.Dec 18 2019, 1:09 PM
llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test
5–6

I think it is important for GNU. As output shows, the Ndx column is misplaced. Here I document this behavior.

If the correct behavior seems straightforward, we can fix it and do not necessarily strictly follow GNU. I've filed bugs about GNU readelf display issues and I find they are willing to fix issues.

This revision was automatically updated to reflect the committed changes.