From 9f236eaf0bfb9190e1daf0c265579e627f90352e Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Thu, 17 Aug 2023 14:42:01 -0700 Subject: [PATCH 1/2] [MC][COFF][AArch64] Fix the storage class for private linkage symbols. Use IMAGE_SYM_CLASS_STATIC like X86. Differential Revision: https://reviews.llvm.org/D158122 --- llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 6 +++--- .../CodeGen/AArch64/local-sym-storage-class.ll | 15 +++++++++++++++ llvm/test/CodeGen/X86/local-sym-storage-class.ll | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/local-sym-storage-class.ll create mode 100644 llvm/test/CodeGen/X86/local-sym-storage-class.ll diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp index a32cff7b03890..ec18e7de559f5 100644 --- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -155,9 +155,9 @@ class AArch64AsmPrinter : public AsmPrinter { SetupMachineFunction(MF); if (STI->isTargetCOFF()) { - bool Internal = MF.getFunction().hasInternalLinkage(); - COFF::SymbolStorageClass Scl = Internal ? COFF::IMAGE_SYM_CLASS_STATIC - : COFF::IMAGE_SYM_CLASS_EXTERNAL; + bool Local = MF.getFunction().hasLocalLinkage(); + COFF::SymbolStorageClass Scl = + Local ? COFF::IMAGE_SYM_CLASS_STATIC : COFF::IMAGE_SYM_CLASS_EXTERNAL; int Type = COFF::IMAGE_SYM_DTYPE_FUNCTION << COFF::SCT_COMPLEX_TYPE_SHIFT; diff --git a/llvm/test/CodeGen/AArch64/local-sym-storage-class.ll b/llvm/test/CodeGen/AArch64/local-sym-storage-class.ll new file mode 100644 index 0000000000000..a3b5a8c4e5850 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/local-sym-storage-class.ll @@ -0,0 +1,15 @@ +; RUN: llc -mtriple aarch64-unknown-windows-msvc %s -o - | FileCheck %s + +define internal void @internal() { + ret void +} + +define private void @private() { + ret void +} + +; Check that the internal and private linkage symbols have IMAGE_SYM_CLASS_STATIC (3). +; CHECK: .def internal; +; CHECK: .scl 3; +; CHECK: .def .Lprivate; +; CHECK: .scl 3; diff --git a/llvm/test/CodeGen/X86/local-sym-storage-class.ll b/llvm/test/CodeGen/X86/local-sym-storage-class.ll new file mode 100644 index 0000000000000..25df0c5a97779 --- /dev/null +++ b/llvm/test/CodeGen/X86/local-sym-storage-class.ll @@ -0,0 +1,15 @@ +; RUN: llc -mtriple x86_64-unknown-windows-msvc %s -o - | FileCheck %s + +define internal void @internal() { + ret void +} + +define private void @private() { + ret void +} + +; Check that the internal and private linkage symbols have IMAGE_SYM_CLASS_STATIC (3). +; CHECK: .def internal; +; CHECK: .scl 3; +; CHECK: .def .Lprivate; +; CHECK: .scl 3; From c3964009a4d910ca8dd6b65751849a6bd74c8ae4 Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Thu, 17 Aug 2023 14:50:32 -0700 Subject: [PATCH 2/2] [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 --- llvm/lib/MC/WinCOFFObjectWriter.cpp | 6 +- .../MCTargetDesc/AArch64AsmBackend.cpp | 7 ++ .../MC/AArch64/coff-relocations-branch26.s | 75 +++++++++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 llvm/test/MC/AArch64/coff-relocations-branch26.s diff --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp index e2d1a5d7b8ee5..fe13ec84006b2 100644 --- a/llvm/lib/MC/WinCOFFObjectWriter.cpp +++ b/llvm/lib/MC/WinCOFFObjectWriter.cpp @@ -672,7 +672,9 @@ void WinCOFFObjectWriter::executePostLayoutBinding(MCAssembler &Asm, defineSection(static_cast(Section), Layout); for (const MCSymbol &Symbol : Asm.symbols()) - if (!Symbol.isTemporary()) + // Define non-temporary or temporary static (private-linkage) symbols + if (!Symbol.isTemporary() || + cast(Symbol).getClass() == COFF::IMAGE_SYM_CLASS_STATIC) DefineSymbol(Symbol, Asm, Layout); } @@ -750,7 +752,7 @@ void WinCOFFObjectWriter::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.find(TargetSection) != SectionMap.end() && diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp index fb6c5af8f5fa3..801e0f75b5e62 100644 --- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp +++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp @@ -313,6 +313,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 (SignedValue > 134217727 || SignedValue < -134217728) 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 0000000000000..4cd47032309ca --- /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
: +// CHECK: 0: d503201f nop +// CHECK: 4: 14000000 b 0x4 +// CHECK: 0000000000000004: IMAGE_REL_ARM64_BRANCH26 .Ltarget +// CHECK: 8: 14000000 b 0x8 +// CHECK: 0000000000000008: IMAGE_REL_ARM64_BRANCH26 .Lother_target +// CHECK: 000000000000000c <.Ltarget>: +// CHECK: c: d65f03c0 ret +// CHECK: 0000000000000000 : +// 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