This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF] - Update and fix gnu-ifunc* tests.
ClosedPublic

Authored by grimar on Aug 10 2020, 5:33 AM.

Details

Summary

It turns that gnu-ifunc-plt-i386.s and gnu-ifunc-plt.s tests are broken.

Initially they were implemented in D27581 and tested that IRELATIVE relocations
are placed after other relocations in .rel.plt.

Later, we started to place IRELATIVE relocations to .rela.dyn (D65651).

Also, at some point .plt was renamed to .iplt (D71520).

Now, gnu-ifunc* tests mentioned do not test what they intended to test initially:
they should test that IRELATIVE relocations are placed after other ones in
.rela.dyn. Also, comments needs to be updated accordingly after changes performed.

This patch updates them.

Diff Detail

Event Timeline

grimar created this revision.Aug 10 2020, 5:33 AM
grimar requested review of this revision.Aug 10 2020, 5:33 AM
MaskRay accepted this revision.Aug 10 2020, 4:11 PM

LGTM.

lld/test/ELF/gnu-ifunc-plt-i386.s
15–16

Perhaps switch to /// while you are updating the comment.

45–49

<zed2+0x401230> does not make much sense. Consider removing them when you are updating them. Next time llvm-objdump -d heuristics change for the target symbol, we won't need to fix them.

lld/test/ELF/gnu-ifunc-plt.s
61–63

<zed2+0x2034a0> does not make much sense. Consider removing it.

This revision is now accepted and ready to land.Aug 10 2020, 4:11 PM
MaskRay added inline comments.Aug 10 2020, 4:13 PM
lld/test/ELF/gnu-ifunc-plt-i386.s
54–55

Consider llvm-objdump --print-imm-hex if you are going to update nearly every address.

grimar marked 4 inline comments as done.Aug 11 2020, 5:14 AM
grimar added inline comments.
lld/test/ELF/gnu-ifunc-plt-i386.s
54–55

Yep, that makes the output a bit better, thanks!

This revision was automatically updated to reflect the committed changes.
grimar marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptAug 11 2020, 5:15 AM