[ARM] Support .reloc *, R_ARM_NONE, *

[MaskRay]

Summary

R_ARM_NONE can be used to create references among sections. When
--gc-sections is used, the referenced section will be retained if the
origin section is retained.

Add a generic MCFixupKind FK_NONE as this kind of no-op relocation is
ubiquitous on ELF and COFF, and probably available on many other binary
formats. See D62014.

[MaskRay]

Delete the change to getFixupKindContainerSizeBytes()

[MaskRay]

Set shouldForceRelocation to allow 0 symbol index, e.g. .reloc 0, R_ARM_NONE, 8

[peter.smith]

I get an assertion failure from this patch on the armv7eb-linux-gnueabi test case: llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp:872! (the llvm_unreachable)

/// 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_Data_1:
    return 1;
  case FK_Data_2:
    return 2;
  case FK_Data_4:
    return 4;

Looks like you'll need to add case ARM::fixup_arm_NONE:

[MaskRay]

Add back getFixupKindContainerSizeBytes() change

[peter.smith]

I think this is looking close. I've got a small concern about what happens on non-ELF platforms Windows and Macho where R_ARM_NONE is not defined. Other than that it is just a few suggestions.

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

50

Is there a way of making R_ARM_NONE only accepted on ELF. There are also Windows (COFF) and MachO users that may come through this code-path, we need to make sure that they don't get a fixup they can't handle.

test/MC/ARM/reloc-directive.s

9

Would it be possible to expand the test case to include something like:

        .text
        .globl foo
        .type foo, %function
foo:    bx lr

        .data
        .reloc 0, R_ARM_NONE, foo

As this will more closely match the real world usage for .reloc.

It may also be worth a test case that doesn't use the filetype=obj to make sure the asmprinter code path works.

18

I suggest adding the reason they are ignored. Something like # Addends are ignored for SHT_REL relocations with no addend field.

[MaskRay]

Improve tests as peter.smith suggested

[MaskRay]

Change to if (STI.getTargetTriple().isOSBinFormatELF() && Name == "R_ARM_NONE")

[MaskRay]

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

50

Good point! Added a check STI.getTargetTriple().isOSBinFormatELF()

% llvm-mc -filetype=obj -triple=armv7a-windows-itanium reloc-directive.s -o t.o
reloc-directive.s:17:13: error: unknown relocation name
  .reloc 4, R_ARM_NONE, foo+4

% llvm-mc -filetype=obj -triple=armv7-apple-macho reloc-directive.s -o t.o
reloc-directive.s:17:13: error: unknown relocation name
  .reloc 4, R_ARM_NONE, foo+4

Before, the diagnostic for coff is:

LLVM ERROR: unsupported relocation type: fixup_arm_NONE

macho:

reloc-directive.s:17:3: error: unsupported relocation on symbol                                                 
  .reloc 4, R_ARM_NONE, foo+4

I think X86 and MIPS need a similar fix..

[peter.smith]

Thanks, this looks good to me. May be worth waiting before committing to see if there are any comments from the US timezone.

[MaskRay]

Add generic FK_NONE as suggested by pcc in the R_{386,X86_64}_NONE patch

[MaskRay]

Fix a typo in the test

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h

public:
   unsigned getNumFixupKinds() const override {
     return ARM::NumTargetFixupKinds;
   }
 
   // FIXME: this should be calculated per fragment as the STI may be
   // different.
   bool hasNOP() const { return STI.getFeatureBits()[ARM::HasV6T2Ops]; }
 
