[Tarantool-patches] [PATCH luajit 0/2] VM events and finalizers in separate state

[Tarantool-patches] [PATCH luajit 0/2] VM events and finalizers in separate state

[Sergey Kaplun via Tarantool-patches]

This patchset fixes the corner case of VM event stack overflow, which
results in broken JIT recording. The second patch is a fixup for the
first one.

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1403-vmevent-crash-on-stkov
Related issues:
* https://github.com/LuaJIT/LuaJIT/issues/1403
* https://github.com/LuaJIT/LuaJIT/issues/1445
* https://github.com/tarantool/tarantool/issues/12134

Mike Pall (2):
  Run VM events and finalizers in separate state.
  Fix VM event error handling for finalizers.

 src/lj_gc.c                                   | 22 +++--
 src/lj_obj.h                                  |  2 +
 src/lj_parse.c                                |  4 +-
 src/lj_state.c                                |  1 +
 src/lj_trace.c                                | 91 +++++++++----------
 src/lj_vmevent.c                              |  5 +
 src/lj_vmevent.h                              | 22 +++--
 test/LuaJIT-tests/lang/gc_debug.lua           | 11 +--
 .../lj-1403-vmevent-crash-on-stkov.test.lua   | 47 ++++++++++
 .../lj-1445-errfin-errmsg.test.lua            | 27 ++++++
 10 files changed, 154 insertions(+), 78 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
 create mode 100644 test/tarantool-tests/lj-1445-errfin-errmsg.test.lua

-- 
2.53.0

[Tarantool-patches] [PATCH luajit 1/2] Run VM events and finalizers in separate state.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit 5c647754a687a910ef40a097fbf8f7415561c8aa)

If `lj_state_checkstack()` in the `lj_vmevent_prepare()` raises the
stack overflow error, `J->cur.startpc` is uninitialized during the
recording, and its further inspection during trace blacklisting leads to
the crash.

To prevent any further inconsistencies in the JIT state due to the VM
event handlers, all these handlers run in the separate `lua_State`
(named V), which is stored in the `global_State`.

Now the debugging in GC finalizers is less verbose. Hence,
`debug.getinfo(2)` will return `nil`, since the finalizer is called on
the separate Lua thread without main context.

Also, the VM event `errfin` handler invocation is broken and will be
fixed in the next commit.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#12134
---
 src/lj_gc.c                                   | 18 ++--
 src/lj_obj.h                                  |  2 +
 src/lj_parse.c                                |  4 +-
 src/lj_state.c                                |  1 +
 src/lj_trace.c                                | 91 +++++++++----------
 src/lj_vmevent.c                              |  5 +
 src/lj_vmevent.h                              | 22 +++--
 test/LuaJIT-tests/lang/gc_debug.lua           | 11 +--
 .../lj-1403-vmevent-crash-on-stkov.test.lua   | 47 ++++++++++
 9 files changed, 124 insertions(+), 77 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua

diff --git a/src/lj_gc.c b/src/lj_gc.c
index 3142482f..83ad9ee9 100644
--- a/src/lj_gc.c
+++ b/src/lj_gc.c
@@ -97,6 +97,7 @@ static void gc_mark_start(global_State *g)
   setgcrefnull(g->gc.weak);
   gc_markobj(g, mainthread(g));
   gc_markobj(g, tabref(mainthread(g)->env));
+  gc_markobj(g, vmthread(g));
   gc_marktv(g, &g->registrytv);
   gc_mark_gcroot(g);
   g->gc.state = GCSpropagate;
