[RISCV] Implement getExprForFDESymbol to ensure RISCV_32_PCREL is used for the FDE...
authorAlex Bradbury <asb@lowrisc.org>
Tue, 20 Aug 2019 12:32:31 +0000 (12:32 +0000)
committerAlex Bradbury <asb@lowrisc.org>
Tue, 20 Aug 2019 12:32:31 +0000 (12:32 +0000)
Follow binutils in using RISCV_32_PCREL for the FDE initial location. As
explained in the relevant binutils commit
<https://github.com/riscv/riscv-binutils-gdb/commit/a6cbf936e3dce68114d28cdf60d510a3f78a6d40>,
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

lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp
lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.h
lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
test/MC/RISCV/fde-reloc.s

index 9c180a51d45528d5f249bef48f18a32517b2f850..1122f275c3336feb1933f6b1cc99deef5d33c866 100644 (file)
@@ -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<RISCVMCExpr>(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;
index 98362969288339a287878174b7797f5fa51493e6..089a2def4c210e03fdb495ae77986a9f8c1529ea 100644 (file)
 //===----------------------------------------------------------------------===//
 
 #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);
+}
index 043fdb7c08c0af1123dd7bb62c4cdfae86cdfccb..6824baf699aaf47a08e794f97850f6b482d78550 100644 (file)
@@ -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
index 6732534afe3ba2117ccbad4673903f07b5893ba0..de99960848a520f5300c2a075e16c1338b99e239 100644 (file)
@@ -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
index b5a292dc1b1a6ece8e7a1b933d837cc1a95b73ba..921df376f3dfc83d265ae6ad4df13f7224dd09fd 100644 (file)
@@ -36,6 +36,7 @@ public:
     VK_RISCV_TLS_GD_HI,
     VK_RISCV_CALL,
     VK_RISCV_CALL_PLT,
+    VK_RISCV_32_PCREL,
     VK_RISCV_Invalid
   };
 
index a2cb0825cff35f8899de4ac0c0a8c8719b538fde..335594de3aba6ca5fd59867b47685d62ee70da5d 100644 (file)
@@ -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: }