Page MenuHomePhabricator

[test/Object] - Move/rewrite 2 more test cases.

Authored by grimar on Aug 15 2019, 6:27 AM.



This patch makes a change for test/Object tests responsible
for relocations.

  1. 2 tests were moved to llvm-readobj/llvm-objdump folders:

Object/elf-reloc-no-sym.test -> tools/llvm-readobj/elf-reloc-no-sym.test
Object/objdump-reloc-shared.test -> tools/llvm-objdump/relocations-in-nonreloc.test

  1. A prerecompiled binary was removed and these tests were refactored.

Diff Detail

Event Timeline

grimar created this revision.Aug 15 2019, 6:27 AM
MaskRay accepted this revision.Aug 15 2019, 7:38 AM
MaskRay added inline comments.
29 ↗(On Diff #215382)

Nit: non-zero r_offset & r_addend for IRELATIVE will be more realistic. At runtime, image_base + r_addend will be called as a function. r_addend should be non-zero.

60 ↗(On Diff #215382)

Nit: assemblers do not produce R_X86_64_IRELATIVE (i.e. they should not appear in object files). You can use a different one, e.g. PLT32. (I believe IRELATIVE in .rela.plt is a historical GNU mistake, see D65651)

This revision is now accepted and ready to land.Aug 15 2019, 7:38 AM
jhenderson added inline comments.Aug 15 2019, 9:08 AM
25 ↗(On Diff #215382)

I don't think SHF_INFO_LINK is needed in any of these inputs.

29 ↗(On Diff #215382)

I don't think we need this test to be particularly realistic. It just needs to have a relocation in it (I say R_X86_64_NONE to show it's suitably arbitrary), and show whether or not that relocation is printed.

19 ↗(On Diff #215382)

Entry address isn't significant to this test.

23 ↗(On Diff #215382)

I don't think SHF_INFO_LINK is needed.

25–27 ↗(On Diff #215382)

I'd just simplify this. The numbers look like they're significant in some way, when really they can just be arbitrary (e.g. 0x12345678 would be more clearly arbitrary, I think).

MaskRay added inline comments.Aug 15 2019, 9:25 AM
29 ↗(On Diff #215382)

R_X86_64_NONE is not arbitrary but it will justify arbitrary r_offset/r_addend (so you can even omit r_addend)

grimar updated this revision to Diff 215541.Aug 16 2019, 1:51 AM
grimar marked 10 inline comments as done.
  • Addressed review comments.
25 ↗(On Diff #215382)

Technically SHF_ALLOC or any other flags are also not needed.
I removed them.

29 ↗(On Diff #215382)

I used R_X86_64_NONE.

jhenderson accepted this revision.Mon, Aug 19, 1:56 AM


25 ↗(On Diff #215541)

Maybe worth making this R_X86_64_NONE too? Doesn't really matter though.

MaskRay added inline comments.Mon, Aug 19, 3:06 AM
25 ↗(On Diff #215541)

Using different relocation types (not all R_*_NONE) adds some variety..

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptTue, Aug 20, 1:24 AM