Hi, Sergey,
LGTM
Hi, Sergey! Thanks for the review! Please, consider my answer below. On 10.06.25, Sergey Bronnikov wrote:Hello, Sergey! Thanks for the patch! See a comment below. Sergey On 6/10/25 13:28, Sergey Kaplun wrote:From: Mike Pall <mike> Reported by Junlong Li. Fixed by Peter Cawley. (cherry picked from commit 86e7123bb1782a5f200ba5e83b8c4f3fbad4f7bc) This patch is a follow-up to the previous commit, which leads to a dirty read of the pseudo-valid PC set for the cframe on snapshot restoration. To avoid these dirty reads, this patch sets the PC to the outer frame as well before possible error throwing. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#11278 --- src/lj_snap.c | 3 +- src/lj_trace.c | 4 +- ...-1196-stack-overflow-snap-restore.test.lua | 65 +++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua diff --git a/src/lj_snap.c b/src/lj_snap.c index 8d7bd868..4cfae579 100644 --- a/src/lj_snap.c +++ b/src/lj_snap.c<snipped>diff --git a/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua new file mode 100644 index 00000000..942d1f82 --- /dev/null +++ b/test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua @@ -0,0 +1,65 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT dirty reads after stack +-- overflow during restoration from the snapshot. +-- The test fails before the patch under Valgrind.Please specify valgrind option that is required for reproducing the bug.This just mean the default valgrind settings (without any flags).
With this command-line the bug is reproduced:
cmake -S . -B build -DLUAJIT_USE_VALGRIND=ON
-DLUAJIT_USE_ASAN=OFF -DLUAJIT_USE_SYSMALLOC=ON
-DLUAJIT_ENABLE_GC64=ONĀ -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON
-DCMAKE_BUILD_TYPE=Debug
cd build && ctest -V -R test/tarantool-tests/lj-1196-stack-overflow-snap-restore.test.lua -V
Cannot reproduce with command line below with reverted patch: VALGRIND_OPTS="--leak-check=no --malloc-fill=0x00 --free-fill=0x00" ctest -V -R test/tarantool-tests/lj-1196-partial-snap-restore.test.lua -VThis doesn't fail due to --leadk-check=no flag. Also, I suppose this should fail under ASan.+-- +-- luacheck: push no max_comment_line_length +-- +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1196, +--https://www.freelists.org/post/luajit/Invalid-read-found-by-valgrind. +-- +-- luacheck: pop +<snipped>+test:done(true)