Re: [Tarantool-patches] [PATCH luajit 2/2] Different fix for partial snapshot restore due to stack overflow.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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

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

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

-- 
Best regards,
Sergey Kaplun