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

[MaskRay]

Summary

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)

[MaskRay]

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

[MaskRay]

Raise MaxTargetFixupKind to FirstLiteralRelocationKind + 1032

[MaskRay]

Delete a static local variable

[psmith]

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.

llvm/include/llvm/MC/MCFixup.h

59

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.

61

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]

llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

180

Deserves a comment I think.

llvm/test/MC/ARM/reloc-directive-err.s

1 ↗

(On Diff #252493)

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

[grimar]

llvm/test/MC/ARM/reloc-directive-err.s

1 ↗

(On Diff #252493)

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]

llvm/include/llvm/MC/MCFixup.h

61

Thanks for the wording suggestion!

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

llvm/test/MC/ARM/reloc-directive-err.s

1 ↗

(On Diff #252493)

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]

Improve comments

[psmith]

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

[grimar]

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

[MaskRay]

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

llvm/include/llvm/MC/MCFixup.h

enum MCFixupKind {
   FK_Data_Add_6b, ///< A six-bits add fixup.
   FK_Data_Sub_1,  ///< A one-byte sub fixup.
   FK_Data_Sub_2,  ///< A two-byte sub fixup.
   FK_Data_Sub_4,  ///< A four-byte sub fixup.
   FK_Data_Sub_8,  ///< A eight-byte sub fixup.
   FK_Data_Sub_6b, ///< A six-bits sub fixup.
 
   FirstTargetFixupKind = 128,
+  FirstLiteralRelocationKind = 256,
 
   // Limit range of target fixups, in case we want to pack more efficiently
   // later.
-  MaxTargetFixupKind = (1 << 8)
+  MaxTargetFixupKind = 512
 };
 
 /// Encode information on a single operation to perform on a byte
 /// sequence (e.g., an encoded instruction) which requires assemble- or run-
 /// time patching.
 ///
 /// Fixups are used any time the target instruction encoder needs to represent
 /// some value in an instruction which is not yet concrete. The encoder will

llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

 public:
   ARMELFObjectWriter(uint8_t OSABI)
       : MCELFObjectTargetWriter(/*Is64Bit*/ false, OSABI, ELF::EM_ARM,
                                 /*HasRelocationAddend*/ false) {}
 };
 } // end anonymous namespace
 
 Optional<MCFixupKind> ARMAsmBackend::getFixupKind(StringRef Name) const {
-  if (STI.getTargetTriple().isOSBinFormatELF() && Name == "R_ARM_NONE")
-    return FK_NONE;
+  if (!STI.getTargetTriple().isOSBinFormatELF())
+    return None;
 
-  return MCAsmBackend::getFixupKind(Name);
+  unsigned Type = llvm::StringSwitch<unsigned>(Name)
+#define ELF_RELOC(X, Y)  .Case(#X, Y)
+#include "llvm/BinaryFormat/ELFRelocs/ARM.def"
+#undef ELF_RELOC
+                      .Default(-1u);
+  return static_cast<MCFixupKind>(FirstLiteralRelocationKind + Type);
 }
 
 const MCFixupKindInfo &ARMAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
   unsigned IsPCRelConstant =
       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_Constant;
   const static MCFixupKindInfo InfosLE[ARM::NumTargetFixupKinds] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // ARMFixupKinds.h.
