Re: [Tarantool-patches] [PATCH luajit 1/2] Handle partial snapshot restore due to stack overflow.

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

Hi, Sergey!
Thanks for the review!
Fixed your comment and force-pushed the branch.

On 10.06.25, Sergey Bronnikov wrote:
> Hello, Sergey!
> 
> Thanks for the patch! LGTM with a minor below.
> 
> Sergey
> 
> On 6/10/25 13:28, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Reported by pwnhacker0x18. Fixed by Peter Cawley.
> >
> > (cherry picked from commit 811c5322c8ab6bdbb6784cd43aa57041a1cc9360)
> >
> > `lj_snap_restore()` restores the PC for the inner cframe, but not the
> > outer (before the protected call to the `trace_exit_cp()`). If the stack
> > overflow is observed during the further snapshot restoration, it doesn't
> > fix up the outer cframe's PC. After that, in the following error
> > rethrowing from the right C frame, in case of error handler set, the
> > stack overflow error may be raised again, and with an incorrect value of
> > the PC for that frame, it leads to the crash in the `debug_framepc()`.
> >
> > This patch prevents it by inserting the special pseudo-valid value `L`.
> > Unfortunately, this leads to the uninitialized reads by the
> > `debug_framepc()` (by the address `L - 4`), if the error handler
> > observes the resulted PC. This will be fixed in the next patch.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> >
> > Part of tarantool/tarantool#11278
> > ---

<snipped>

> > +-- This function starts the first trace.
> > +local function recursive_f()
> > +  -- Function with the single result to cause the trace stitching.
> > +  tonumber('')
> > +  -- Prereserved stack space before the call.
> > +  -- luacheck: no unused
> > +  local _, _, _, _, _, _, _, _, _, _, _
> > +  -- Link from the stitched trace to the parent one.
> > +  recursive_f()
> > +  -- Additional stack required for the snapshot restoration.
> /stack/stack space/?

Rephrased:

===================================================================
diff --git a/test/tarantool-tests/lj-1196-partial-snap-restore.test.lua b/test/tarantool-tests/lj-1196-partial-snap-restore.test.lua
index 8ee8f673..4ab78d31 100644
--- a/test/tarantool-tests/lj-1196-partial-snap-restore.test.lua
+++ b/test/tarantool-tests/lj-1196-partial-snap-restore.test.lua
@@ -31,7 +31,7 @@ local function recursive_f()
   local _, _, _, _, _, _, _, _, _, _, _
   -- Link from the stitched trace to the parent one.
   recursive_f()
-  -- Additional stack required for the snapshot restoration.
+  -- Additional stack space required for the snapshot restoration.
   -- luacheck: no unused
   local _, _, _
 end
===================================================================

> > +  -- luacheck: no unused
> > +  local _, _, _
> > +end
> > +
> > +-- Use coroutine wrap for the fixed stack size at the start.
> > +coroutine.wrap(function()
> > +  -- XXX: Special stack slot offset.
> > +  -- luacheck: no unused
> > +  local _, _, _, _, _, _, _, _, _, _
> > +  -- The error is observed only if we have the error handler set,
> > +  -- since we try to resize stack for its call.
> > +  xpcall(recursive_f, function() end)
> > +end)()
> > +
> > +test:ok(true, 'no crash during snapshot restoring')
> > +
> > +test:done(true)

-- 
Best regards,
Sergey Kaplun