[ARM] [COFF] Properly produce cross-section relative relocations

[mstorsjo]

Summary

Same as D99572 but for ARM.

[compnerd]

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

63

I'm not sure I understand entirely how this works. If this is used in a data only table, the data is now PC-relative. Is that really the same thing? Everything on COFF is PC-relative if it is base relocated, but that requires generation in the frontend to account for that, which understands the semantics for the data better than we do at this point.

[mstorsjo]

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

63

If it's a table of plain addresses, not address differences, I think IsCrossSection isn't set. The coff-relocations.s testcase has a check for it, see around the addr32 label.

As for who makes sure the offset is adjusted to be PC relative (when the subtracted bit in the asm expression isn't the PC but some other address), not entirely sure, but apparently it's done by some other MC layer. This corresponds pretty much 1:1 to the same in the X86 target, see https://github.com/llvm/llvm-project/blob/llvmorg-11.1.0/llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFObjectWriter.cpp#L46-L52.

[mstorsjo]

Ping

[rnk]

Approving to unblock.

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

 //===-- ARMWinCOFFObjectWriter.cpp - ARM Windows COFF Object Writer -- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/ARMFixupKinds.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/MC/MCAsmBackend.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCFixup.h"
 #include "llvm/MC/MCFixupKindInfo.h"
 #include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/MC/MCWinCOFFObjectWriter.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
unsigned ARMWinCOFFObjectWriter::getRelocType(MCContext &Ctx,
                                               bool IsCrossSection,
                                               const MCAsmBackend &MAB) const {
   assert(getMachine() == COFF::IMAGE_FILE_MACHINE_ARMNT &&
          "AArch64 support not yet implemented");
 
   MCSymbolRefExpr::VariantKind Modifier =
     Target.isAbsolute() ? MCSymbolRefExpr::VK_None : Target.getSymA()->getKind();
 
-  switch (static_cast<unsigned>(Fixup.getKind())) {
+  unsigned FixupKind = Fixup.getKind();
+  if (IsCrossSection) {
+    if (FixupKind != FK_Data_4) {
+      Ctx.reportError(Fixup.getLoc(), "Cannot represent this expression");
+      return COFF::IMAGE_REL_ARM_ADDR32;
+    }
+    FixupKind = FK_PCRel_4;
+  }
+
+
+  switch (FixupKind) {
   default: {
     const MCFixupKindInfo &Info = MAB.getFixupKindInfo(Fixup.getKind());
     report_fatal_error(Twine("unsupported relocation type: ") + Info.Name);
   }
   case FK_Data_4:
     switch (Modifier) {
     case MCSymbolRefExpr::VK_COFF_IMGREL32:
       return COFF::IMAGE_REL_ARM_ADDR32NB;
     case MCSymbolRefExpr::VK_SECREL:
       return COFF::IMAGE_REL_ARM_SECREL;
     default:
       return COFF::IMAGE_REL_ARM_ADDR32;
     }
+  case FK_PCRel_4:
+    return COFF::IMAGE_REL_ARM_REL32;
   case FK_SecRel_2:
     return COFF::IMAGE_REL_ARM_SECTION;
   case FK_SecRel_4:
     return COFF::IMAGE_REL_ARM_SECREL;
   case ARM::fixup_t2_condbranch:
     return COFF::IMAGE_REL_ARM_BRANCH20T;
   case ARM::fixup_t2_uncondbranch:
   case ARM::fixup_arm_thumb_bl:

llvm/test/MC/ARM/coff-relocations.s

 @ RUN: llvm-mc -triple thumbv7-windows-itanium -filetype obj -o - %s \
 @ RUN:   | llvm-readobj -r - | FileCheck %s -check-prefix CHECK-RELOCATION
 
 @ RUN: llvm-mc -triple thumbv7-windows-itanium -filetype obj -o - %s \
 @ RUN:   | llvm-objdump -d - | FileCheck %s --check-prefix=CHECK-ENCODING
 
+@ RUN: llvm-mc -triple thumbv7-windows-itanium -filetype obj -o - %s \
+@ RUN:   | llvm-objdump -s - | FileCheck %s --check-prefix=CHECK-DATA
+
   .syntax unified
   .text
   .thumb
 
   .global target
 
   .thumb_func
 branch24t_0:
@ CHECK-ENCODING-NEXT: movs r0, r0
        .thumb_func
 secrel:
   ldr r0, .Lsecrel
   bx r0
   trap
 .Lsecrel:
   .long target(secrel32)
 
+  .section .rdata, "dr"
+.Ltable:
+  .word addr32nb - .Ltable
+  .word secrel - .Ltable
+
 @ CHECK-ENCODING-LABEL: <secrel>:
 @ CHECK-ENCODING-NEXT: ldr.w r0, [pc, #4]
 @ CHECK-ENCODING-NEXT: bx r0
 @ CHECK-ENCODING-NEXT: trap
 @ CHECK-ENCODING-NEXT: movs r0, r0
 @ CHECK-ENCODING-NEXT: movs r0, r0
 
 @ CHECK-RELOCATION: Relocations [
 @ CHECK-RELOCATION:   Section (1) .text {
 @ CHECK-RELOCATION:     0x0 IMAGE_REL_ARM_BRANCH24T
 @ CHECK-RELOCATION:     0x4 IMAGE_REL_ARM_BRANCH24T
 @ CHECK-RELOCATION:     0x8 IMAGE_REL_ARM_BRANCH20T
 @ CHECK-RELOCATION:     0xC IMAGE_REL_ARM_BLX23T
 @ CHECK-RELOCATION:     0x10 IMAGE_REL_ARM_MOV32T
 @ CHECK-RELOCATION:     0x20 IMAGE_REL_ARM_ADDR32
 @ CHECK-RELOCATION:     0x2C IMAGE_REL_ARM_ADDR32NB
 @ CHECK-RELOCATION:     0x38 IMAGE_REL_ARM_SECREL
 @ CHECK-RELOCATION:   }
+@ CHECK-RELOCATION:   Section (4) .rdata {
+@ CHECK-RELOCATION:     0x0 IMAGE_REL_ARM_REL32
+@ CHECK-RELOCATION:     0x4 IMAGE_REL_ARM_REL32
+@ CHECK-RELOCATION:   }
 @ CHECK-RELOCATION: ]
 
+@ CHECK-DATA: Contents of section .rdata:
+@ CHECK-DATA-NEXT: 0000 00000000 04000000