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
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)