[llvm] 6a14a56 - [JITLink][RISCV] ADD/SUB relocs: read value from working memory

[Job Noorman via llvm-commits]

Author: Job Noorman
Date: 2023-04-07T09:47:49+02:00
New Revision: 6a14a56aaf77c615f9e4ab77b35029b0e6c8909f

URL: https://github.com/llvm/llvm-project/commit/6a14a56aaf77c615f9e4ab77b35029b0e6c8909f
DIFF: https://github.com/llvm/llvm-project/commit/6a14a56aaf77c615f9e4ab77b35029b0e6c8909f.diff

LOG: [JITLink][RISCV] ADD/SUB relocs: read value from working memory

The various ADD/SUB relocations work by reading the current value the
relocation points to, transforming it, and then writing it back to
memory. While the current implementation writes the value back to
working memory, it reads the current value from the execution address of
the relocation. This causes at least wrong results, but often crashes,
when the addresses of working memory are not equal to execution
addresses. This patch fixes this by reading the current value from
working memory.

Reviewed By: lhames

Differential Revision: https://reviews.llvm.org/D147693

Added: 
    

Modified: 
    llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
    llvm/test/ExecutionEngine/JITLink/RISCV/riscv_reloc_add.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
index e89d72a553d32..0622b6e307a9d 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
@@ -322,63 +322,52 @@ class ELFJITLinker_riscv : public JITLinker<ELFJITLinker_riscv> {
     case R_RISCV_ADD8: {
       int64_t Value =
           (E.getTarget().getAddress() +
-           *(reinterpret_cast<const uint8_t *>(FixupAddress.getValue())) +
-           E.getAddend())
+           *(reinterpret_cast<const uint8_t *>(FixupPtr)) + E.getAddend())
               .getValue();
       *FixupPtr = static_cast<uint8_t>(Value);
       break;
     }
     case R_RISCV_ADD16: {
       int64_t Value = (E.getTarget().getAddress() +
-                       support::endian::read16le(reinterpret_cast<const void *>(
-                           FixupAddress.getValue())) +
-                       E.getAddend())
+                       support::endian::read16le(FixupPtr) + E.getAddend())
                           .getValue();
       *(little16_t *)FixupPtr = static_cast<uint16_t>(Value);
       break;
     }
     case R_RISCV_ADD32: {
       int64_t Value = (E.getTarget().getAddress() +
-                       support::endian::read32le(reinterpret_cast<const void *>(
-                           FixupAddress.getValue())) +
-                       E.getAddend())
+                       support::endian::read32le(FixupPtr) + E.getAddend())
                           .getValue();
       *(little32_t *)FixupPtr = static_cast<uint32_t>(Value);
       break;
     }
     case R_RISCV_ADD64: {
       int64_t Value = (E.getTarget().getAddress() +
-                       support::endian::read64le(reinterpret_cast<const void *>(
-                           FixupAddress.getValue())) +
-                       E.getAddend())
+                       support::endian::read64le(FixupPtr) + E.getAddend())
                           .getValue();
       *(little64_t *)FixupPtr = static_cast<uint64_t>(Value);
       break;
     }
     case R_RISCV_SUB8: {
-      int64_t Value =
-          *(reinterpret_cast<const uint8_t *>(FixupAddress.getValue())) -
-          E.getTarget().getAddress().getValue() - E.getAddend();
+      int64_t Value = *(reinterpret_cast<const uint8_t *>(FixupPtr)) -
+                      E.getTarget().getAddress().getValue() - E.getAddend();
       *FixupPtr = static_cast<uint8_t>(Value);
       break;
     }
     case R_RISCV_SUB16: {
-      int64_t Value = support::endian::read16le(reinterpret_cast<const void *>(
-                          FixupAddress.getValue())) -
+      int64_t Value = support::endian::read16le(FixupPtr) -
                       E.getTarget().getAddress().getValue() - E.getAddend();
       *(little16_t *)FixupPtr = static_cast<uint32_t>(Value);
       break;
     }
     case R_RISCV_SUB32: {
-      int64_t Value = support::endian::read32le(reinterpret_cast<const void *>(
-                          FixupAddress.getValue())) -
+      int64_t Value = support::endian::read32le(FixupPtr) -
                       E.getTarget().getAddress().getValue() - E.getAddend();
       *(little32_t *)FixupPtr = static_cast<uint32_t>(Value);
       break;
     }
     case R_RISCV_SUB64: {
-      int64_t Value = support::endian::read64le(reinterpret_cast<const void *>(
-                          FixupAddress.getValue())) -
+      int64_t Value = support::endian::read64le(FixupPtr) -
                       E.getTarget().getAddress().getValue() - E.getAddend();
       *(little64_t *)FixupPtr = static_cast<uint64_t>(Value);
       break;
@@ -419,8 +408,7 @@ class ELFJITLinker_riscv : public JITLinker<ELFJITLinker_riscv> {
       break;
     }
     case R_RISCV_SUB6: {
-      int64_t Value =
-          *(reinterpret_cast<const uint8_t *>(FixupAddress.getValue())) & 0x3f;
+      int64_t Value = *(reinterpret_cast<const uint8_t *>(FixupPtr)) & 0x3f;
       Value -= E.getTarget().getAddress().getValue() - E.getAddend();
       *FixupPtr = (*FixupPtr & 0xc0) | (static_cast<uint8_t>(Value) & 0x3f);
       break;

diff  --git a/llvm/test/ExecutionEngine/JITLink/RISCV/riscv_reloc_add.s b/llvm/test/ExecutionEngine/JITLink/RISCV/riscv_reloc_add.s
index c334ce4857dbe..13689b6d8a026 100644
--- a/llvm/test/ExecutionEngine/JITLink/RISCV/riscv_reloc_add.s
+++ b/llvm/test/ExecutionEngine/JITLink/RISCV/riscv_reloc_add.s
@@ -1,8 +1,10 @@
 # RUN: rm -rf %t && mkdir -p %t
 # RUN: llvm-mc -triple=riscv64 -filetype=obj -o %t/riscv64_reloc_add.o %s
 # RUN: llvm-mc -triple=riscv32 -filetype=obj -o %t/riscv32_reloc_add.o %s
-# RUN: llvm-jitlink -noexec -check %s %t/riscv64_reloc_add.o
-# RUN: llvm-jitlink -noexec -check %s %t/riscv32_reloc_add.o
+# RUN: llvm-jitlink -noexec -check %s %t/riscv64_reloc_add.o \
+# RUN:     -slab-allocate=1Mb -slab-address=0x1000 -slab-page-size=0x1000
+# RUN: llvm-jitlink -noexec -check %s %t/riscv32_reloc_add.o \
+# RUN:     -slab-allocate=1Mb -slab-address=0x1000 -slab-page-size=0x1000
 
 # jitlink-check: *{8}(named_data) = 0x8
 # jitlink-check: *{4}(named_data+8) = 0x8