Hi, Sergey,

thanks for the patch!

Please add a message "Also, it removes all related UBSAN suppressions, since they are fixed."

to commit message, like you did for the second patch.


LGTM

On 25.06.2024 18:54, Sergey Kaplun wrote:
From: Mike Pall <mike>

Thanks to Sergey Kaplun.

(cherry picked from commit 4a22050df9e76a28ef904382e4b4c69578973cd5)

When saving FPR registers during while from a trace and restoring
data from a snapshot, UB sanitizer produces the following warning:
| lj_snap.c:804:32: runtime error: index 23 out of bounds for type 'intptr_t [16]'

due to indexing `ex->gpr` with a fpr register, whose number is >=
`RID_MAX_GPR`. The situation itself is harmless since this is read from
`spill[256]` array and is rewritten in the next if branch.

This patch fixes the out-of-bounds access to read from `ex->gpr` only
conditionally.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#9924
Relates to tarantool/tarantool#8473
---
 src/lj_snap.c                                 | 13 +++------
 ...93-out-of-bounds-snap-restoredata.test.lua | 28 +++++++++++++++++++
 2 files changed, 32 insertions(+), 9 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua

diff --git a/src/lj_snap.c b/src/lj_snap.c
index 7dc4fe35..8a33dc22 100644
--- a/src/lj_snap.c
+++ b/src/lj_snap.c
@@ -756,13 +756,6 @@ static void snap_restoreval(jit_State *J, GCtrace *T, ExitState *ex,
 }
 
 #if LJ_HASFFI
-# if LUAJIT_USE_UBSAN
-/* See https://github.com/LuaJIT/LuaJIT/issues/1193. */
-static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
-			     SnapNo snapno, BloomFilter rfilt,
-			     IRRef ref, void *dst, CTSize sz)
-  __attribute__((no_sanitize("bounds")));
-# endif
 /* Restore raw data from the trace exit state. */
 static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
 			     SnapNo snapno, BloomFilter rfilt,
@@ -801,7 +794,6 @@ static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
 	*(lua_Number *)dst = (lua_Number)*(int32_t *)dst;
 	return;
       }
-      src = (int32_t *)&ex->gpr[r-RID_MIN_GPR];
 #if !LJ_SOFTFP
       if (r >= RID_MAX_GPR) {
 	src = (int32_t *)&ex->fpr[r-RID_MIN_FPR];
@@ -815,7 +807,10 @@ static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
 #endif
       } else
 #endif
-      if (LJ_64 && LJ_BE && sz == 4) src++;
+      {
+	src = (int32_t *)&ex->gpr[r-RID_MIN_GPR];
+	if (LJ_64 && LJ_BE && sz == 4) src++;
+      }
     }
   }
   lj_assertJ(sz == 1 || sz == 2 || sz == 4 || sz == 8,
diff --git a/test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua b/test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua
new file mode 100644
index 00000000..6c5fc3f6
--- /dev/null
+++ b/test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua
@@ -0,0 +1,28 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT's out-of-bounds access during
+-- the saving of registers content in `snap_restoredata()`.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1193.
+
+local test = tap.test('lj-1193-out-of-bounds-snap-restoredata'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+local ffi = require('ffi')
+
+test:plan(1)
+
+local double_type = ffi.typeof('double')
+
+jit.opt.start('hotloop=1')
+local x = 1LL
+for _ = 1, 4 do
+  -- `x` is saved in the fpr register and will be restored in the
+  -- `ex->fpr` during exit from the snapshot. But out-of-bounds
+  -- access is happening due to indexing `ex->gpr` occasionally.
+  x = double_type(x + 1)
+end
+
+test:ok(true, 'no out-of-bounds failure')
+
+test:done(true)