This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Add test for a variety of arm64 relocations
ClosedPublic

Authored by int3 on Feb 19 2021, 7:11 PM.

Details

Reviewers
smeenai
Group Reviewers
Restricted Project
Commits
rG8da88d4b605b: [lld-macho] Add test for a variety of arm64 relocations

Diff Detail

Event Timeline

int3 created this revision.Feb 19 2021, 7:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2021, 7:11 PM
int3 requested review of this revision.Feb 19 2021, 7:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2021, 7:11 PM
alexander-shaposhnikov added inline comments.
lld/test/MachO/arm64-relocs.s
4

wouldn't llvm-objdump --syms --macho -d --section=__const %t be sufficient ? (+ reordering the checks)

int3 added inline comments.Feb 23 2021, 12:57 PM
lld/test/MachO/arm64-relocs.s
4

I find it more natural to have the symbol table first, since everything is defined exactly once there. If OTOH the __const data was first, then we would have the first matching line defining BAZ and the second line checking that the value matches, which seems kind of misleading when we're really matching on the same output

19–20

(the comment above was talking about these two lines in particular)

smeenai accepted this revision.Feb 26 2021, 4:33 PM
smeenai added a subscriber: smeenai.

LGTM

lld/test/MachO/arm64-relocs.s
13

I imagine it's changing from 123 to 120 because the instruction (and presumably the relocation type) can only encode 4-byte aligned addresses, but can you add a comment explaining that?

14

Same here.

45

Do local symbols (as in symbols prefixed with L) do the trick?

This revision is now accepted and ready to land.Feb 26 2021, 4:33 PM
int3 updated this revision to Diff 326870.Feb 26 2021, 7:07 PM
int3 marked 2 inline comments as done.

address comments

lld/test/MachO/arm64-relocs.s
45

Nope. From further investigation, it seems that UNSIGNED section relocations are only generated for compact unwind sections in arm64. So to test that case, I'll extend the compact unwind test to cover arm64.

This revision was landed with ongoing or failed builds.Feb 27 2021, 9:32 AM
This revision was automatically updated to reflect the committed changes.