[JITLink][RISCV] Implement .eh_frame handling #66067

mtvec · 2023-09-12T10:33:17Z

This patch enables .eh_frame handling on RISC-V by using the common DWARFRecordSectionSplitter, EHFrameEdgeFixer, and EHFrameNullTerminator passes.

This mostly works out of the box but a minor change was needed for EHFrameEdgeFixer: on RISC-V, ADD/SUB relocations are used to calculate the length of a sequence of instructions when relaxation is enabled. Since both relocations are at the same offset, this caused an error to be raised by EHFrameEdgeFixer. I have solved this issue by simply ignoring relocations at the same offset on RISC-V. I believe this is fine since the DWARF fields where they are used (PC-range and DW_CFA_advance_loc) don't need any special handling.

Besides this, two new edge kinds needed to be implemented for RISC-V: Delta64 and NegDelta32

llvmbot · 2023-09-12T10:34:20Z

@llvm/pr-subscribers-backend-risc-v

Changes

This patch enables .eh_frame handling on RISC-V by using the common DWARFRecordSectionSplitter, EHFrameEdgeFixer, and EHFrameNullTerminator passes.

This mostly works out of the box but a minor change was needed for EHFrameEdgeFixer: on RISC-V, ADD/SUB relocations are used to calculate the length of a sequence of instructions when relaxation is enabled. Since both relocations are at the same offset, this caused an error to be raised by EHFrameEdgeFixer. I have solved this issue by simply ignoring relocations at the same offset on RISC-V. I believe this is fine since the DWARF fields where they are used (PC-range and DW_CFA_advance_loc) don't need any special handling.

Besides this, two new edge kinds needed to be implemented for RISC-V: `Delta64` and `NegDelta32`

Full diff: https://github.com/llvm/llvm-project/pull/66067.diff

6 Files Affected:

(modified) llvm/include/llvm/ExecutionEngine/JITLink/riscv.h (+21)
(modified) llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp (+8-1)
(modified) llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp (+21)
(modified) llvm/lib/ExecutionEngine/JITLink/riscv.cpp (+4)
(added) llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.s (+77)
(added) llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.test (+104)

diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/riscv.h b/llvm/include/llvm/ExecutionEngine/JITLink/riscv.h
index cb66289180880ce..b805dd1ed5bdc85 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/riscv.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/riscv.h
@@ -203,6 +203,27 @@ enum EdgeKind_riscv : Edge::Kind {
   ///   Fixup <- (Target - Fixup + Addend)
   R_RISCV_32_PCREL,
 
+  /// A 64-bit delta.
+  ///
+  /// Delta from the fixup to the target.
+  ///
+  /// Fixup expression:
+  ///   Fixup <- Target - Fixup + Addend : int64
+  ///
+  Delta64,
+
+  /// A 32-bit negative delta.
+  ///
+  /// Delta from the target back to the fixup.
+  ///
+  /// Fixup expression:
+  ///   Fixup <- Fixup - Target + Addend : int32
+  ///
+  /// Errors:
+  ///   - The result of the fixup expression must fit into an int32, otherwise
+  ///     an out-of-range error will be returned.
+  NegDelta32,
+
   /// An auipc/jalr pair eligible for linker relaxation.
   ///
   /// Linker relaxation will replace this with R_RISCV_RVC_JUMP or R_RISCV_JAL
diff --git a/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp b/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp
index 86249591a9be053..ea56a6d533f7a7d 100644
--- a/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp
@@ -129,11 +129,18 @@ Error EHFrameEdgeFixer::processBlock(ParseContext &PC, Block &B) {
   BlockEdgeMap BlockEdges;
   for (auto &E : B.edges())
     if (E.isRelocation()) {
-      if (BlockEdges.count(E.getOffset()))
+      if (BlockEdges.count(E.getOffset())) {
+        // RISC-V may use ADD/SUB relocation pairs for PC-range and
+        // DW_CFA_advance_loc. We don't need to process these fields here so
+        // just ignore this on RISC-V.
+        if (PC.G.getTargetTriple().isRISCV())
+          continue;
+
         return make_error(
             "Multiple relocations at offset " +
             formatv("{0:x16}", E.getOffset()) + " in " + EHFrameSectionName +
             " block at address " + formatv("{0:x16}", B.getAddress()));
+      }
 
       BlockEdges[E.getOffset()] = EdgeTarget(E);
     }
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
index 410dd7fedad1a40..664cb8a56277265 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
@@ -11,10 +11,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ExecutionEngine/JITLink/ELF_riscv.h"
+#include "EHFrameSupportImpl.h"
 #include "ELFLinkGraphBuilder.h"
 #include "JITLinkGeneric.h"
 #include "PerGraphGOTAndPLTStubsBuilder.h"
 #include "llvm/BinaryFormat/ELF.h"
+#include "llvm/ExecutionEngine/JITLink/DWARFRecordSectionSplitter.h"
 #include "llvm/ExecutionEngine/JITLink/JITLink.h"
 #include "llvm/ExecutionEngine/JITLink/riscv.h"
 #include "llvm/Object/ELF.h"
@@ -453,6 +455,18 @@ class ELFJITLinker_riscv : public JITLinker {
       *(little32_t *)FixupPtr = Word32;
       break;
     }
+    case Delta64: {
+      int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
+      *(little64_t *)FixupPtr = Value;
+      break;
+    }
+    case NegDelta32: {
+      int64_t Value = FixupAddress - E.getTarget().getAddress() + E.getAddend();
+      if (LLVM_UNLIKELY(!isInRangeForImm(Value, 32)))
+        return makeTargetOutOfRangeError(G, B, E);
+      *(little32_t *)FixupPtr = Value;
+      break;
+    }
     case AlignRelaxable:
       // Ignore when the relaxation pass did not run
       break;
@@ -959,6 +973,13 @@ void link_ELF_riscv(std::unique_ptr G,
   PassConfiguration Config;
   const Triple &TT = G->getTargetTriple();
   if (Ctx->shouldAddDefaultTargetPasses(TT)) {
+    // Add eh-frame passses.
+    Config.PrePrunePasses.push_back(DWARFRecordSectionSplitter(".eh_frame"));
+    Config.PrePrunePasses.push_back(
+        EHFrameEdgeFixer(".eh_frame", TT.isRISCV32() ? 4 : 8, riscv::R_RISCV_32,
+                         riscv::R_RISCV_64, riscv::R_RISCV_32_PCREL,
+                         riscv::Delta64, riscv::NegDelta32));
+    Config.PrePrunePasses.push_back(EHFrameNullTerminator(".eh_frame"));
     if (auto MarkLive = Ctx->getMarkLivePass(TT))
       Config.PrePrunePasses.push_back(std::move(MarkLive));
     else
diff --git a/llvm/lib/ExecutionEngine/JITLink/riscv.cpp b/llvm/lib/ExecutionEngine/JITLink/riscv.cpp
index a78843b1614795e..207f73cb72f9f5d 100644
--- a/llvm/lib/ExecutionEngine/JITLink/riscv.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/riscv.cpp
@@ -78,6 +78,10 @@ const char *getEdgeKindName(Edge::Kind K) {
     return "R_RISCV_SET32";
   case R_RISCV_32_PCREL:
     return "R_RISCV_32_PCREL";
+  case Delta64:
+    return "Delta64";
+  case NegDelta32:
+    return "NegDelta32";
   case CallRelaxable:
     return "CallRelaxable";
   case AlignRelaxable:
diff --git a/llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.s b/llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.s
new file mode 100644
index 000000000000000..0bb2e60b81ba775
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.s
@@ -0,0 +1,77 @@
+# REQUIRES: asserts
+
+# RUN: llvm-mc -triple=riscv32 -mattr=+relax -filetype=obj -o %t.32.o %s
+# RUN: llvm-jitlink -noexec -phony-externals -debug-only=jitlink %t.32.o 2>&1 | \
+# RUN:   FileCheck %s
+
+# RUN: llvm-mc -triple=riscv64 -mattr=+relax -filetype=obj -o %t.64.o %s
+# RUN: llvm-jitlink -noexec -phony-externals -debug-only=jitlink %t.64.o 2>&1 | \
+# RUN:   FileCheck %s
+
+# Check that splitting of eh-frame sections works.
+#
+# CHECK: DWARFRecordSectionSplitter: Processing .eh_frame...
+# CHECK:  Processing block at
+# CHECK:    Processing CFI record at
+# CHECK:      Extracted {{.*}} section = .eh_frame
+# CHECK:    Processing CFI record at
+# CHECK:      Extracted {{.*}} section = .eh_frame
+# CHECK: EHFrameEdgeFixer: Processing .eh_frame in "{{.*}}"...
+# CHECK:   Processing block at
+# CHECK:     Processing CFI record at
+# CHECK:       Record is CIE
+# CHECK:   Processing block at
+# CHECK:     Processing CFI record at
+# CHECK:       Record is FDE
+# CHECK:         Adding edge at {{.*}} to CIE at: {{.*}}
+# CHECK:         Existing edge at {{.*}} to PC begin at {{.*}}
+# CHECK:         Adding keep-alive edge from target at {{.*}} to FDE at {{.*}}
+# CHECK:   Processing block at
+# CHECK:     Processing CFI record at
+# CHECK:       Record is FDE
+# CHECK:         Adding edge at {{.*}} to CIE at: {{.*}}
+# CHECK:         Existing edge at {{.*}} to PC begin at {{.*}}
+# CHECK:         Adding keep-alive edge from target at {{.*}} to FDE at {{.*}}
+
+## This is "int main { throw 1; }" compiled for riscv32. We use the 32-bit
+## version because it is also legal for riscv64.
+	.text
+	.globl	main
+	.p2align	1
+	.type	main,@function
+main:
+	.cfi_startproc
+	addi	sp, sp, -16
+	.cfi_def_cfa_offset 16
+	sw	ra, 12(sp)
+	.cfi_offset ra, -4
+	li	a0, 4
+	call	__cxa_allocate_exception
+	li	a1, 1
+	sw	a1, 0(a0)
+  la a1, _ZTIi
+	li	a2, 0
+	call	__cxa_throw
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+
+	.globl	dup
+	.p2align	1
+	.type	dup,@function
+dup:
+	.cfi_startproc
+	addi	sp, sp, -16
+	.cfi_def_cfa_offset 16
+	sw	ra, 12(sp)
+	.cfi_offset ra, -4
+	li	a0, 4
+	call	__cxa_allocate_exception
+	li	a1, 1
+	sw	a1, 0(a0)
+  la a1, _ZTIi
+	li	a2, 0
+	call	__cxa_throw
+.Lfunc_end1:
+	.size	dup, .Lfunc_end1-dup
+	.cfi_endproc
diff --git a/llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.test b/llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.test
new file mode 100644
index 000000000000000..95666d2e232b70d
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/RISCV/ELF_ehframe.test
@@ -0,0 +1,104 @@
+# RUN: yaml2obj -DELFCLASS=ELFCLASS32 -o %t.32.o %s
+# RUN: llvm-jitlink -noexec -check %s %t.32.o
+# RUN: yaml2obj -DELFCLASS=ELFCLASS64 -o %t.64.o %s
+# RUN: llvm-jitlink -noexec -check %s %t.64.o
+
+### Compiled from the following code with -mattr=+relax to force relocations for
+### address_range and DW_CFA_advance_loc (both needed for .balign).
+## 	.text
+## 	.globl	main
+## 	.p2align	1
+## 	.type	main,@function
+## main:
+## 	.cfi_startproc
+##     .balign 8
+## 	addi	sp, sp, -16
+## cfa_advance_loc:
+## 	.cfi_def_cfa_offset 16
+##     nop
+## main_end:
+## 	.size	main, main_end-main
+## 	.cfi_endproc
+
+--- !ELF
+FileHeader:
+  Class:           [[ELFCLASS]]
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_RISCV
+  SectionHeaderStringTable: .strtab
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x8
+    Content:         13000000130101FF13000000
+  - Name:            .eh_frame
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x8
+    Content:         1000000000000000017A5200017801011B0C02001000000018000000000000000000000000400E10
+  - Name:            .rela.text
+    Type:            SHT_RELA
+    Flags:           [ SHF_INFO_LINK ]
+    Link:            .symtab
+    AddressAlign:    0x8
+    Info:            .text
+    Relocations:
+      - Type:            R_RISCV_ALIGN
+        Addend:          4
+  - Name:            .rela.eh_frame
+    Type:            SHT_RELA
+    Flags:           [ SHF_INFO_LINK ]
+    Link:            .symtab
+    AddressAlign:    0x8
+    Info:            .eh_frame
+    Relocations:
+      - Offset:          0x1C
+        Symbol:          main
+        Type:            R_RISCV_32_PCREL
+      - Offset:          0x20
+        Symbol:          main_end
+        Type:            R_RISCV_ADD32
+      - Offset:          0x20
+        Symbol:          main
+        Type:            R_RISCV_SUB32
+      - Offset:          0x25
+        Symbol:          cfa_advance_loc
+        Type:            R_RISCV_SET6
+      - Offset:          0x25
+        Symbol:          main
+        Type:            R_RISCV_SUB6
+  - Type:            SectionHeaderTable
+    Sections:
+      - Name:            .strtab
+      - Name:            .text
+      - Name:            .rela.text
+      - Name:            .eh_frame
+      - Name:            .rela.eh_frame
+      - Name:            .symtab
+Symbols:
+  - Name:            cfa_advance_loc
+    Section:         .text
+    Value:           0x8
+  - Name:            main_end
+    Section:         .text
+    Value:           0xC
+  - Name:            main
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Size:            0xC
+  - Name:            eh_frame
+    Type:            STT_SECTION
+    Binding:         STB_GLOBAL
+    Section:         .eh_frame
+    Size:            0x28
+...
+
+## CIE_pointer
+# jitlink-check: *{4}(eh_frame + 0x1c) = main - (eh_frame + 0x1c)
+## address_range
+# jitlink-check: *{4}(eh_frame + 0x20) = main_end - main
+## DW_CFA_advance_loc
+# jitlink-check: (*{1}(eh_frame + 0x25)) & 0x3f = cfa_advance_loc - main

mtvec · 2023-09-13T08:27:03Z

Use lga instead of la to force GOT entry creation and fix Windows test failure.

This patch enables .eh_frame handling on RISC-V by using the common `DWARFRecordSectionSplitter`, `EHFrameEdgeFixer`, and `EHFrameNullTerminator` passes. This mostly works out of the box but a minor change was needed for `EHFrameEdgeFixer`: on RISC-V, ADD/SUB relocations are used to calculate the length of a sequence of instructions when relaxation is enabled. Since both relocations are at the same offset, this caused an error to be raised by `EHFrameEdgeFixer`. I have solved this issue by simply ignoring relocations at the same offset on RISC-V. I believe this is fine since the DWARF fields where they are used (PC-range and `DW_CFA_advance_loc`) don't need any special handling. Besides this, two new edge kinds needed to be implemented for RISC-V: `Delta64` and `NegDelta32`

mtvec · 2023-09-13T12:22:24Z

Rebased after #66192 to fix Windows ci issues.

lhames · 2023-09-18T05:05:55Z

@mtvec -- @hahnjo opened a similar review a while back: https://reviews.llvm.org/D157802

The AddFDEToCIEEdges solution would be best, but as a stop-gap we could also just comment out the assertion and add a FIXME to reintroduce it once we've figured out the right edge design.

asb · 2023-09-27T11:41:43Z

I haven't followed the set of related discussions on this, but just to note the lack of this is currently causing a test failure in the clang-repl tests for RISC-V (seemingly a test case that was enabled by https://reviews.llvm.org/D159167). As the rv64gc qemu-user builder has been stable for a long time (i.e. only flagging actual bugs introduced by patches), I'd like to move it from the staging buildmaster, but would obviously like to see it go green before doing so.

This test fails as .eh_frame handling is not yet implemented for RISC-V in JITLink. llvm#66067 is proposed to address this. Skip the test until the issue is resolved. It seems that D159167 enabled this test for more than just ppc64.

This test fails as .eh_frame handling is not yet implemented for RISC-V in JITLink. #66067 is proposed to address this. Skip the test until the issue is resolved. It seems that D159167 enabled this test for more than just ppc64. As the test always failed, it just wasn't run until now, I think skipping is the correct interim approach (as is already done for Arm, Darwin, and others).

hahnjo · 2023-10-04T20:20:06Z

Please see #68252 and #68253 for a proposal along the lines of our discussion on https://reviews.llvm.org/D157802 and https://reviews.llvm.org/D157803. It passes my very basic testing, more thorough checks would be appreciated.

hahnjo · 2023-10-29T10:46:35Z

I believe this PR can be closed now, I merged #68253.

mtvec · 2023-10-30T07:51:02Z

Indeed, thanks for implementing this!

mtvec added the jitlink label Sep 12, 2023

mtvec requested review from lhames and luxufan September 12, 2023 10:33

mtvec requested a review from a team as a code owner September 12, 2023 10:33

llvmbot added the backend:RISC-V label Sep 12, 2023

mtvec force-pushed the jitlink-riscv-ehframe branch from d21b83e to 8fec2d7 Compare September 13, 2023 08:26

mtvec force-pushed the jitlink-riscv-ehframe branch from 8fec2d7 to 836d7e7 Compare September 13, 2023 12:21

asb mentioned this pull request Oct 4, 2023

[clang-repl] Disable InterpreterExceptionTest on RISC-V #68216

Merged

hahnjo mentioned this pull request Oct 4, 2023

[JITLink][RISCV] Implement eh_frame handling #68253

Merged

mtvec closed this Oct 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JITLink][RISCV] Implement .eh_frame handling #66067

[JITLink][RISCV] Implement .eh_frame handling #66067

mtvec commented Sep 12, 2023

llvmbot commented Sep 12, 2023

Besides this, two new edge kinds needed to be implemented for RISC-V: `Delta64` and `NegDelta32`

mtvec commented Sep 13, 2023

mtvec commented Sep 13, 2023

lhames commented Sep 18, 2023

asb commented Sep 27, 2023

hahnjo commented Oct 4, 2023

hahnjo commented Oct 29, 2023

mtvec commented Oct 30, 2023

[JITLink][RISCV] Implement .eh_frame handling #66067

[JITLink][RISCV] Implement .eh_frame handling #66067

Conversation

mtvec commented Sep 12, 2023

llvmbot commented Sep 12, 2023

Besides this, two new edge kinds needed to be implemented for RISC-V: Delta64 and NegDelta32

mtvec commented Sep 13, 2023

mtvec commented Sep 13, 2023

lhames commented Sep 18, 2023

asb commented Sep 27, 2023

hahnjo commented Oct 4, 2023

hahnjo commented Oct 29, 2023

mtvec commented Oct 30, 2023

Besides this, two new edge kinds needed to be implemented for RISC-V: `Delta64` and `NegDelta32`