[COFF, ARM64] Add initial relocation types

[compnerd]

Please add tests for the other relocation types.

[mgrang]

Added more relocations.
Could not find a test case for IMAGE_REL_ARM64_ADDR32NB.
I see that IMAGE_REL_ARM64_SECTION is generated only on Windows with dumpbin (and not on Linux with llvm-objdump). Does it make sense to limit this unit test only for Windows?

[rnk]

test/MC/AArch64/coff-relocations.ll

1 ↗

(On Diff #106788)

Use a .s test instead. You can probably run this file through llc, and then run llvm-mc on that to generate an object.

78 ↗

(On Diff #106788)

With a .s test, you won't need all this complex debug info to make SECREL and SECTION relocations, you can just directly use the .secrel and .secidx directives.

[mgrang]

@rnk Got it! Thanks. Will push an updated patch.

[mgrang]

Changed unit test to .s according to comments.
Unit tests for IMAGE_REL_ARM64_PAGEOFFSET_12A and IMAGE_REL_ARM64_PAGEOFFSET_12L not added as llvm-mc currently cannot parse the assembly for these. Here is my code for the above two relocations:

adrp x0, foo
add x0, x0, foo

This is the error I get:

error: expected compatible register, symbol or integer in range [0, 4095]
add x0, x0, foo
                  ^

[mstorsjo]

In D34857#810706, @mgrang wrote:

I see that IMAGE_REL_ARM64_SECTION is generated only on Windows with dumpbin (and not on Linux with llvm-objdump). Does it make sense to limit this unit test only for Windows?

I'd rather have a look at llvm-objdump and see if it can easily be fixed to show what you want.

In D34857#810748, @mgrang wrote:

Unit tests for IMAGE_REL_ARM64_PAGEOFFSET_12A and IMAGE_REL_ARM64_PAGEOFFSET_12L not added as llvm-mc currently cannot parse the assembly for these. Here is my code for the above two relocations:

adrp x0, foo
add x0, x0, foo

The correct syntax that llvm-mc should accept is this:

add x0, x0, :lo12:foo

IIRC this should also work with ldr/str:

ldr x0, [x0, :lo12:foo]

[mgrang]

Added test cases for all the implemented relocation types.

[mgrang]

In D34857#810769, @mstorsjo wrote:

In D34857#810706, @mgrang wrote:

I see that IMAGE_REL_ARM64_SECTION is generated only on Windows with dumpbin (and not on Linux with llvm-objdump). Does it make sense to limit this unit test only for Windows?

I'd rather have a look at llvm-objdump and see if it can easily be fixed to show what you want.

In D34857#810748, @mgrang wrote:

Unit tests for IMAGE_REL_ARM64_PAGEOFFSET_12A and IMAGE_REL_ARM64_PAGEOFFSET_12L not added as llvm-mc currently cannot parse the assembly for these. Here is my code for the above two relocations:

adrp x0, foo
add x0, x0, foo

The correct syntax that llvm-mc should accept is this:

add x0, x0, :lo12:foo

IIRC this should also work with ldr/str:

ldr x0, [x0, :lo12:foo]

Thanks @mstorsjo for pointing me to the correct assembly for add/ldr. My test cases now work for all the implemented relocation types.

[compnerd]

Please fix the relocation type before you commit this.

lib/Target/AArch64/MCTargetDesc/AArch64WinCOFFObjectWriter.cpp

50 ↗

(On Diff #106809)

Use auto and clang-format.

66 ↗

(On Diff #106809)

Shouldn't this be IMAGE_REL_ARM64_SECREL?

67 ↗

(On Diff #106809)

Why no newline here? Can you make the newline spacing uniform?

test/MC/AArch64/coff-relocations.s

2 ↗

(On Diff #106809)

Don't think that you need the second RUN: here.

[mgrang]

Addressed comments.

[mgrang]

I have addressed comments, and my previous patch was accepted by @compnerd. So unless someone has any more comments I will go ahead and commit this.

test/MC/AArch64/coff-relocations.s

2 ↗

(On Diff #106809)

Without the second RUN, this line is reported as malformed. I checked that a lot of the other tests also use a similar format of multiple RUNs.

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

default:
     llvm_unreachable("Unknown fixup kind!");
 
   case AArch64::fixup_aarch64_tlsdesc_call:
     return 0;
 
   case FK_Data_1:
     return 1;
 
-  case FK_Data_2:
   case AArch64::fixup_aarch64_movw:
+  case FK_Data_2:
+  case FK_SecRel_2:
     return 2;
 
   case AArch64::fixup_aarch64_pcrel_branch14:
   case AArch64::fixup_aarch64_add_imm12:
   case AArch64::fixup_aarch64_ldst_imm12_scale1:
   case AArch64::fixup_aarch64_ldst_imm12_scale2:
   case AArch64::fixup_aarch64_ldst_imm12_scale4:
   case AArch64::fixup_aarch64_ldst_imm12_scale8:
   case AArch64::fixup_aarch64_ldst_imm12_scale16:
   case AArch64::fixup_aarch64_ldr_pcrel_imm19:
   case AArch64::fixup_aarch64_pcrel_branch19:
     return 3;
 
   case AArch64::fixup_aarch64_pcrel_adr_imm21:
   case AArch64::fixup_aarch64_pcrel_adrp_imm21:
   case AArch64::fixup_aarch64_pcrel_branch26:
   case AArch64::fixup_aarch64_pcrel_call26:
   case FK_Data_4:
+  case FK_SecRel_4:
     return 4;
 
   case FK_Data_8:
     return 8;
   }
 }
 
 static unsigned AdrImmBits(unsigned Value) {
case AArch64::fixup_aarch64_pcrel_call26:
     // Low two bits are not encoded (4-byte alignment assumed).
     if (Value & 0x3)
       Ctx.reportError(Fixup.getLoc(), "fixup not sufficiently aligned");
     return (Value >> 2) & 0x3ffffff;
   case FK_Data_1:
   case FK_Data_2:
   case FK_Data_4:
   case FK_Data_8:
+  case FK_SecRel_2:
+  case FK_SecRel_4:
     return Value;
   }
 }
 
 /// getFixupKindContainereSizeInBytes - The number of bytes of the
 /// container involved in big endian or 0 if the item is little endian
 unsigned AArch64AsmBackend::getFixupKindContainereSizeInBytes(unsigned Kind) const {
   if (IsLittleEndian)

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

 
 using namespace llvm;
 
 namespace {
 
 class AArch64WinCOFFObjectWriter : public MCWinCOFFObjectTargetWriter {
 public:
   AArch64WinCOFFObjectWriter()
-    : MCWinCOFFObjectTargetWriter(COFF::IMAGE_FILE_MACHINE_ARM64) {
-  }
+      : MCWinCOFFObjectTargetWriter(COFF::IMAGE_FILE_MACHINE_ARM64) {}
 
   ~AArch64WinCOFFObjectWriter() override = default;
 
   unsigned getRelocType(MCContext &Ctx, const MCValue &Target,
                         const MCFixup &Fixup, bool IsCrossSection,
                         const MCAsmBackend &MAB) const override;
 
-   bool recordRelocation(const MCFixup &) const override;
 };
 
 } // end anonymous namespace
 
-unsigned
-AArch64WinCOFFObjectWriter::getRelocType(MCContext &Ctx,
-                                         const MCValue &Target,
-                                         const MCFixup &Fixup,
-                                         bool IsCrossSection,
-                                         const MCAsmBackend &MAB) const {
+unsigned AArch64WinCOFFObjectWriter::getRelocType(
+    MCContext &Ctx, const MCValue &Target, const MCFixup &Fixup,
+    bool IsCrossSection, const MCAsmBackend &MAB) const {
+  auto Modifier = Target.isAbsolute() ? MCSymbolRefExpr::VK_None
+                                      : Target.getSymA()->getKind();
+
+  switch (static_cast<unsigned>(Fixup.getKind())) {
+  default: {
     const MCFixupKindInfo &Info = MAB.getFixupKindInfo(Fixup.getKind());
     report_fatal_error(Twine("unsupported relocation type: ") + Info.Name);
   }
 
+  case FK_Data_4:
+    switch (Modifier) {
+    default:
+      return COFF::IMAGE_REL_ARM64_ADDR32;
+    case MCSymbolRefExpr::VK_COFF_IMGREL32:
+      return COFF::IMAGE_REL_ARM64_ADDR32NB;
+    case MCSymbolRefExpr::VK_SECREL:
+      return COFF::IMAGE_REL_ARM64_SECREL;
+    }
+
+  case FK_Data_8:
+    return COFF::IMAGE_REL_ARM64_ADDR64;
+
+  case FK_SecRel_2:
+    return COFF::IMAGE_REL_ARM64_SECTION;
+
+  case FK_SecRel_4:
+    return COFF::IMAGE_REL_ARM64_SECREL;
+
+  case AArch64::fixup_aarch64_add_imm12:
+    return COFF::IMAGE_REL_ARM64_PAGEOFFSET_12A;
+
+  case AArch64::fixup_aarch64_ldst_imm12_scale1:
+  case AArch64::fixup_aarch64_ldst_imm12_scale2:
+  case AArch64::fixup_aarch64_ldst_imm12_scale4:
+  case AArch64::fixup_aarch64_ldst_imm12_scale8:
+  case AArch64::fixup_aarch64_ldst_imm12_scale16:
+    return COFF::IMAGE_REL_ARM64_PAGEOFFSET_12L;
+
+  case AArch64::fixup_aarch64_pcrel_adrp_imm21:
+    return COFF::IMAGE_REL_ARM64_PAGEBASE_REL21;
+
+  case AArch64::fixup_aarch64_pcrel_branch26:
+  case AArch64::fixup_aarch64_pcrel_call26:
+    return COFF::IMAGE_REL_ARM64_BRANCH26;
+  }
+}
+
 bool AArch64WinCOFFObjectWriter::recordRelocation(const MCFixup &Fixup) const {
   return true;
 }
 
 namespace llvm {
 
 MCObjectWriter *createAArch64WinCOFFObjectWriter(raw_pwrite_stream &OS) {
   MCWinCOFFObjectTargetWriter *MOTW = new AArch64WinCOFFObjectWriter();
   return createWinCOFFObjectWriter(MOTW, OS);
 }
 
 } // end namespace llvm

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

+; RUN: llvm-mc -triple aarch64-windows -filetype obj -o - %s | \
+; RUN: llvm-readobj -r - | FileCheck %s
+
+; IMAGE_REL_ARM64_ADDR32
+.Linfo_foo:
+  .asciz "foo"
+  .long foo
+
+; IMAGE_REL_ARM64_ADDR32NB
+.long func@IMGREL
+
+; IMAGE_REL_ARM64_ADDR64
+.globl struc
+struc:
+  .quad arr
+
+; IMAGE_REL_ARM64_BRANCH26
+b target
+
+; IMAGE_REL_ARM64_PAGEBASE_REL21
+adrp x0, foo
+
+; IMAGE_REL_ARM64_PAGEOFFSET_12A
+add x0, x0, :lo12:foo
+
+; IMAGE_REL_ARM64_PAGEOFFSET_12L
+ldr x0, [x0, :lo12:foo]
+
+; IMAGE_REL_ARM64_SECREL
+.secrel32 .Linfo_bar
+.Linfo_bar:
+
+; IMAGE_REL_ARM64_SECTION
+.secidx func
+
+
+; 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_SECREL .text
+; CHECK: 0x28 IMAGE_REL_ARM64_SECTION func
+; CHECK:   }
+; CHECK: ]