Hi, Sergey,

thanks for the patch! LGTM

Sergey

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