[llvm] 1e7f592 - [MC][COFF][AArch64] Avoid incorrect IMAGE_REL_ARM64_BRANCH26 relocations.

[Hiroshi Yamauchi via llvm-commits]

Author: Hiroshi Yamauchi
Date: 2023-08-17T14:31:31-07:00
New Revision: 1e7f592a890aad860605cf5220530b3744e107ba

URL: https://github.com/llvm/llvm-project/commit/1e7f592a890aad860605cf5220530b3744e107ba
DIFF: https://github.com/llvm/llvm-project/commit/1e7f592a890aad860605cf5220530b3744e107ba.diff

LOG: [MC][COFF][AArch64] Avoid incorrect IMAGE_REL_ARM64_BRANCH26 relocations.

For a b/bl instruction that branches a temporary symbol (private
assembly label), an IMAGE_REL_ARM64_BRANCH26 relocation to another
symbol + a non-zero offset could be emitted but the linkers don't
support this type of relocation and cause incorrect relocations and
crashes. Avoid emitting this type of relocations.

Differential Revision: https://reviews.llvm.org/D155732

Added: 
    llvm/test/MC/AArch64/coff-relocations-branch26.s

Modified: 
    llvm/lib/MC/WinCOFFObjectWriter.cpp
    llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp
index c203280d2c107e..fd8af1f8cdb8b2 100644
--- a/llvm/lib/MC/WinCOFFObjectWriter.cpp
+++ b/llvm/lib/MC/WinCOFFObjectWriter.cpp
@@ -847,7 +847,9 @@ void WinCOFFWriter::executePostLayoutBinding(MCAssembler &Asm,
 
   if (Mode != DwoOnly)
     for (const MCSymbol &Symbol : Asm.symbols())
-      if (!Symbol.isTemporary())
+      // Define non-temporary or temporary static (private-linkage) symbols
+      if (!Symbol.isTemporary() ||
+          cast<MCSymbolCOFF>(Symbol).getClass() == COFF::IMAGE_SYM_CLASS_STATIC)
         DefineSymbol(Symbol, Asm, Layout);
 }
 
@@ -909,7 +911,7 @@ void WinCOFFWriter::recordRelocation(MCAssembler &Asm,
   Reloc.Data.VirtualAddress = Layout.getFragmentOffset(Fragment);
 
   // Turn relocations for temporary symbols into section relocations.
-  if (A.isTemporary()) {
+  if (A.isTemporary() && !SymbolMap[&A]) {
     MCSection *TargetSection = &A.getSection();
     assert(
         SectionMap.contains(TargetSection) &&

diff  --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
index 59cf53e85c824a..b5d230668a5e9f 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
@@ -314,6 +314,13 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, const MCValue &Target,
     return (Value >> 2) & 0x3fff;
   case AArch64::fixup_aarch64_pcrel_branch26:
   case AArch64::fixup_aarch64_pcrel_call26:
+    if (TheTriple.isOSBinFormatCOFF() && !IsResolved && SignedValue != 0) {
+      // MSVC link.exe and lld do not support this relocation type
+      // with a non-zero offset
+      Ctx.reportError(Fixup.getLoc(),
+                      "cannot perform a PC-relative fixup with a non-zero "
+                      "symbol offset");
+    }
     // Signed 28-bit immediate
     if (!isInt<28>(SignedValue))
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");

diff  --git a/llvm/test/MC/AArch64/coff-relocations-branch26.s b/llvm/test/MC/AArch64/coff-relocations-branch26.s
new file mode 100644
index 00000000000000..4cd47032309cae
--- /dev/null
+++ b/llvm/test/MC/AArch64/coff-relocations-branch26.s
@@ -0,0 +1,75 @@
+// RUN: llvm-mc -triple aarch64-unknown-windows-msvc -filetype obj %s -o - | llvm-objdump -D -r - | FileCheck %s
+// RUN: not llvm-mc -triple aarch64-unknown-windows-msvc -filetype obj --defsym ERR=1 %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR
+
+    .text
+main:
+    nop
+    b .Ltarget
+    b .Lother_target
+
+// A privte label target in the same section
+    .def .Ltarget
+    .scl 3
+    .type 32
+    .endef
+    .p2align 2
+.Ltarget:
+    ret
+
+// A privte label target in another section
+    .section "other"
+    nop
+    nop
+    nop
+    nop
+    nop
+    nop
+    nop
+    nop
+    .def .Lother_target
+    .scl 3
+    .type 32
+    .endef
+    .p2align 2
+.Lother_target:
+    ret
+
+// Check that both branches have a relocation with a zero offset.
+//
+// CHECK: 0000000000000000 <main>:
+// CHECK:        0: d503201f      nop
+// CHECK:        4: 14000000      b       0x4 <main+0x4>
+// CHECK:                 0000000000000004:  IMAGE_REL_ARM64_BRANCH26     .Ltarget
+// CHECK:        8: 14000000      b       0x8 <main+0x8>
+// CHECK:                 0000000000000008:  IMAGE_REL_ARM64_BRANCH26     .Lother_target
+// CHECK: 000000000000000c <.Ltarget>:
+// CHECK:        c: d65f03c0      ret
+// CHECK: 0000000000000000 <other>:
+// CHECK:        0: d503201f      nop
+// CHECK:        4: d503201f      nop
+// CHECK:        8: d503201f      nop
+// CHECK:        c: d503201f      nop
+// CHECK:       10: d503201f      nop
+// CHECK:       14: d503201f      nop
+// CHECK:       18: d503201f      nop
+// CHECK:       1c: d503201f      nop
+// CHECK: 0000000000000020 <.Lother_target>:
+// CHECK:       20: d65f03c0      ret
+
+.ifdef ERR
+    .section "err"
+err:
+    nop
+    b .Lerr_target+4
+// ERR: [[#@LINE-1]]:5: error: cannot perform a PC-relative fixup with a non-zero symbol offset
+
+    .def .Lerr_target
+    .scl 3
+    .type 32
+    .p2align 2
+    .endef
+.Lerr_target:
+    nop
+    nop
+    ret
+.endif