Hi, Sergey,

thanks for the patch! LGTM

Sergey

On 8/19/25 20:11, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Sergey Kaplun.
>
> (cherry picked from commit e3fa3c48d8a4aadcf86429e9f7f6f1171914b15a)
>
> In case when the saved PC in the snapshot is the first (0th index) PC in
> the prototype like JFUNC*, the subtraction to determine the previous PC
> in the `debug_framepc()` overflows and contains `NO_BCPOS` value. After
> that, the pos is greater than sizebc. Hence, the code below may
> interpret the bits in `pt->varinfo` like `bc_isret()` and assign an
> invalid value to `pos` to be returned. Further, it may lead to the
> assertion failure in the lj_debug_frameline().
>
> This patch fixes it by pretending that this means the first non-header
> bytecode in the prototype. Also, this patch removes the skipcond
> introduced in the commit a74e5be07d54b4e98b85493de73317db520b3f71
> ("test: conditionally disable flaky lj-1196"). The new test isn't added
> since the assertion failure depends on the specific memory address of
> the `varinfo`, so it is too hard to create a stable reproducer.
>
> Sergey Kaplun:
> * added the description for the problem
>
> Part of tarantool/tarantool#11691
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1369-stackov-invalid-bc
> Related issues:
> *https://github.com/tarantool/tarantool/issues/11691
> *https://github.com/LuaJIT/LuaJIT/issues/1369
> *https://github.com/LuaJIT/LuaJIT/issues/1359
> *https://github.com/LuaJIT/LuaJIT/issues/1196
>
>   src/lj_debug.c                                         |  1 +
>   .../lj-1196-partial-snap-restore.test.lua              | 10 +---------
>   2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/src/lj_debug.c b/src/lj_debug.c
> index 76e48aca..bc057cf6 100644
> --- a/src/lj_debug.c
> +++ b/src/lj_debug.c
> @@ -101,6 +101,7 @@ static BCPos debug_framepc(lua_State *L, GCfunc *fn, cTValue *nextframe)
>     pt = funcproto(fn);
>     pos = proto_bcpos(pt, ins) - 1;
>   #if LJ_HASJIT
> +  if (pos == NO_BCPOS) return 1;  /* Pretend it's the first bytecode. */
>     if (pos > pt->sizebc) {  /* Undo the effects of lj_trace_exit for JLOOP. */
>       if (bc_isret(bc_op(ins[-1]))) {
>         GCtrace *T = (GCtrace *)((char *)(ins-1) - offsetof(GCtrace, startins));
> 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 5199ca00..a74f97bd 100644
> --- a/test/tarantool-tests/lj-1196-partial-snap-restore.test.lua
> +++ b/test/tarantool-tests/lj-1196-partial-snap-restore.test.lua
> @@ -4,15 +4,7 @@ local tap = require('tap')
>   -- in case of the stack overflow.
>   -- See also:https://github.com/LuaJIT/LuaJIT/issues/1196.
>   
> -local test = tap.test('lj-1196-partial-snap-restore'):skipcond({
> -  -- Disable test for Tarantool to avoid failures, see also:
> -  --https://github.com/LuaJIT/LuaJIT/issues/1369.
> -  ['Disabled for Tarantool due to lj-1369'] = _TARANTOOL,
> -  -- Also, it may fail on some non-arm64 runners stable after
> -  -- adding the skip condition above.
> -  ['Disabled for x86/x64 due to lj-1369'] = jit.arch ~= 'arm64',
> -})
> -
> +local test = tap.test('lj-1196-partial-snap-restore')
>   test:plan(1)
>   
>   -- XXX: The reproducer below uses several stack slot offsets to