@@ -501,24 +502,25 @@ static void gc_call_finalizer(global_State *g, lua_State *L,
   uint8_t oldh = hook_save(g);
   GCSize oldt = g->gc.threshold;
   int errcode;
+  lua_State *VL = vmthread(g);
   TValue *top;
   lj_trace_abort(g);
   hook_entergc(g);  /* Disable hooks and new traces during __gc. */
   if (LJ_HASPROFILE && (oldh & HOOK_PROFILE)) lj_dispatch_update(g);
   g->gc.threshold = LJ_MAX_MEM;  /* Prevent GC steps. */
-  top = L->top;
-  copyTV(L, top++, mo);
+  top = VL->top;
+  copyTV(VL, top++, mo);
   if (LJ_FR2) setnilV(top++);
-  setgcV(L, top, o, ~o->gch.gct);
-  L->top = top+1;
-  errcode = lj_vm_pcall(L, top, 1+0, -1);  /* Stack: |mo|o| -> | */
+  setgcV(VL, top, o, ~o->gch.gct);
+  VL->top = top+1;
+  errcode = lj_vm_pcall(VL, top, 1+0, -1);  /* Stack: |mo|o| -> | */
+  setgcref(g->cur_L, obj2gco(L));
   hook_restore(g, oldh);
   if (LJ_HASPROFILE && (oldh & HOOK_PROFILE)) lj_dispatch_update(g);
   g->gc.threshold = oldt;  /* Restore GC threshold. */
   if (errcode) {
-    ptrdiff_t errobj = savestack(L, L->top-1);  /* Stack may be resized. */
-    lj_vmevent_send(L, ERRFIN,
-      copyTV(L, L->top++, restorestack(L, errobj));
+    lj_vmevent_send(g, ERRFIN,
+      copyTV(V, V->top++, L->top-1);
     );
     L->top--;
   }
diff --git a/src/lj_obj.h b/src/lj_obj.h
index 06ea0cd0..1a6e9488 100644
--- a/src/lj_obj.h
+++ b/src/lj_obj.h
@@ -672,6 +672,7 @@ typedef struct global_State {
   GCupval uvhead;	/* Head of double-linked list of all open upvalues. */
   volatile int32_t vmstate;  /* VM state or current JIT code trace number. */
   int32_t hookcstart;	/* Start count for instruction hook counter. */
+  GCRef vmthref;	/* Link to VM thread. */
   lua_Hook hookf;	/* Hook function. */
   lua_CFunction wrapf;	/* Wrapper for C function calls. */
   lua_CFunction panic;	/* Called as a last resort for errors. */
@@ -688,6 +689,7 @@ typedef struct global_State {
 } global_State;
 
 #define mainthread(g)	(&gcref(g->mainthref)->th)
+#define vmthread(g)	(&gcref(g->vmthref)->th)
 #define niltv(L) \
   check_exp(tvisnil(&G(L)->nilnode.val), &G(L)->nilnode.val)
 #define niltvg(g) \
diff --git a/src/lj_parse.c b/src/lj_parse.c
index 88108c01..9863f2c6 100644
--- a/src/lj_parse.c
+++ b/src/lj_parse.c
@@ -1595,8 +1595,8 @@ static GCproto *fs_finish(LexState *ls, BCLine line)
   fs_fixup_line(fs, pt, (void *)((char *)pt + ofsli), numline);
   fs_fixup_var(ls, pt, (uint8_t *)((char *)pt + ofsdbg), ofsvar);
 
-  lj_vmevent_send(L, BC,
-    setprotoV(L, L->top++, pt);
+  lj_vmevent_send(G(L), BC,
+    setprotoV(V, V->top++, pt);
   );
 
   /* Add a new prototype to the profiler. */
diff --git a/src/lj_state.c b/src/lj_state.c
index 8a1acf87..133a86a2 100644
--- a/src/lj_state.c
+++ b/src/lj_state.c
@@ -208,6 +208,7 @@ static TValue *cpluaopen(lua_State *L, lua_CFunction dummy, void *ud)
 #endif
   lj_trace_initstate(g);
   lj_err_verify();
+  setgcref(g->vmthref, obj2gco(lj_state_new(L)));
   return NULL;
 }
 
diff --git a/src/lj_trace.c b/src/lj_trace.c
index 0d1d233a..0dfbfa9f 100644
--- a/src/lj_trace.c
+++ b/src/lj_trace.c
@@ -309,8 +309,8 @@ int lj_trace_flushall(lua_State *L)
   /* Free the whole machine code and invalidate all exit stub groups. */
   lj_mcode_free(J);
   memset(J->exitstubgroup, 0, sizeof(J->exitstubgroup));
-  lj_vmevent_send(L, TRACE,
-    setstrV(L, L->top++, lj_str_newlit(L, "flush"));
+  lj_vmevent_send(J2G(J), TRACE,
+    setstrV(V, V->top++, lj_str_newlit(V, "flush"));
   );
   return 0;
 }
@@ -416,7 +416,6 @@ setpenalty:
 /* Start tracing. */
 static void trace_start(jit_State *J)
 {
-  lua_State *L;
   TraceNo traceno;
 
   if ((J->pt->flags & PROTO_NOJIT)) {  /* JIT disabled for this proto? */
@@ -460,20 +459,19 @@ static void trace_start(jit_State *J)
   J->ktrace = 0;
   setgcref(J->cur.startpt, obj2gco(J->pt));
 
-  L = J->L;
-  lj_vmevent_send(L, TRACE,
-    setstrV(L, L->top++, lj_str_newlit(L, "start"));
-    setintV(L->top++, traceno);
-    setfuncV(L, L->top++, J->fn);
-    setintV(L->top++, proto_bcpos(J->pt, J->pc));
+  lj_vmevent_send(J2G(J), TRACE,
+    setstrV(V, V->top++, lj_str_newlit(V, "start"));
+    setintV(V->top++, traceno);
+    setfuncV(V, V->top++, J->fn);
+    setintV(V->top++, proto_bcpos(J->pt, J->pc));
     if (J->parent) {
-      setintV(L->top++, J->parent);
-      setintV(L->top++, J->exitno);
+      setintV(V->top++, J->parent);
+      setintV(V->top++, J->exitno);
     } else {
       BCOp op = bc_op(*J->pc);
       if (op == BC_CALLM || op == BC_CALL || op == BC_ITERC) {
-	setintV(L->top++, J->exitno);  /* Parent of stitched trace. */
-	setintV(L->top++, -1);
+	setintV(V->top++, J->exitno);  /* Parent of stitched trace. */
+	setintV(V->top++, -1);
       }
     }
   );
@@ -488,7 +486,6 @@ static void trace_stop(jit_State *J)
   GCproto *pt = &gcref(J->cur.startpt)->pt;
   TraceNo traceno = J->cur.traceno;
   GCtrace *T = J->curfinal;
-  lua_State *L;
 
   switch (op) {
   case BC_FORL:
@@ -544,11 +541,10 @@ static void trace_stop(jit_State *J)
   J->postproc = LJ_POST_NONE;
   trace_save(J, T);
 
-  L = J->L;
-  lj_vmevent_send(L, TRACE,
-    setstrV(L, L->top++, lj_str_newlit(L, "stop"));
-    setintV(L->top++, traceno);
-    setfuncV(L, L->top++, J->fn);
+  lj_vmevent_send(J2G(J), TRACE,
+    setstrV(V, V->top++, lj_str_newlit(V, "stop"));
+    setintV(V->top++, traceno);
+    setfuncV(V, V->top++, J->fn);
   );
 }
 
@@ -605,18 +601,17 @@ static int trace_abort(jit_State *J)
   /* Is there anything to abort? */
   traceno = J->cur.traceno;
   if (traceno) {
-    ptrdiff_t errobj = savestack(L, L->top-1);  /* Stack may be resized. */
     J->cur.link = 0;
     J->cur.linktype = LJ_TRLINK_NONE;
-    lj_vmevent_send(L, TRACE,
+    lj_vmevent_send(J2G(J), TRACE,
       cTValue *bot = tvref(L->stack)+LJ_FR2;
       cTValue *frame;
       const BCIns *pc;
       BCPos pos = 0;
-      setstrV(L, L->top++, lj_str_newlit(L, "abort"));
-      setintV(L->top++, traceno);
+      setstrV(V, V->top++, lj_str_newlit(V, "abort"));
+      setintV(V->top++, traceno);
       /* Find original Lua function call to generate a better error message. */
-      for (frame = J->L->base-1, pc = J->pc; ; frame = frame_prev(frame)) {
+      for (frame = L->base-1, pc = J->pc; ; frame = frame_prev(frame)) {
 	if (isluafunc(frame_func(frame))) {
 	  pos = proto_bcpos(funcproto(frame_func(frame)), pc);
 	  break;
@@ -628,10 +623,10 @@ static int trace_abort(jit_State *J)
 	  pc = frame_pc(frame) - 1;
 	}
       }
-      setfuncV(L, L->top++, frame_func(frame));
-      setintV(L->top++, pos);
-      copyTV(L, L->top++, restorestack(L, errobj));
-      copyTV(L, L->top++, &J->errinfo);
+      setfuncV(V, V->top++, frame_func(frame));
+      setintV(V->top++, pos);
+      copyTV(V, V->top++, L->top-1);
+      copyTV(V, V->top++, &J->errinfo);
     );
     /* Drop aborted trace after the vmevent (which may still access it). */
     setgcrefnull(J->trace[traceno]);
@@ -678,16 +673,16 @@ static TValue *trace_state(lua_State *L, lua_CFunction dummy, void *ud)
     case LJ_TRACE_RECORD:
       trace_pendpatch(J, 0);
       setvmstate(J2G(J), RECORD);
-      lj_vmevent_send_(L, RECORD,
+      lj_vmevent_send_(J2G(J), RECORD,
 	/* Save/restore state for trace recorder. */
 	TValue savetv = J2G(J)->tmptv;
 	TValue savetv2 = J2G(J)->tmptv2;
 	TraceNo parent = J->parent;
 	ExitNo exitno = J->exitno;
-	setintV(L->top++, J->cur.traceno);
-	setfuncV(L, L->top++, J->fn);
-	setintV(L->top++, J->pt ? (int32_t)proto_bcpos(J->pt, J->pc) : -1);
-	setintV(L->top++, J->framedepth);
+	setintV(V->top++, J->cur.traceno);
+	setfuncV(V, V->top++, J->fn);
+	setintV(V->top++, J->pt ? (int32_t)proto_bcpos(J->pt, J->pc) : -1);
+	setintV(V->top++, J->framedepth);
       ,
 	J2G(J)->tmptv = savetv;
 	J2G(J)->tmptv2 = savetv2;
@@ -837,23 +832,23 @@ static TValue *trace_exit_gc_cp(lua_State *L, lua_CFunction dummy, void *unused)
 
 #ifndef LUAJIT_DISABLE_VMEVENT
 /* Push all registers from exit state. */
-static void trace_exit_regs(lua_State *L, ExitState *ex)
+static void trace_exit_regs(lua_State *V, ExitState *ex)
 {
   int32_t i;
-  setintV(L->top++, RID_NUM_GPR);
-  setintV(L->top++, RID_NUM_FPR);
+  setintV(V->top++, RID_NUM_GPR);
+  setintV(V->top++, RID_NUM_FPR);
   for (i = 0; i < RID_NUM_GPR; i++) {
     if (sizeof(ex->gpr[i]) == sizeof(int32_t))
-      setintV(L->top++, (int32_t)ex->gpr[i]);
+      setintV(V->top++, (int32_t)ex->gpr[i]);
     else
-      setnumV(L->top++, (lua_Number)ex->gpr[i]);
+      setnumV(V->top++, (lua_Number)ex->gpr[i]);
   }
 #if !LJ_SOFTFP
   for (i = 0; i < RID_NUM_FPR; i++) {
-    setnumV(L->top, ex->fpr[i]);
-    if (LJ_UNLIKELY(tvisnan(L->top)))
-      setnanV(L->top);
-    L->top++;
+    setnumV(V->top, ex->fpr[i]);
+    if (LJ_UNLIKELY(tvisnan(V->top)))
+      setnanV(V->top);
+    V->top++;
   }
 #endif
 }
@@ -895,6 +890,8 @@ int LJ_FASTCALL lj_trace_exit(jit_State *J, void *exptr)
 
 #ifdef EXITSTATE_PCREG
   J->parent = trace_exit_find(J, (MCode *)(intptr_t)ex->gpr[EXITSTATE_PCREG]);
+#else
+  UNUSED(ex);
 #endif
   T = traceref(J, J->parent); UNUSED(T);
 #ifdef EXITSTATE_CHECKEXIT
@@ -915,11 +912,11 @@ int LJ_FASTCALL lj_trace_exit(jit_State *J, void *exptr)
   if (exitcode) copyTV(L, L->top++, &exiterr);  /* Anchor the error object. */
 
   if (!(LJ_HASPROFILE && (G(L)->hookmask & HOOK_PROFILE)))
-    lj_vmevent_send(L, TEXIT,
-      lj_state_checkstack(L, 4+RID_NUM_GPR+RID_NUM_FPR+LUA_MINSTACK);
-      setintV(L->top++, J->parent);
-      setintV(L->top++, J->exitno);
-      trace_exit_regs(L, ex);
+    lj_vmevent_send(G(L), TEXIT,
+      lj_state_checkstack(V, 4+RID_NUM_GPR+RID_NUM_FPR+LUA_MINSTACK);
+      setintV(V->top++, J->parent);
+      setintV(V->top++, J->exitno);
+      trace_exit_regs(V, ex);
     );
 
   pc = exd.pc;
diff --git a/src/lj_vmevent.c b/src/lj_vmevent.c
index 86640804..d85ece86 100644
--- a/src/lj_vmevent.c
+++ b/src/lj_vmevent.c
@@ -38,6 +38,7 @@ ptrdiff_t lj_vmevent_prepare(lua_State *L, VMEvent ev)
 void lj_vmevent_call(lua_State *L, ptrdiff_t argbase)
 {
   global_State *g = G(L);
+  lua_State *oldL = gco2th(gcref(g->cur_L));
   uint8_t oldmask = g->vmevmask;
   uint8_t oldh = hook_save(g);
   int status;
@@ -51,6 +52,10 @@ void lj_vmevent_call(lua_State *L, ptrdiff_t argbase)
     fputs(tvisstr(L->top) ? strVdata(L->top) : "?", stderr);
     fputc('\n', stderr);
   }
+  setgcref(g->cur_L, obj2gco(oldL));
+#if LJ_HASJIT
+  G2J(g)->L = oldL;
+#endif
   hook_restore(g, oldh);
   if (g->vmevmask != VMEVENT_NOCACHE)
     g->vmevmask = oldmask;  /* Restore event mask, but not if not modified. */
diff --git a/src/lj_vmevent.h b/src/lj_vmevent.h
index 56c0d798..5b60d96b 100644
--- a/src/lj_vmevent.h
+++ b/src/lj_vmevent.h
@@ -32,23 +32,25 @@ typedef enum {
 } VMEvent;
 
 #ifdef LUAJIT_DISABLE_VMEVENT
-#define lj_vmevent_send(L, ev, args)		UNUSED(L)
-#define lj_vmevent_send_(L, ev, args, post)	UNUSED(L)
+#define lj_vmevent_send(g, ev, args)		UNUSED(g)
+#define lj_vmevent_send_(g, ev, args, post)	UNUSED(g)
 #else
-#define lj_vmevent_send(L, ev, args) \
-  if (G(L)->vmevmask & VMEVENT_MASK(LJ_VMEVENT_##ev)) { \
-    ptrdiff_t argbase = lj_vmevent_prepare(L, LJ_VMEVENT_##ev); \
+#define lj_vmevent_send(g, ev, args) \
+  if ((g)->vmevmask & VMEVENT_MASK(LJ_VMEVENT_##ev)) { \
+    lua_State *V = vmthread(g); \
+    ptrdiff_t argbase = lj_vmevent_prepare(V, LJ_VMEVENT_##ev); \
     if (argbase) { \
       args \
-      lj_vmevent_call(L, argbase); \
+      lj_vmevent_call(V, argbase); \
     } \
   }
-#define lj_vmevent_send_(L, ev, args, post) \
-  if (G(L)->vmevmask & VMEVENT_MASK(LJ_VMEVENT_##ev)) { \
-    ptrdiff_t argbase = lj_vmevent_prepare(L, LJ_VMEVENT_##ev); \
+#define lj_vmevent_send_(g, ev, args, post) \
+  if ((g)->vmevmask & VMEVENT_MASK(LJ_VMEVENT_##ev)) { \
+    lua_State *V = vmthread(g); \
+    ptrdiff_t argbase = lj_vmevent_prepare(V, LJ_VMEVENT_##ev); \
     if (argbase) { \
       args \
-      lj_vmevent_call(L, argbase); \
+      lj_vmevent_call(V, argbase); \
       post \
     } \
   }
diff --git a/test/LuaJIT-tests/lang/gc_debug.lua b/test/LuaJIT-tests/lang/gc_debug.lua
index bb30adc1..83ab6074 100644
--- a/test/LuaJIT-tests/lang/gc_debug.lua
+++ b/test/LuaJIT-tests/lang/gc_debug.lua
@@ -1,12 +1,9 @@
-local caught, caught_line, caught_mm
+local caught
 
 local function gcmeta()
   if caught ~= "end" then
     -- This may point to the wrong instruction if in a JIT trace.
     -- But there's no guarantee if, when or where any GC steps occur.
-    local dbg = debug.getinfo(2)
-    caught_line = dbg.currentline
-    caught_mm = debug.getinfo(1).name
     caught = true
   end
 end
@@ -25,12 +22,6 @@ local function testgc(mm, f)
   if not caught then
     error(mm.." metamethod not called", 2)
   end
-  if type(caught_line) ~= "number" or caught_line < 0 then
-    error("bad linenumber in debug info", 2)
-  end
-  if caught_mm ~= mm then
-    error("bad name for metamethod in debug info", 2)
-  end
 end
 
 do --- Test __gc metamethod info
diff --git a/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
new file mode 100644
index 00000000..ad275e17
--- /dev/null
+++ b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
@@ -0,0 +1,47 @@
+local tap = require('tap')
+
+-- The test file to demonstrate LuaJIT crash during stack overflow
+-- in the VM event handle.
+-- See also, https://github.com/LuaJIT/LuaJIT/issues/1403.
+
+local test = tap.test('lj-1403-vmevent-crash-on-stkov'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+local jit_dump = require('jit.dump')
+
+-- XXX: Some specific stack usage without a stack top check by the
+-- Lua function header.
+local t = setmetatable({}, {__newindex = pcall, __call = type})
+-- luacheck: no unused
+local function prober(...)
+  -- Invokes `pcall(t, t, t)`.
+  t[t] = t
+end
+
+jit.opt.start('hotloop=1')
+-- Need the invocation of the VM event.
+jit_dump.start('i', '/dev/null')
+
+-- The code below causes the stack overflow in the VM event
+-- handler. The unwinding of the error breaks the JIT semantics
+-- and leads to a crash.
+local function looper()
+  local r = pcall(prober)
+  if not r then
+    local n = 1
+    while n < 3 do
+      prober(1, 2)
+      n = n + 1
+    end
+  end
+  looper()
+end
+
+pcall(coroutine.wrap(looper))
+
+test:ok(true, 'no crash')
+
+test:done(true)
-- 
2.53.0

Re: [Tarantool-patches] [PATCH luajit 1/2] Run VM events and finalizers in separate state.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

Hi, Sergey!

thanks for the patch! LGTM with a minor comment below.

Sergey

On 3/28/26 18:31, Sergey Kaplun wrote:


<snipped>

> diff --git a/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
> new file mode 100644
> index 00000000..ad275e17
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
> @@ -0,0 +1,47 @@
> +local tap = require('tap')
> +
> +-- The test file to demonstrate LuaJIT crash during stack overflow
> +-- in the VM event handle.
> +-- See also,https://github.com/LuaJIT/LuaJIT/issues/1403.
> +
> +local test = tap.test('lj-1403-vmevent-crash-on-stkov'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(1)
> +
> +local jit_dump = require('jit.dump')
> +
> +-- XXX: Some specific stack usage without a stack top check by the
> +-- Lua function header.
> +local t = setmetatable({}, {__newindex = pcall, __call = type})

reproduced without __call metamethod, please add a comment with explanation

why it is needed

> +-- luacheck: no unused
> +local function prober(...)
> +  -- Invokes `pcall(t, t, t)`.
> +  t[t] = t
> +end
> +
> +jit.opt.start('hotloop=1')
> +-- Need the invocation of the VM event.
> +jit_dump.start('i', '/dev/null')
> +
> +-- The code below causes the stack overflow in the VM event
> +-- handler. The unwinding of the error breaks the JIT semantics
> +-- and leads to a crash.
> +local function looper()
> +  local r = pcall(prober)
> +  if not r then
> +    local n = 1
> +    while n < 3 do
> +      prober(1, 2)
> +      n = n + 1
> +    end
> +  end
> +  looper()
> +end
> +
> +pcall(coroutine.wrap(looper))
> +
> +test:ok(true, 'no crash')
> +
> +test:done(true)

[-- Attachment #2: Type: text/html, Size: 2478 bytes --]

Re: [Tarantool-patches] [PATCH luajit 1/2] Run VM events and finalizers in separate state.

[Sergey Kaplun via Tarantool-patches]

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

On 30.03.26, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> thanks for the patch! LGTM with a minor comment below.
> 
> Sergey
> 
> On 3/28/26 18:31, Sergey Kaplun wrote:
> 
> 
> <snipped>
> 
> > diff --git a/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
> > new file mode 100644
> > index 00000000..ad275e17
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
> > @@ -0,0 +1,47 @@
> > +local tap = require('tap')
> > +
> > +-- The test file to demonstrate LuaJIT crash during stack overflow
> > +-- in the VM event handle.
> > +-- See also,https://github.com/LuaJIT/LuaJIT/issues/1403.
> > +
> > +local test = tap.test('lj-1403-vmevent-crash-on-stkov'):skipcond({
> > +  ['Test requires JIT enabled'] = not jit.status(),
> > +})
> > +
> > +test:plan(1)
> > +
> > +local jit_dump = require('jit.dump')
> > +
> > +-- XXX: Some specific stack usage without a stack top check by the
> > +-- Lua function header.
> > +local t = setmetatable({}, {__newindex = pcall, __call = type})
> 
> reproduced without __call metamethod, please add a comment with explanation
> 
> why it is needed

Removed:
===================================================================
diff --git a/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
index ad275e17..6812c330 100644
--- a/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
+++ b/test/tarantool-tests/lj-1403-vmevent-crash-on-stkov.test.lua
@@ -14,7 +14,7 @@ local jit_dump = require('jit.dump')
 
 -- XXX: Some specific stack usage without a stack top check by the
 -- Lua function header.
-local t = setmetatable({}, {__newindex = pcall, __call = type})
+local t = setmetatable({}, {__newindex = pcall})
 -- luacheck: no unused
 local function prober(...)
   -- Invokes `pcall(t, t, t)`.
===================================================================

> 
> > +-- luacheck: no unused
> > +local function prober(...)
> > +  -- Invokes `pcall(t, t, t)`.
> > +  t[t] = t
> > +end
> > +
> > +jit.opt.start('hotloop=1')
> > +-- Need the invocation of the VM event.
> > +jit_dump.start('i', '/dev/null')
> > +
> > +-- The code below causes the stack overflow in the VM event
> > +-- handler. The unwinding of the error breaks the JIT semantics
> > +-- and leads to a crash.
> > +local function looper()
> > +  local r = pcall(prober)
> > +  if not r then
> > +    local n = 1
> > +    while n < 3 do
> > +      prober(1, 2)
> > +      n = n + 1
> > +    end
> > +  end
> > +  looper()
> > +end
> > +
> > +pcall(coroutine.wrap(looper))
> > +
> > +test:ok(true, 'no crash')
> > +
> > +test:done(true)

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/2] Run VM events and finalizers in separate state.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]


On 3/30/26 12:38, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed your comment and force-pushed the branch.
Thanks! LGTM

[-- Attachment #2: Type: text/html, Size: 563 bytes --]

[Tarantool-patches] [PATCH luajit 2/2] Fix VM event error handling for finalizers.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit fbb36bb6bfa88716a47c58bcf9ce9f2ef752abac)

After the previous commit, the VM handler for the 'errfin' VM event
works incorrectly. The error object is taken not from the stack on which
the VM handler is invoked. Hence, it breaks the non-VM stack and returns
an incorrect error message.

This patch fixes the issue by copying the object from the corresponding
Lua stack.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#12134
---
 src/lj_gc.c                                   |  6 +++--
 .../lj-1445-errfin-errmsg.test.lua            | 27 +++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1445-errfin-errmsg.test.lua

diff --git a/src/lj_gc.c b/src/lj_gc.c
index 83ad9ee9..7e4f0a47 100644
--- a/src/lj_gc.c
+++ b/src/lj_gc.c
@@ -519,10 +519,12 @@ static void gc_call_finalizer(global_State *g, lua_State *L,
   if (LJ_HASPROFILE && (oldh & HOOK_PROFILE)) lj_dispatch_update(g);
   g->gc.threshold = oldt;  /* Restore GC threshold. */
   if (errcode) {
+    TValue tmp;
+    copyTV(VL, &tmp, VL->top-1);
+    VL->top--;
     lj_vmevent_send(g, ERRFIN,
-      copyTV(V, V->top++, L->top-1);
+      copyTV(V, V->top++, &tmp);
     );
-    L->top--;
   }
 }
 
diff --git a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
new file mode 100644
index 00000000..def8b79c
--- /dev/null
+++ b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
@@ -0,0 +1,27 @@
+local tap = require('tap')
+local test = tap.test('lj-1445-errfin-errmsg')
+
+test:plan(2)
+
+local EXPECTED_ERR = 'expected err'
+local function bad_fin()
+  error(EXPECTED_ERR)
+end
+local EXPECTED_LOCATION = debug.getinfo(bad_fin).linedefined + 1
+
+local match_err = false
+local match_line = false
+local function errfin_handler(errmsg)
+  match_err = errmsg:match(EXPECTED_ERR)
+  match_line = errmsg:match(EXPECTED_LOCATION)
+end
+
+jit.attach(errfin_handler, 'errfin')
+
+debug.getmetatable(newproxy(true)).__gc = bad_fin
+collectgarbage()
+
+test:ok(match_err, 'correct error message in errfin handler')
+test:ok(match_line, 'correct source line in errfin handler')
+
+test:done(true)
-- 
2.53.0

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix VM event error handling for finalizers.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 2871 bytes --]

Hi, Sergey!

thanks for the patch! LGTM with a couple of comments below

Sergey

On 3/28/26 18:31, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Sergey Kaplun.
>
> (cherry picked from commit fbb36bb6bfa88716a47c58bcf9ce9f2ef752abac)
>
> After the previous commit, the VM handler for the 'errfin' VM event
> works incorrectly. The error object is taken not from the stack on which
> the VM handler is invoked. Hence, it breaks the non-VM stack and returns
> an incorrect error message.
>
> This patch fixes the issue by copying the object from the corresponding
> Lua stack.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#12134
> ---
>   src/lj_gc.c                                   |  6 +++--
>   .../lj-1445-errfin-errmsg.test.lua            | 27 +++++++++++++++++++
>   2 files changed, 31 insertions(+), 2 deletions(-)
>   create mode 100644 test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
>
> diff --git a/src/lj_gc.c b/src/lj_gc.c
> index 83ad9ee9..7e4f0a47 100644
> --- a/src/lj_gc.c
> +++ b/src/lj_gc.c
> @@ -519,10 +519,12 @@ static void gc_call_finalizer(global_State *g, lua_State *L,
>     if (LJ_HASPROFILE && (oldh & HOOK_PROFILE)) lj_dispatch_update(g);
>     g->gc.threshold = oldt;  /* Restore GC threshold. */
>     if (errcode) {
> +    TValue tmp;
> +    copyTV(VL, &tmp, VL->top-1);
> +    VL->top--;
>       lj_vmevent_send(g, ERRFIN,
> -      copyTV(V, V->top++, L->top-1);
> +      copyTV(V, V->top++, &tmp);
>       );
> -    L->top--;
>     }
>   }
>   
> diff --git a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
> new file mode 100644
> index 00000000..def8b79c
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
> @@ -0,0 +1,27 @@
> +local tap = require('tap')
> +local test = tap.test('lj-1445-errfin-errmsg')
> +

Usually you add a comment with test description and a link to the issue.

But this time a comment is absent. Should we add it?

> +test:plan(2)
> +
> +local EXPECTED_ERR = 'expected err'

s/err/error/

feel free to ignore

> +local function bad_fin()
> +  error(EXPECTED_ERR)
> +end
> +local EXPECTED_LOCATION = debug.getinfo(bad_fin).linedefined + 1
> +
> +local match_err = false
> +local match_line = false
> +local function errfin_handler(errmsg)
> +  match_err =errmsg:match(EXPECTED_ERR)
> +  match_line =errmsg:match(EXPECTED_LOCATION)
> +end
> +
> +jit.attach(errfin_handler, 'errfin')
> +
> +debug.getmetatable(newproxy(true)).__gc = bad_fin

I would define userdata previously for clarity:

local ud = newproxy(true)

and then use it in getmetatable().

feel free to ignore


> +collectgarbage()
> +
> +test:ok(match_err, 'correct error message in errfin handler')
> +test:ok(match_line, 'correct source line in errfin handler')
> +
> +test:done(true)

[-- Attachment #2: Type: text/html, Size: 4055 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix VM event error handling for finalizers.

[Sergey Kaplun via Tarantool-patches]

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

On 30.03.26, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> thanks for the patch! LGTM with a couple of comments below
> 
> Sergey
> 
> On 3/28/26 18:31, Sergey Kaplun wrote:

<snipped>

> > diff --git a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
> > new file mode 100644
> > index 00000000..def8b79c
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
> > @@ -0,0 +1,27 @@
> > +local tap = require('tap')
> > +local test = tap.test('lj-1445-errfin-errmsg')
> > +
> 
> Usually you add a comment with test description and a link to the issue.
> 
> But this time a comment is absent. Should we add it?

Added, thanks!

===================================================================
diff --git a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
index 7170065f..a52f6a06 100644
--- a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
+++ b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
@@ -1,4 +1,9 @@
 local tap = require('tap')
+
+-- The test file to demostrate LuaJIT's incorrect invocation of
+-- the VM event handler for the 'errfin' VM event.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1445.
+
 local test = tap.test('lj-1445-errfin-errmsg')
 
 test:plan(2)
===================================================================

> 
> > +test:plan(2)
> > +
> > +local EXPECTED_ERR = 'expected err'
> 
> s/err/error/

Fixed:
===================================================================
diff --git a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
index def8b79c..205cd2d0 100644
--- a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
+++ b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
@@ -3,7 +3,7 @@ local test = tap.test('lj-1445-errfin-errmsg')
 
 test:plan(2)
 
-local EXPECTED_ERR = 'expected err'
+local EXPECTED_ERR = 'expected error'
 local function bad_fin()
   error(EXPECTED_ERR)
 end
===================================================================

> 
> feel free to ignore
> 
> > +local function bad_fin()
> > +  error(EXPECTED_ERR)
> > +end
> > +local EXPECTED_LOCATION = debug.getinfo(bad_fin).linedefined + 1
> > +
> > +local match_err = false
> > +local match_line = false
> > +local function errfin_handler(errmsg)
> > +  match_err =errmsg:match(EXPECTED_ERR)
> > +  match_line =errmsg:match(EXPECTED_LOCATION)
> > +end
> > +
> > +jit.attach(errfin_handler, 'errfin')
> > +
> > +debug.getmetatable(newproxy(true)).__gc = bad_fin
> 
> I would define userdata previously for clarity:
> 
> local ud = newproxy(true)
> 
> and then use it in getmetatable().
> 
> feel free to ignore
> 

Don't want to use `ud = nil`. Added the comment.

===================================================================
diff --git a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
index def8b79c..205cd2d0 100644
--- a/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
+++ b/test/tarantool-tests/lj-1445-errfin-errmsg.test.lua
@@ -18,6 +18,7 @@ end
 
 jit.attach(errfin_handler, 'errfin')
 
+-- Create the userdata with the finalizer and collect it.
 debug.getmetatable(newproxy(true)).__gc = bad_fin
 collectgarbage()
===================================================================

> 
> > +collectgarbage()
> > +
> > +test:ok(match_err, 'correct error message in errfin handler')
> > +test:ok(match_line, 'correct source line in errfin handler')
> > +
> > +test:done(true)

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/2] Fix VM event error handling for finalizers.

[Sergey Bronnikov via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 159 bytes --]


On 3/30/26 12:44, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed the comments and force-pushed the branch.
>
>
Thanks for the fixes! LGTM

[-- Attachment #2: Type: text/html, Size: 589 bytes --]