From 186e81ebe97bcb6808fa092388b143b0aea50f94 Mon Sep 17 00:00:00 2001
From: Afonso Bordado <afonsobordado@az8.co>
Date: Tue, 7 Mar 2023 12:18:20 +0000
Subject: [PATCH] riscv64: Fix underflow in call relocation handling

Under some test case layouts the call relocation
panicking with an underflow. Use `wrapping_sub` to
signal that this is expected.

The fuzzer took a while to generate such a test case.
And I can't introduce it as a regression test because
when running via the regular clif-util run tests the
layout is different and the test case passes!

I think this is because in the fuzzer we only add
one trampoline, while in clif-util we build trampolines
for each funcion in the file.

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>
---
 cranelift/jit/src/compiled_blob.rs | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/cranelift/jit/src/compiled_blob.rs b/cranelift/jit/src/compiled_blob.rs
index 7c60454e7909..652f4bd9707a 100644
--- a/cranelift/jit/src/compiled_blob.rs
+++ b/cranelift/jit/src/compiled_blob.rs
@@ -120,10 +120,18 @@ impl CompiledBlob {
                     // See https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#pc-relative-symbol-addresses
                     // for a better explanation of the following code.
                     //
-                    // Unlike the regular symbol relocations, here both "sub-relocations" point
-                    // to the same address.
+                    // Unlike the regular symbol relocations, here both "sub-relocations" point to the same address.
+                    //
+                    // `pcrel` is a signed value (+/- 2GiB range), when splitting it into two parts, we need to
+                    // ensure that `hi20` is close enough to `pcrel` to be able to add `lo12` to it and still
+                    // get a valid address.
+                    //
+                    // `lo12` is also a signed offset (+/- 2KiB range) relative to the `hi20` value.
+                    //
+                    // `hi20` should also be shifted right to be the "true" value. But we also need it
+                    // left shifted for the `lo12` calculation and it also matches the instruction encoding.
                     let hi20 = pcrel.wrapping_add(0x800) & 0xFFFFF000;
-                    let lo12 = (pcrel - hi20) & 0xFFF;
+                    let lo12 = pcrel.wrapping_sub(hi20) & 0xFFF;
 
                     unsafe {
                         // Do a R_RISCV_PCREL_HI20 on the `auipc`