This is an archive of the discontinued LLVM Phabricator instance.

[MC][ARM] Make .reloc support arbitrary relocation types

Authored by MaskRay on Mar 24 2020, 7:33 PM.



Generalizes D61992. In GNU as, the .reloc directive supports arbitrary relocation types.

A MCFixupKind value V larger than or equal to FirstLiteralRelocationKind
is used to represent the relocation type whose number is V-FirstLiteralRelocationKind.

This is useful for linker tests. Without the feature the assembler
cannot produce certain relocation records (e.g. R_ARM_ALU_PC_G0/R_ARM_LDR_PC_G0)

Diff Detail

Event Timeline

MaskRay created this revision.Mar 24 2020, 7:33 PM
MaskRay updated this revision to Diff 252481.Mar 24 2020, 7:43 PM

Don't crash when an invalid relocation is seen. Add reloc-directive-err.s

MaskRay updated this revision to Diff 252491.Mar 24 2020, 8:54 PM

Raise MaxTargetFixupKind to FirstLiteralRelocationKind + 1032

MaskRay updated this revision to Diff 252493.Mar 24 2020, 9:01 PM

Delete a static local variable

I think this could work, I've got a few comments on the generic part, but otherwise looks reasonable to me. Would be nice to extend this to AArch64 as there are some relocs that GCC/GNU as can emit that llvm-mc does not and it would be good to support these in LLVM.


It could be worth making it explicit that the range delimited by LiteralRelocationKind to MaxFixupKind (see next comment) is used for relocations coming from .reloc directive.


I recommend we rename MaxTargetFixupKind to MaxFixupKind as MaxTargetFixupKind is now 255

I'm guessing R_AARCH64_IRELATIVE is the largest reloc type in use throughout the targets? For a 64-bit RELA relocation this could be any 32-bit value. I think it will be worth giving a little room for aarch64 to expand and explaining what the limit is. For example:

Set limit to accommodate the highest reloc type in use for all Targets, currently R_AARCH64_IRELATIVE at 1032, including room for expansion.
MaxFixupKind = FirstLiteralRelocationKind + 1032 + 32,
grimar added inline comments.Mar 25 2020, 2:29 AM

Deserves a comment I think.


Should this test be MC/ARM and use -triple=armv7? Could it be a generic test?

grimar added inline comments.Mar 25 2020, 2:34 AM

I mean R_INVALID looks like it is a generic invalid relocation and I'd expect to have a generic test for "unknown relocation name" message.
I am not an expert in llvm-mc though, perhaps I am missing something.

MaskRay marked 6 inline comments as done.Mar 25 2020, 11:00 AM
MaskRay added inline comments.

Thanks for the wording suggestion!

MaxFixupKind is only used once in an assertion to forbid apparently erroneous fixup kinds.


This test corresponds to the -1u logic in ARMAsmBackend::getFixupKind. Sadly it is target specific and we need to duplicate the code for each target.

R_INVALID is just an arbitrary name not used as a relocation name (R_ARM_*). It can be anything. I am open to any suggestion if it can make the intention clear.

A llvm-mc test has to be target specific. For general ones we can create tests under test/MC/ or test/MC/ELF/

MaskRay updated this revision to Diff 252628.Mar 25 2020, 11:00 AM
MaskRay marked 6 inline comments as done.

Improve comments

Thanks for the update. I'm happy with the new comments.

I have no more comments either, but I do not feel I am a right person to accept.

Assuming Peter has accepted:) Will commit after local testing.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 27 2020, 12:36 PM
This revision was automatically updated to reflect the committed changes.