const static MCFixupKindInfo InfosBE[ARM::NumTargetFixupKinds] = {
       {"fixup_bf_target", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_bfl_target", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_bfc_target", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_bfcsel_else_target", 0, 32, 0},
       {"fixup_wls", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_le", 0, 32, MCFixupKindInfo::FKF_IsPCRel}
   };
 
+  static MCFixupKindInfo Empty;
+  if (Kind >= FirstLiteralRelocationKind)
+    return Empty;
   if (Kind < FirstTargetFixupKind)
     return MCAsmBackend::getFixupKindInfo(Kind);
 
   assert(unsigned(Kind - FirstTargetFixupKind) < getNumFixupKinds() &&
          "Invalid kind!");
   return (Endian == support::little ? InfosLE
                                     : InfosBE)[Kind - FirstTargetFixupKind];
 }
if (A->hasSubsectionsViaSymbols() && Asm.isThumbFunc(&A->getSymbol()) &&
          Kind == ARM::fixup_t2_movt_hi16))
       Value |= 1;
   }
 
   switch (Kind) {
   default:
     Ctx.reportError(Fixup.getLoc(), "bad relocation fixup type");
     return 0;
-  case FK_NONE:
   case FK_Data_1:
   case FK_Data_2:
   case FK_Data_4:
     return Value;
   case FK_SecRel_2:
     return Value;
   case FK_SecRel_4:
     return Value;

 }
 
 bool ARMAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
                                           const MCFixup &Fixup,
                                           const MCValue &Target) {
   const MCSymbolRefExpr *A = Target.getSymA();
   const MCSymbol *Sym = A ? &A->getSymbol() : nullptr;
   const unsigned FixupKind = Fixup.getKind();
-  if (FixupKind == FK_NONE)
+  if (FixupKind >= FirstLiteralRelocationKind)
     return true;
   if (FixupKind == ARM::fixup_arm_thumb_bl) {
     assert(Sym && "How did we resolve this?");
 
     // If the symbol is external the linker will handle it.
     // FIXME: Should we handle it as an optimization?
 
     // If the symbol is out of range, produce a relocation and hope the

 }
 
 /// getFixupKindNumBytes - The number of bytes the fixup may change.
 static unsigned getFixupKindNumBytes(unsigned Kind) {
   switch (Kind) {
   default:
     llvm_unreachable("Unknown fixup kind!");
 
-  case FK_NONE:
-    return 0;
-
   case FK_Data_1:
   case ARM::fixup_arm_thumb_bcc:
   case ARM::fixup_arm_thumb_cp:
   case ARM::fixup_thumb_adr_pcrel_10:
     return 1;
 
   case FK_Data_2:
   case ARM::fixup_arm_thumb_br:

 
 /// getFixupKindContainerSizeBytes - The number of bytes of the
 /// container involved in big endian.
 static unsigned getFixupKindContainerSizeBytes(unsigned Kind) {
   switch (Kind) {
   default:
     llvm_unreachable("Unknown fixup kind!");
 
-  case FK_NONE:
-    return 0;
-
   case FK_Data_1:
     return 1;
   case FK_Data_2:
     return 2;
   case FK_Data_4:
     return 4;
 
   case ARM::fixup_arm_thumb_bcc:
static unsigned getFixupKindContainerSizeBytes(unsigned Kind) {
   }
 }
 
 void ARMAsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                                const MCValue &Target,
                                MutableArrayRef<char> Data, uint64_t Value,
                                bool IsResolved,
                                const MCSubtargetInfo* STI) const {
-  unsigned NumBytes = getFixupKindNumBytes(Fixup.getKind());
+  unsigned Kind = Fixup.getKind();
+  if (Kind >= FirstLiteralRelocationKind)
+    return;
+  unsigned NumBytes = getFixupKindNumBytes(Kind);
   MCContext &Ctx = Asm.getContext();
   Value = adjustFixupValue(Asm, Fixup, Target, Value, IsResolved, Ctx, STI);
   if (!Value)
     return; // Doesn't change encoding.
 
   unsigned Offset = Fixup.getOffset();
   assert(Offset + NumBytes <= Data.size() && "Invalid fixup offset!");
 
   // Used to point to big endian bytes.
   unsigned FullSizeBytes;
   if (Endian == support::big) {
-    FullSizeBytes = getFixupKindContainerSizeBytes(Fixup.getKind());
+    FullSizeBytes = getFixupKindContainerSizeBytes(Kind);
     assert((Offset + FullSizeBytes) <= Data.size() && "Invalid fixup size!");
     assert(NumBytes <= FullSizeBytes && "Invalid fixup size!");
   }
 
   // For each byte of the fragment that the fixup touches, mask in the bits from
   // the fixup value. The Value has been "split up" into the appropriate
   // bitfields above.
   for (unsigned i = 0; i != NumBytes; ++i) {

llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp

unsigned ARMELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target,
                                           bool IsPCRel) const {
   return GetRelocTypeInner(Target, Fixup, IsPCRel, Ctx);
 }
 
 unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target,
                                                const MCFixup &Fixup,
                                                bool IsPCRel,
                                                MCContext &Ctx) const {
+  unsigned Kind = Fixup.getTargetKind();
+  if (Kind >= FirstLiteralRelocationKind)
+    return Kind - FirstLiteralRelocationKind;
   MCSymbolRefExpr::VariantKind Modifier = Target.getAccessVariant();
 
   if (IsPCRel) {
     switch (Fixup.getTargetKind()) {
     default:
       Ctx.reportFatalError(Fixup.getLoc(), "unsupported relocation on symbol");
       return ELF::R_ARM_NONE;
     case FK_Data_4:
if (IsPCRel) {
     case ARM::fixup_bf_target:
       return ELF::R_ARM_THM_BF16;
     case ARM::fixup_bfc_target:
       return ELF::R_ARM_THM_BF12;
     case ARM::fixup_bfl_target:
       return ELF::R_ARM_THM_BF18;
     }
   }
-  switch (Fixup.getTargetKind()) {
+  switch (Kind) {
   default:
     Ctx.reportFatalError(Fixup.getLoc(), "unsupported relocation on symbol");
     return ELF::R_ARM_NONE;
-  case FK_NONE:
-    return ELF::R_ARM_NONE;
   case FK_Data_1:
     switch (Modifier) {
     default:
       llvm_unreachable("unsupported Modifier");
     case MCSymbolRefExpr::VK_None:
       return ELF::R_ARM_ABS8;
     }
   case FK_Data_2:

llvm/test/MC/ARM/reloc-directive.s

 .text
   bx lr
   nop
   nop
   .reloc 8, R_ARM_NONE, .data
   .reloc 4, R_ARM_NONE, foo+4
   .reloc 0, R_ARM_NONE, 8
 
+  .reloc 0, R_ARM_ALU_PC_G0, .data+2
+  .reloc 0, R_ARM_LDR_PC_G0, foo+3
+  .reloc 0, R_ARM_THM_ALU_PREL_11_0, 5
+
 .data
 .globl foo
 foo:
   .word 0
   .word 0
   .word 0
 
 # PRINT: .reloc 8, R_ARM_NONE, .data
 # PRINT: .reloc 4, R_ARM_NONE, foo+4
 # PRINT: .reloc 0, R_ARM_NONE, 8
+# PRINT: .reloc 0, R_ARM_ALU_PC_G0, .data+2
+# PRINT: .reloc 0, R_ARM_LDR_PC_G0, foo+3
+# PRINT: .reloc 0, R_ARM_THM_ALU_PREL_11_0, 5
 
 # ARM relocations use the Elf32_Rel format. Addends are neither stored in the
 # relocation entries nor applied in the referenced locations.
 # CHECK:      0x8 R_ARM_NONE .data 0x0
 # CHECK-NEXT: 0x4 R_ARM_NONE foo 0x0
 # CHECK-NEXT: 0x0 R_ARM_NONE - 0x0
+# CHECK-NEXT: 0x0 R_ARM_ALU_PC_G0 .data 0x0
+# CHECK-NEXT: 0x0 R_ARM_LDR_PC_G0 foo 0x0
+# CHECK-NEXT: 0x0 R_ARM_THM_ALU_PREL_11_0 - 0x0
 
 # HEX: 0x00000000 00000000 00000000