From efea2847860eedae9dd6fabdc44193f6795eb4d6 Mon Sep 17 00:00:00 2001 From: Alex Bradbury Date: Tue, 20 Aug 2019 12:32:31 +0000 Subject: [PATCH] [RISCV] Implement getExprForFDESymbol to ensure RISCV_32_PCREL is used for the FDE location Follow binutils in using RISCV_32_PCREL for the FDE initial location. As explained in the relevant binutils commit , the ADD/SUB pair of relocations is problematic in the presence of linker relaxation. This patch has the same end goal as D64715 but includes test changes and avoids adding a new global VariantKind to MCExpr.h (preferring RISCVMCExpr VKs like the rest of the RISC-V backend). Differential Revision: https://reviews.llvm.org/D66419 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@369375 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../MCTargetDesc/RISCVELFObjectWriter.cpp | 5 +++++ .../RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp | 20 +++++++++++++++++++ .../RISCV/MCTargetDesc/RISCVMCAsmInfo.h | 3 +++ .../RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp | 1 + lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h | 1 + test/MC/RISCV/fde-reloc.s | 8 +------- 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp b/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp index 9c180a51d45..1122f275c33 100644 --- a/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp +++ b/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "MCTargetDesc/RISCVFixupKinds.h" +#include "MCTargetDesc/RISCVMCExpr.h" #include "MCTargetDesc/RISCVMCTargetDesc.h" #include "llvm/MC/MCELFObjectWriter.h" #include "llvm/MC/MCFixup.h" @@ -47,6 +48,7 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target, const MCFixup &Fixup, bool IsPCRel) const { + const MCExpr *Expr = Fixup.getValue(); // Determine the type of the relocation unsigned Kind = Fixup.getKind(); if (IsPCRel) { @@ -87,6 +89,9 @@ unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx, default: llvm_unreachable("invalid fixup kind!"); case FK_Data_4: + if (Expr->getKind() == MCExpr::Target && + cast(Expr)->getKind() == RISCVMCExpr::VK_RISCV_32_PCREL) + return ELF::R_RISCV_32_PCREL; return ELF::R_RISCV_32; case FK_Data_8: return ELF::R_RISCV_64; diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp b/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp index 98362969288..089a2def4c2 100644 --- a/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp +++ b/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp @@ -11,7 +11,10 @@ //===----------------------------------------------------------------------===// #include "RISCVMCAsmInfo.h" +#include "MCTargetDesc/RISCVMCExpr.h" #include "llvm/ADT/Triple.h" +#include "llvm/BinaryFormat/Dwarf.h" +#include "llvm/MC/MCStreamer.h" using namespace llvm; void RISCVMCAsmInfo::anchor() {} @@ -25,3 +28,20 @@ RISCVMCAsmInfo::RISCVMCAsmInfo(const Triple &TT) { Data16bitsDirective = "\t.half\t"; Data32bitsDirective = "\t.word\t"; } + +const MCExpr *RISCVMCAsmInfo::getExprForFDESymbol(const MCSymbol *Sym, + unsigned Encoding, + MCStreamer &Streamer) const { + if (!(Encoding & dwarf::DW_EH_PE_pcrel)) + return MCAsmInfo::getExprForFDESymbol(Sym, Encoding, Streamer); + + // The default symbol subtraction results in an ADD/SUB relocation pair. + // Processing this relocation pair is problematic when linker relaxation is + // enabled, so we follow binutils in using the R_RISCV_32_PCREL relocation + // for the FDE initial location. + MCContext &Ctx = Streamer.getContext(); + const MCExpr *ME = + MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, Ctx); + assert(Encoding & dwarf::DW_EH_PE_sdata4 && "Unexpected encoding"); + return RISCVMCExpr::create(ME, RISCVMCExpr::VK_RISCV_32_PCREL, Ctx); +} diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.h b/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.h index 043fdb7c08c..6824baf699a 100644 --- a/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.h +++ b/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.h @@ -23,6 +23,9 @@ class RISCVMCAsmInfo : public MCAsmInfoELF { public: explicit RISCVMCAsmInfo(const Triple &TargetTriple); + + const MCExpr *getExprForFDESymbol(const MCSymbol *Sym, unsigned Encoding, + MCStreamer &Streamer) const override; }; } // namespace llvm diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp index 6732534afe3..de99960848a 100644 --- a/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp +++ b/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp @@ -267,6 +267,7 @@ unsigned RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo, switch (RVExpr->getKind()) { case RISCVMCExpr::VK_RISCV_None: case RISCVMCExpr::VK_RISCV_Invalid: + case RISCVMCExpr::VK_RISCV_32_PCREL: llvm_unreachable("Unhandled fixup kind!"); case RISCVMCExpr::VK_RISCV_TPREL_ADD: // tprel_add is only used to indicate that a relocation should be emitted diff --git a/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h b/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h index b5a292dc1b1..921df376f3d 100644 --- a/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h +++ b/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h @@ -36,6 +36,7 @@ public: VK_RISCV_TLS_GD_HI, VK_RISCV_CALL, VK_RISCV_CALL_PLT, + VK_RISCV_32_PCREL, VK_RISCV_Invalid }; diff --git a/test/MC/RISCV/fde-reloc.s b/test/MC/RISCV/fde-reloc.s index a2cb0825cff..335594de3ab 100644 --- a/test/MC/RISCV/fde-reloc.s +++ b/test/MC/RISCV/fde-reloc.s @@ -8,14 +8,8 @@ func: ret .cfi_endproc -# TODO: Should produce R_RISCV_32_PCREL for the FDE pc relocation. Many of the -# ADD32/SUB32 relocations also can be safely resolved even with linker -# relaxation enabled. This test is written to capture current behaviour, in -# preparation for follow-on patches to fix it. - # RELAX-RELOC: Section (4) .rela.eh_frame { -# RELAX-RELOC-NEXT: 0x1C R_RISCV_ADD32 - 0x0 -# RELAX-RELOC-NEXT: 0x1C R_RISCV_SUB32 - 0x0 +# RELAX-RELOC-NEXT: 0x1C R_RISCV_32_PCREL - 0x0 # RELAX-RELOC-NEXT: 0x20 R_RISCV_ADD32 - 0x0 # RELAX-RELOC-NEXT: 0x20 R_RISCV_SUB32 - 0x0 # RELAX-RELOC-NEXT: } -- 2.40.0