From 8e8ccf4712cf58562a91c197da3efd4f9963ce0d Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Mon, 13 Jan 2020 13:52:48 +0000 Subject: [PATCH] [MIPS] Don't emit R_(MICRO)MIPS_JALR relocations against data symbols The R_(MICRO)MIPS_JALR optimization only works when used against functions. Using the relocation against a data symbol (e.g. function pointer) will cause some linkers that don't ignore the hint in this case (e.g. LLD prior to commit 5bab291b7b) to generate a relative branch to the data symbol which crashes at run time. Before this patch, LLVM was erroneously emitting these relocations against local-dynamic TLS function pointers and global function pointers with internal visibility. Reviewers: atanasyan, jrtc27, vstefanovic Reviewed By: atanasyan Differential Revision: https://reviews.llvm.org/D72571 --- llvm/lib/Target/Mips/MipsISelLowering.cpp | 9 ++ llvm/test/CodeGen/Mips/reloc-jalr.ll | 143 +++++++++++++++++++----------- 2 files changed, 99 insertions(+), 53 deletions(-) diff --git a/llvm/lib/Target/Mips/MipsISelLowering.cpp b/llvm/lib/Target/Mips/MipsISelLowering.cpp index ea242b4..46b1f35 100644 --- a/llvm/lib/Target/Mips/MipsISelLowering.cpp +++ b/llvm/lib/Target/Mips/MipsISelLowering.cpp @@ -3111,6 +3111,14 @@ void MipsTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI, StringRef Sym; if (const GlobalAddressSDNode *G = dyn_cast_or_null(TargetAddr)) { + // We must not emit the R_MIPS_JALR relocation against data symbols + // since this will cause run-time crashes if the linker replaces the + // call instruction with a relative branch to the data symbol. + if (!isa(G->getGlobal())) { + LLVM_DEBUG(dbgs() << "Not adding R_MIPS_JALR against data symbol " + << G->getGlobal()->getName() << "\n"); + return; + } Sym = G->getGlobal()->getName(); } else if (const ExternalSymbolSDNode *ES = @@ -3123,6 +3131,7 @@ void MipsTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI, MachineFunction *MF = MI.getParent()->getParent(); MCSymbol *S = MF->getContext().getOrCreateSymbol(Sym); + LLVM_DEBUG(dbgs() << "Adding R_MIPS_JALR against " << Sym << "\n"); MI.addOperand(MachineOperand::CreateMCSymbol(S, MipsII::MO_JALR)); } } diff --git a/llvm/test/CodeGen/Mips/reloc-jalr.ll b/llvm/test/CodeGen/Mips/reloc-jalr.ll index f8fd903..ef36026 100644 --- a/llvm/test/CodeGen/Mips/reloc-jalr.ll +++ b/llvm/test/CodeGen/Mips/reloc-jalr.ll @@ -1,64 +1,64 @@ ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-32R2,TAILCALL-32R2 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-32,JALR-32R2,TAILCALL-32R2 ; RUN: llc -mtriple=mips64-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-64R2,TAILCALL-64R2 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-64,JALR-64R2,TAILCALL-64R2 ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 -mcpu=mips32r6 -mips-compact-branches=always < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-32R6,TAILCALL-32R6 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-32,JALR-32R6,TAILCALL-32R6 ; RUN: llc -mtriple=mips64-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 -mcpu=mips64r6 -mips-compact-branches=always < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-64R6,TAILCALL-64R6 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-64,JALR-64R6,TAILCALL-64R6 ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 -mcpu=mips32r6 -mips-compact-branches=never < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-32R2,TAILCALL-32R2 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-32,JALR-32R2,TAILCALL-32R2 ; RUN: llc -mtriple=mips64-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 -mcpu=mips64r6 -mips-compact-branches=never < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-64R2,TAILCALL-64R2 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-64,JALR-64R2,TAILCALL-64R2 ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 -mattr=+micromips -mcpu=mips32r2 < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-MM,TAILCALL-MM +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-MM,TAILCALL-MM ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 -mattr=+micromips -mcpu=mips32r6 < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-MM +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-MM,TAILCALL-MM ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic \ -; RUN: -O0 < %s | FileCheck %s -check-prefixes=ALL,JALR-32R2 +; RUN: -O0 < %s | FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-32,JALR-32R2,PIC-NOTAILCALL-R2 ; RUN: llc -mtriple=mips64-linux-gnu -relocation-model=pic \ -; RUN: -O0 < %s | FileCheck %s -check-prefixes=ALL,JALR-64R2 +; RUN: -O0 < %s | FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-64,JALR-64R2,PIC-NOTAILCALL-R2 ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic \ ; RUN: -O0 -mcpu=mips32r6 -mips-compact-branches=always < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-32R6 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-32,JALR-32R6,PIC-NOTAILCALL-R6 ; RUN: llc -mtriple=mips64-linux-gnu -relocation-model=pic \ ; RUN: -O0 -mcpu=mips64r6 -mips-compact-branches=always < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-64R6 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-64,JALR-64R6,PIC-NOTAILCALL-R6 ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic \ ; RUN: -O0 -mcpu=mips32r6 -mips-compact-branches=never < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-32R2 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-32,JALR-32R2,PIC-NOTAILCALL-R2 ; RUN: llc -mtriple=mips64-linux-gnu -relocation-model=pic \ ; RUN: -O0 -mcpu=mips64r6 -mips-compact-branches=never < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-64R2 +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-64,JALR-64R2,PIC-NOTAILCALL-R2 ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic \ ; RUN: -O0 -mattr=+micromips -mcpu=mips32r2 < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-MM +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-MM,PIC-NOTAILCALL-MM ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic \ ; RUN: -O0 -mattr=+micromips -mcpu=mips32r6 < %s | \ -; RUN: FileCheck %s -check-prefixes=ALL,JALR-MM +; RUN: FileCheck %s -check-prefixes=ALL,JALR-ALL,JALR-MM,PIC-NOTAILCALL-MM ; RUN: llc -mtriple=mips-linux-gnu -relocation-model=pic -mips-tail-calls=1 \ ; RUN: -O2 -mips-jalr-reloc=false < %s | \ @@ -100,55 +100,92 @@ entry: define void @checkCall() { entry: ; ALL-LABEL: checkCall: +; ALL-NOT: MIPS_JALR call void @foo() -; JALR-32R2: .reloc ([[TMPLABEL:.*]]), R_MIPS_JALR, foo -; JALR-32R2-NEXT: [[TMPLABEL]]: -; JALR-32R2-NEXT: jalr $25 - -; JALR-64R2: .reloc [[TMPLABEL:.*]], R_MIPS_JALR, foo -; JALR-64R2-NEXT: [[TMPLABEL]]: -; JALR-64R2-NEXT: jalr $25 - -; JALR-MM: .reloc ([[TMPLABEL:.*]]), R_MICROMIPS_JALR, foo -; JALR-MM-NEXT: [[TMPLABEL]]: -; JALR-MM-NEXT: jalr $25 - -; JALR-32R6: .reloc ([[TMPLABEL:.*]]), R_MIPS_JALR, foo -; JALR-32R6-NEXT: [[TMPLABEL]]: -; JALR-32R6-NEXT: jalrc $25 - -; JALR-64R6: .reloc [[TMPLABEL:.*]], R_MIPS_JALR, foo -; JALR-64R6-NEXT: [[TMPLABEL]]: -; JALR-64R6-NEXT: jalrc $25 - -; NORELOC-NOT: R_MIPS_JALR +; JALR-32: .reloc ([[TMPLABEL:\$.+]]), R_MIPS_JALR, foo +; JALR-64: .reloc [[TMPLABEL:\..+]], R_MIPS_JALR, foo +; JALR-MM: .reloc ([[TMPLABEL:\$.+]]), R_MICROMIPS_JALR, foo +; JALR-ALL-NEXT: [[TMPLABEL]]: +; JALR-32R2-NEXT: jalr $25 +; JALR-64R2-NEXT: jalr $25 +; JALR-32R6-NEXT: jalrc $25 +; JALR-64R6-NEXT: jalrc $25 +; JALR-MM-NEXT: jalr $25 +; ALL-NOT: MIPS_JALR ret void } define void @checkTailCall() { entry: ; ALL-LABEL: checkTailCall: +; ALL-NOT: MIPS_JALR tail call void @foo() -; TAILCALL-32R2: .reloc ([[TMPLABEL:.*]]), R_MIPS_JALR, foo -; TAILCALL-32R2-NEXT: [[TMPLABEL]]: -; TAILCALL-32R2-NEXT: jr $25 +; JALR-32: .reloc ([[TMPLABEL:\$.+]]), R_MIPS_JALR, foo +; JALR-64: .reloc [[TMPLABEL:\..+]], R_MIPS_JALR, foo +; JALR-MM: .reloc ([[TMPLABEL:\$.+]]), R_MICROMIPS_JALR, foo +; JALR-ALL-NEXT: [[TMPLABEL]]: +; TAILCALL-32R2-NEXT: jr $25 +; TAILCALL-64R2-NEXT: jr $25 +; TAILCALL-MM-NEXT: jrc $25 +; TAILCALL-32R6-NEXT: jrc $25 +; TAILCALL-64R6-NEXT: jrc $25 +; PIC-NOTAILCALL-R2-NEXT: jalr $25 +; PIC-NOTAILCALL-R6-NEXT: jalrc $25 +; PIC-NOTAILCALL-MM-NEXT: jalr $25 +; ALL-NOT: MIPS_JALR + ret void +} -; TAILCALL-64R2: .reloc [[TMPLABEL:.*]], R_MIPS_JALR, foo -; TAILCALL-64R2-NEXT: [[TMPLABEL]]: -; TAILCALL-64R2-NEXT: jr $25 +; Check that we don't emit R_MIPS_JALR relocations against function pointers. +; This resulted in run-time crashes until lld was modified to ignore +; R_MIPS_JALR relocations against data symbols (commit 5bab291b7b). +; However, the better approach is to not emit these relocations in the first +; place so check that we no longer emit them. +; Previously we were adding them for local dynamic TLS function pointers and +; function pointers with internal linkage. -; TAILCALL-MM: .reloc ([[TMPLABEL:.*]]), R_MICROMIPS_JALR, foo -; TAILCALL-MM-NEXT: [[TMPLABEL]]: -; TAILCALL-MM-NEXT: jrc $25 +@fnptr_internal = internal global void()* @checkFunctionPointerCall +@fnptr_internal_const = internal constant void()* @checkFunctionPointerCall +@fnptr_const = constant void()* @checkFunctionPointerCall +@fnptr_global = global void()* @checkFunctionPointerCall -; TAILCALL-32R6: .reloc ([[TMPLABEL:.*]]), R_MIPS_JALR, foo -; TAILCALL-32R6-NEXT: [[TMPLABEL]]: -; TAILCALL-32R6-NEXT: jrc $25 +define void @checkFunctionPointerCall() { +entry: +; ALL-LABEL: checkFunctionPointerCall: +; ALL-NOT: MIPS_JALR + %func_internal = load void()*, void()** @fnptr_internal + call void %func_internal() + %func_internal_const = load void()*, void()** @fnptr_internal_const + call void %func_internal_const() + %func_const = load void()*, void()** @fnptr_const + call void %func_const() + %func_global = load void()*, void()** @fnptr_global + call void %func_global() + ret void +} -; TAILCALL-64R6: .reloc [[TMPLABEL:.*]], R_MIPS_JALR, foo -; TAILCALL-64R6-NEXT: [[TMPLABEL]]: -; TAILCALL-64R6-NEXT: jrc $25 +@tls_fnptr_gd = thread_local global void()* @checkTlsFunctionPointerCall +@tls_fnptr_ld = thread_local(localdynamic) global void()* @checkTlsFunctionPointerCall +@tls_fnptr_ie = thread_local(initialexec) global void()* @checkTlsFunctionPointerCall +@tls_fnptr_le = thread_local(localexec) global void()* @checkTlsFunctionPointerCall -; NORELOC-NOT: R_MIPS_JALR +define void @checkTlsFunctionPointerCall() { +entry: +; There should not be any *JALR relocations in this function other than the +; calls to __tls_get_addr: +; ALL-LABEL: checkTlsFunctionPointerCall: +; ALL-NOT: MIPS_JALR +; JALR-ALL: .reloc {{.+}}MIPS_JALR, __tls_get_addr +; ALL-NOT: MIPS_JALR +; JALR-ALL: .reloc {{.+}}MIPS_JALR, __tls_get_addr +; ALL-NOT: _MIPS_JALR + %func_gd = load void()*, void()** @tls_fnptr_gd + call void %func_gd() + %func_ld = load void()*, void()** @tls_fnptr_ld + call void %func_ld() + %func_ie = load void()*, void()** @tls_fnptr_ie + call void %func_ie() + %func_le = load void()*, void()** @tls_fnptr_le + call void %func_le() ret void } -- 2.7.4