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

[mstorsjo]

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.

[mgrang]

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]

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

[mstorsjo]

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

[mstorsjo]

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.

[mgrang]

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

llvm/trunk/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

 #include "llvm/Support/ErrorHandling.h"
 using namespace llvm;
 
 namespace {
 
 class AArch64AsmBackend : public MCAsmBackend {
   static const unsigned PCRelFlagVal =
       MCFixupKindInfo::FKF_IsAlignedDownTo32Bits | MCFixupKindInfo::FKF_IsPCRel;
+  Triple TheTriple;
+
 public:
   bool IsLittleEndian;
 
 public:
-  AArch64AsmBackend(const Target &T, bool IsLittleEndian)
-     : MCAsmBackend(), IsLittleEndian(IsLittleEndian) {}
+  AArch64AsmBackend(const Target &T, const Triple &TT, bool IsLittleEndian)
+      : MCAsmBackend(), TheTriple(TT), IsLittleEndian(IsLittleEndian) {}
 
   unsigned getNumFixupKinds() const override {
     return AArch64::NumTargetFixupKinds;
   }
 
   const MCFixupKindInfo &getFixupKindInfo(MCFixupKind Kind) const override {
     const static MCFixupKindInfo Infos[AArch64::NumTargetFixupKinds] = {
         // This table *must* be in the order that the fixup_* kinds are defined

 
 static unsigned AdrImmBits(unsigned Value) {
   unsigned lo2 = Value & 0x3;
   unsigned hi19 = (Value & 0x1ffffc) >> 2;
   return (hi19 << 5) | (lo2 << 29);
 }
 
 static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
-                                 MCContext &Ctx) {
+                                 MCContext &Ctx, const Triple &TheTriple,
+                                 bool IsResolved) {
   unsigned Kind = Fixup.getKind();
   int64_t SignedValue = static_cast<int64_t>(Value);
   switch (Kind) {
   default:
     llvm_unreachable("Unknown fixup kind!");
   case AArch64::fixup_aarch64_pcrel_adr_imm21:
     if (SignedValue > 2097151 || SignedValue < -2097152)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     return AdrImmBits(Value & 0x1fffffULL);
   case AArch64::fixup_aarch64_pcrel_adrp_imm21:
+    assert(!IsResolved);
+    if (TheTriple.isOSBinFormatCOFF())
+      return AdrImmBits(Value & 0x1fffffULL);
     return AdrImmBits((Value & 0x1fffff000ULL) >> 12);
   case AArch64::fixup_aarch64_ldr_pcrel_imm19:
   case AArch64::fixup_aarch64_pcrel_branch19:
     // Signed 21-bit immediate
     if (SignedValue > 2097151 || SignedValue < -2097152)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     if (Value & 0x3)
       Ctx.reportError(Fixup.getLoc(), "fixup not sufficiently aligned");
     // Low two bits are not encoded.
     return (Value >> 2) & 0x7ffff;
   case AArch64::fixup_aarch64_add_imm12:
   case AArch64::fixup_aarch64_ldst_imm12_scale1:
+    if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
+      Value &= 0xfff;
     // Unsigned 12-bit immediate
     if (Value >= 0x1000)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     return Value;
   case AArch64::fixup_aarch64_ldst_imm12_scale2:
+    if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
+      Value &= 0xfff;
     // Unsigned 12-bit immediate which gets multiplied by 2
     if (Value >= 0x2000)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     if (Value & 0x1)
       Ctx.reportError(Fixup.getLoc(), "fixup must be 2-byte aligned");
     return Value >> 1;
   case AArch64::fixup_aarch64_ldst_imm12_scale4:
+    if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
+      Value &= 0xfff;
     // Unsigned 12-bit immediate which gets multiplied by 4
     if (Value >= 0x4000)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     if (Value & 0x3)
       Ctx.reportError(Fixup.getLoc(), "fixup must be 4-byte aligned");
     return Value >> 2;
   case AArch64::fixup_aarch64_ldst_imm12_scale8:
+    if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
+      Value &= 0xfff;
     // Unsigned 12-bit immediate which gets multiplied by 8
     if (Value >= 0x8000)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     if (Value & 0x7)
       Ctx.reportError(Fixup.getLoc(), "fixup must be 8-byte aligned");
     return Value >> 3;
   case AArch64::fixup_aarch64_ldst_imm12_scale16:
+    if (TheTriple.isOSBinFormatCOFF() && !IsResolved)
+      Value &= 0xfff;
     // Unsigned 12-bit immediate which gets multiplied by 16
     if (Value >= 0x10000)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     if (Value & 0xf)
       Ctx.reportError(Fixup.getLoc(), "fixup must be 16-byte aligned");
     return Value >> 4;
   case AArch64::fixup_aarch64_movw:
     Ctx.reportError(Fixup.getLoc(),
void AArch64AsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                                    MutableArrayRef<char> Data, uint64_t Value,
                                    bool IsResolved) const {
   unsigned NumBytes = getFixupKindNumBytes(Fixup.getKind());
   if (!Value)
     return; // Doesn't change encoding.
   MCFixupKindInfo Info = getFixupKindInfo(Fixup.getKind());
   MCContext &Ctx = Asm.getContext();
   // Apply any target-specific value adjustments.
-  Value = adjustFixupValue(Fixup, Value, Ctx);
+  Value = adjustFixupValue(Fixup, Value, Ctx, TheTriple, IsResolved);
 
   // Shift the value into position.
   Value <<= Info.TargetOffset;
 
   unsigned Offset = Fixup.getOffset();
   assert(Offset + NumBytes <= Data.size() && "Invalid fixup offset!");
 
   // Used to point to big endian bytes.
class DarwinAArch64AsmBackend : public AArch64AsmBackend {
   /// \brief Encode compact unwind stack adjustment for frameless functions.
   /// See UNWIND_ARM64_FRAMELESS_STACK_SIZE_MASK in compact_unwind_encoding.h.
   /// The stack size always needs to be 16 byte aligned.
   uint32_t encodeStackAdjustment(uint32_t StackSize) const {
     return (StackSize / 16) << 12;
   }
 
 public:
-  DarwinAArch64AsmBackend(const Target &T, const MCRegisterInfo &MRI)
-      : AArch64AsmBackend(T, /*IsLittleEndian*/true), MRI(MRI) {}
+  DarwinAArch64AsmBackend(const Target &T, const Triple &TT,
+                          const MCRegisterInfo &MRI)
+      : AArch64AsmBackend(T, TT, /*IsLittleEndian*/ true), MRI(MRI) {}
 
   MCObjectWriter *createObjectWriter(raw_pwrite_stream &OS) const override {
     return createAArch64MachObjectWriter(OS, MachO::CPU_TYPE_ARM64,
                                          MachO::CPU_SUBTYPE_ARM64_ALL);
   }
 
   /// \brief Generate the compact unwind encoding from the CFI directives.
   uint32_t generateCompactUnwindEncoding(

 
 namespace {
 
 class ELFAArch64AsmBackend : public AArch64AsmBackend {
 public:
   uint8_t OSABI;
   bool IsILP32;
 
-  ELFAArch64AsmBackend(const Target &T, uint8_t OSABI, bool IsLittleEndian,
-                       bool IsILP32)
-    : AArch64AsmBackend(T, IsLittleEndian), OSABI(OSABI), IsILP32(IsILP32) {}
+  ELFAArch64AsmBackend(const Target &T, const Triple &TT, uint8_t OSABI,
+                       bool IsLittleEndian, bool IsILP32)
+      : AArch64AsmBackend(T, TT, IsLittleEndian), OSABI(OSABI),
+        IsILP32(IsILP32) {}
 
   MCObjectWriter *createObjectWriter(raw_pwrite_stream &OS) const override {
     return createAArch64ELFObjectWriter(OS, OSABI, IsLittleEndian, IsILP32);
   }
 };
 
 }
 
 namespace {
 class COFFAArch64AsmBackend : public AArch64AsmBackend {
 public:
   COFFAArch64AsmBackend(const Target &T, const Triple &TheTriple)
-      : AArch64AsmBackend(T, /*IsLittleEndian*/true) {}
+      : AArch64AsmBackend(T, TheTriple, /*IsLittleEndian*/ true) {}
 
   MCObjectWriter *createObjectWriter(raw_pwrite_stream &OS) const override {
     return createAArch64WinCOFFObjectWriter(OS);
   }
 };
 }
 
 MCAsmBackend *llvm::createAArch64leAsmBackend(const Target &T,
                                               const MCRegisterInfo &MRI,
                                               const Triple &TheTriple,
                                               StringRef CPU,
                                               const MCTargetOptions &Options) {
   if (TheTriple.isOSBinFormatMachO())
-    return new DarwinAArch64AsmBackend(T, MRI);
+    return new DarwinAArch64AsmBackend(T, TheTriple, MRI);
 
   if (TheTriple.isOSBinFormatCOFF())
     return new COFFAArch64AsmBackend(T, TheTriple);
 
   assert(TheTriple.isOSBinFormatELF() && "Invalid target");
 
   uint8_t OSABI = MCELFObjectTargetWriter::getOSABI(TheTriple.getOS());
   bool IsILP32 = Options.getABIName() == "ilp32";
-  return new ELFAArch64AsmBackend(T, OSABI, /*IsLittleEndian=*/true, IsILP32);
+  return new ELFAArch64AsmBackend(T, TheTriple, OSABI, /*IsLittleEndian=*/true,
+                                  IsILP32);
 }
 
 MCAsmBackend *llvm::createAArch64beAsmBackend(const Target &T,
                                               const MCRegisterInfo &MRI,
                                               const Triple &TheTriple,
                                               StringRef CPU,
                                               const MCTargetOptions &Options) {
   assert(TheTriple.isOSBinFormatELF() &&
          "Big endian is only supported for ELF targets!");
   uint8_t OSABI = MCELFObjectTargetWriter::getOSABI(TheTriple.getOS());
   bool IsILP32 = Options.getABIName() == "ilp32";
-  return new ELFAArch64AsmBackend(T, OSABI, /*IsLittleEndian=*/false, IsILP32);
+  return new ELFAArch64AsmBackend(T, TheTriple, OSABI, /*IsLittleEndian=*/false,
+                                  IsILP32);
 }

llvm/trunk/test/MC/AArch64/coff-relocations.s

-; RUN: llvm-mc -triple aarch64-windows -filetype obj -o - %s | \
-; RUN: llvm-readobj -r - | FileCheck %s
+; RUN: llvm-mc -triple aarch64-windows -filetype obj -o %t.obj %s
+; RUN: llvm-readobj -r %t.obj | FileCheck %s
+; RUN: llvm-objdump -d %t.obj | FileCheck %s -check-prefix DISASM
 
 ; IMAGE_REL_ARM64_ADDR32
 .Linfo_foo:
   .asciz "foo"
   .long foo
 
 ; IMAGE_REL_ARM64_ADDR32NB
 .long func@IMGREL

 
 ; IMAGE_REL_ARM64_SECREL
 .secrel32 .Linfo_bar
 .Linfo_bar:
 
 ; IMAGE_REL_ARM64_SECTION
 .secidx func
 
+.align 2
+adrp x0, baz + 0x12345
+baz:
+add x0, x0, :lo12:foo + 0x12345
+ldrb w0, [x0, :lo12:foo + 0x12345]
+ldr x0, [x0, :lo12:foo + 0x12348]
+
 ; CHECK: Format: COFF-ARM64
 ; CHECK: Arch: aarch64
 ; CHECK: AddressSize: 64bit
 ; CHECK: Relocations [
 ; CHECK:   Section (1) .text {
 ; CHECK: 0x4 IMAGE_REL_ARM64_ADDR32 foo
 ; CHECK: 0x8 IMAGE_REL_ARM64_ADDR32NB func
 ; CHECK: 0xC IMAGE_REL_ARM64_ADDR64 arr
 ; CHECK: 0x14 IMAGE_REL_ARM64_BRANCH26 target
 ; CHECK: 0x18 IMAGE_REL_ARM64_PAGEBASE_REL21 foo
 ; CHECK: 0x1C IMAGE_REL_ARM64_PAGEOFFSET_12A foo
 ; CHECK: 0x20 IMAGE_REL_ARM64_PAGEOFFSET_12L foo
 ; CHECK: 0x24 IMAGE_REL_ARM64_PAGEBASE_REL21 bar
 ; CHECK: 0x28 IMAGE_REL_ARM64_SECREL .text
 ; CHECK: 0x2C IMAGE_REL_ARM64_SECTION func
+; CHECK: 0x30 IMAGE_REL_ARM64_PAGEBASE_REL21 baz
+; CHECK: 0x34 IMAGE_REL_ARM64_PAGEOFFSET_12A foo
+; CHECK: 0x38 IMAGE_REL_ARM64_PAGEOFFSET_12L foo
+; CHECK: 0x3C IMAGE_REL_ARM64_PAGEOFFSET_12L foo
 ; CHECK:   }
 ; CHECK: ]
+
+; DISASM: 30:       20 1a 09 b0     adrp    x0, #305418240
+; DISASM: 34:       00 14 0d 91     add     x0, x0, #837
+; DISASM: 38:       00 14 4d 39     ldrb    w0, [x0, #837]
+; DISASM: 3c:       00 a4 41 f9     ldr     x0, [x0, #840]

llvm/trunk/test/MC/AArch64/fixup-out-of-range.s

 // RUN: not llvm-mc -triple aarch64--none-eabi -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
+// RUN: not llvm-mc -triple aarch64-windows -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
 
 // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup value out of range
   adr x0, distant
 
 // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup value out of range
   ldr x0, distant
 
 // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup not sufficiently aligned