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

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

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