+  Optional<MCFixupKind> getFixupKind(StringRef Name) const override;
+
   const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override;
 
   bool shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup,
                              const MCValue &Target) override;
 
   unsigned adjustFixupValue(const MCAssembler &Asm, const MCFixup &Fixup,
                             const MCValue &Target, uint64_t Value,
                             bool IsResolved, MCContext &Ctx,

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

 class ARMELFObjectWriter : public MCELFObjectTargetWriter {
 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 (Name == "R_ARM_NONE")
+    return (MCFixupKind)ARM::fixup_arm_NONE;
+  return MCAsmBackend::getFixupKind(Name);
+}
+
 const MCFixupKindInfo &ARMAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
   const static MCFixupKindInfo InfosLE[ARM::NumTargetFixupKinds] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // ARMFixupKinds.h.
       //
       // Name                      Offset (bits) Size (bits)     Flags
+      {"fixup_arm_NONE", 0, 0, 0},
       {"fixup_arm_ldst_pcrel_12", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_ldst_pcrel_12", 0, 32,
        MCFixupKindInfo::FKF_IsPCRel |
            MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
       {"fixup_arm_pcrel_10_unscaled", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_arm_pcrel_10", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_pcrel_10", 0, 32,
        MCFixupKindInfo::FKF_IsPCRel |
const static MCFixupKindInfo InfosLE[ARM::NumTargetFixupKinds] = {
       {"fixup_arm_mod_imm", 0, 12, 0},
       {"fixup_t2_so_imm", 0, 26, 0},
   };
   const static MCFixupKindInfo InfosBE[ARM::NumTargetFixupKinds] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // ARMFixupKinds.h.
       //
       // Name                      Offset (bits) Size (bits)     Flags
+      {"fixup_arm_NONE", 0, 0, 0},
       {"fixup_arm_ldst_pcrel_12", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_ldst_pcrel_12", 0, 32,
        MCFixupKindInfo::FKF_IsPCRel |
            MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
       {"fixup_arm_pcrel_10_unscaled", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_arm_pcrel_10", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_pcrel_10", 0, 32,
        MCFixupKindInfo::FKF_IsPCRel |
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 ARM::fixup_arm_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;

 }
 
 /// getFixupKindNumBytes - The number of bytes the fixup may change.
 static unsigned getFixupKindNumBytes(unsigned Kind) {
   switch (Kind) {
   default:
     llvm_unreachable("Unknown fixup kind!");
 
+  case ARM::fixup_arm_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 ARM::fixup_arm_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:

lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp

case MCSymbolRefExpr::VK_TLSCALL:
       return ELF::R_ARM_TLS_CALL;
     case MCSymbolRefExpr::VK_TLSDESC:
       return ELF::R_ARM_TLS_GOTDESC;
     case MCSymbolRefExpr::VK_TLSLDM:
       return ELF::R_ARM_TLS_LDM32;
     case MCSymbolRefExpr::VK_ARM_TLSDESCSEQ:
       return ELF::R_ARM_TLS_DESCSEQ;
     }
+  case ARM::fixup_arm_NONE:
+    return ELF::R_ARM_NONE;
   case ARM::fixup_arm_condbranch:
   case ARM::fixup_arm_uncondbranch:
     return ELF::R_ARM_JUMP24;
   case ARM::fixup_arm_movt_hi16:
     switch (Modifier) {
     default:
       llvm_unreachable("Unsupported Modifier");
     case MCSymbolRefExpr::VK_None:

lib/Target/ARM/MCTargetDesc/ARMFixupKinds.h

 //===-- ARMFixupKinds.h - ARM Specific Fixup Entries ------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_ARM_MCTARGETDESC_ARMFIXUPKINDS_H
 #define LLVM_LIB_TARGET_ARM_MCTARGETDESC_ARMFIXUPKINDS_H
 
 #include "llvm/MC/MCFixup.h"
 
 namespace llvm {
 namespace ARM {
 enum Fixups {
+  // Fixup resulting in R_ARM_NONE.
+  fixup_arm_NONE = FirstTargetFixupKind,
+
   // 12-bit PC relative relocation for symbol addresses
-  fixup_arm_ldst_pcrel_12 = FirstTargetFixupKind,
+  fixup_arm_ldst_pcrel_12,
 
   // Equivalent to fixup_arm_ldst_pcrel_12, with the 16-bit halfwords reordered.
   fixup_t2_ldst_pcrel_12,
 
   // 10-bit PC relative relocation for symbol addresses used in
   // LDRD/LDRH/LDRB/etc. instructions. All bits are encoded.
   fixup_arm_pcrel_10_unscaled,
   // 10-bit PC relative relocation for symbol addresses used in VFP instructions

test/MC/ARM/reloc-directive.s

+# RUN: llvm-mc -filetype=obj -triple=armv7-linux-gnueabi %s -o %t
+# RUN: llvm-readobj -r %t | FileCheck %s
+# RUN: llvm-readelf -x .data %t | FileCheck --check-prefix=HEX %s
+
+# RUN: llvm-mc -filetype=obj -triple=armv7eb-linux-gnueabi %s -o %t
+# RUN: llvm-readobj -r %t | FileCheck %s
+# RUN: llvm-readelf -x .data %t | FileCheck --check-prefix=HEX %s
+
+.data
+  .reloc 4, R_ARM_NONE, .data
+  .reloc 0, R_ARM_NONE, .data+4
+  .space 8
+
+# Addends are ignored.
+# CHECK:      0x4 R_ARM_NONE .data 0x0
+# CHECK-NEXT: 0x0 R_ARM_NONE .data 0x0
+
+# HEX: 0x00000000 00000000 00000000