This is an archive of the discontinued LLVM Phabricator instance.

[COFF, ARM64] Fix symbol offsets in ADRP/ADD/LDR/STR relocations
ClosedPublic

Authored by mstorsjo on Jul 24 2017, 4:07 AM.

Details

Summary

In COFF, a symbol offset can't be stored in the relocation (as is done in ELF or MachO), but is stored as the immediate in the instruction itself. The immediate in the ADRP thus is the symbol offset in bytes, not in pages. For the PAGEOFFSET_12A/L relocations, ignore any offset outside of the lowest 12 bits; they won't have any effect on the ADD/LDR/STR instruction itself but only on the associated ADRP.

This is similar to how the same issue is handled for MOVW/MOVT instructions in ELF (see e.g. SVN r307713, and r307728 in lld).

This fixes "fixup out of range" errors while building larger object files, where temporary symbols end up as a plain section symbol and an offset, and fixes any cases where the symbol offset mean that the actual target ended up on a different page than the symbol itself.

Diff Detail

Event Timeline

mstorsjo created this revision.Jul 24 2017, 4:07 AM
mgrang edited edge metadata.Jul 24 2017, 1:00 PM

LGTM. We recently encountered the "fixup out of range" for temporary symbols. We worked around this by turning off temporary symbols for COFF. Thanks for fixing this!

mstorsjo updated this revision to Diff 107950.Jul 24 2017, 1:13 PM

Added checking of the IsResolved variable, as suggested by Rafael.

mstorsjo updated this revision to Diff 108046.Jul 25 2017, 3:52 AM

Changed the previous check for !IsResolved into an assert, and added checks for it in the other fixups (where it is needed).

mstorsjo updated this revision to Diff 108137.Jul 25 2017, 12:54 PM

Added a testcase for IsResolved==true for COFF as well (repeating an existing test, targeting COFF).

Got LGTM's from Rafael and Mandeep so far, so will commit this one soon unless there's further comments.

I was able to build the entire spec2000 suite with this patch. So looks fine to me.

This revision was automatically updated to reflect the committed changes.