[Tarantool-patches] [PATCH luajit 2/2] Call error function on rethrow after trace exit.

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

From: Mike Pall <mike>

(cherry picked from commit e296f56b825c688c3530a981dc6b495d972f3d01)

This commit changes usage of `lj_err_throw` to `lj_err_run()` in
`lj_vm_exit_interp()` for not GC64 VMs, so the corresponding error
handler for `xpcall()` is called after trace exit. It allows to avoid
calling of error handler, when restoration from a snapshot is failed due
to the Lua stack overflow. This type of error can be handled by the
compiler itself and user shouldn't worry/know about them.

Also, this makes behaviour and code base for GC64 and not GC64 VMs
consistent.

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

Part of tarantool/tarantool#7230
---

The test for this patch is very fragile: if the Lua stack layout is
changed (for example smbd adds a one more parameter in `luaT_call()`)
the test will fail as far as some error not during snapshot restoration
will raised. So this particular test is skipped for the Tarantool.
Also, it is skipped for *BSD as far as there are no traces compiled [1],
so there is no restoration from a snapshot, so the errfunc is called.

[1]: https://github.com/tarantool/tarantool/issues/4819

 src/lj_debug.c                                |  1 +
 src/lj_dispatch.h                             |  2 +-
 src/lj_err.c                                  |  2 +-
 src/lj_err.h                                  |  2 +-
 src/lj_trace.c                                |  4 ++--
 src/vm_arm.dasc                               |  3 +--
 src/vm_mips.dasc                              |  5 ++--
 src/vm_ppc.dasc                               |  3 +--
 src/vm_x86.dasc                               |  4 +---
 .../lj-603-err-snap-restore.test.lua          | 23 ++++++++++++++++++-
 10 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/src/lj_debug.c b/src/lj_debug.c
index 8eb5983b..654dc913 100644
--- a/src/lj_debug.c
+++ b/src/lj_debug.c
@@ -93,6 +93,7 @@ static BCPos debug_framepc(lua_State *L, GCfunc *fn, cTValue *nextframe)
 	}
       }
       ins = cframe_pc(cf);
+      if (!ins) return NO_BCPOS;
     }
   }
   pt = funcproto(fn);
diff --git a/src/lj_dispatch.h b/src/lj_dispatch.h
index 5bda51a2..addf5572 100644
--- a/src/lj_dispatch.h
+++ b/src/lj_dispatch.h
@@ -46,7 +46,7 @@ extern double __divdf3(double a, double b);
   _(asin) _(acos) _(atan) _(sinh) _(cosh) _(tanh) _(frexp) _(modf) _(atan2) \
   _(pow) _(fmod) _(ldexp) _(lj_vm_modi) \
   _(lj_dispatch_call) _(lj_dispatch_ins) _(lj_dispatch_stitch) \
-  _(lj_dispatch_profile) _(lj_err_throw) \
+  _(lj_dispatch_profile) _(lj_err_throw) _(lj_err_run) \
   _(lj_ffh_coroutine_wrap_err) _(lj_func_closeuv) _(lj_func_newL_gc) \
   _(lj_gc_barrieruv) _(lj_gc_step) _(lj_gc_step_fixtop) _(lj_meta_arith) \
   _(lj_meta_call) _(lj_meta_cat) _(lj_meta_comp) _(lj_meta_equal) \
diff --git a/src/lj_err.c b/src/lj_err.c
index b520b3d3..c310daf6 100644
--- a/src/lj_err.c
+++ b/src/lj_err.c
@@ -602,7 +602,7 @@ static ptrdiff_t finderrfunc(lua_State *L)
 }
 
 /* Runtime error. */
-LJ_NOINLINE void lj_err_run(lua_State *L)
+LJ_NOINLINE void LJ_FASTCALL lj_err_run(lua_State *L)
 {
   ptrdiff_t ef = finderrfunc(L);
   if (ef) {
diff --git a/src/lj_err.h b/src/lj_err.h
index cba5fb71..aa4b7e0d 100644
--- a/src/lj_err.h
+++ b/src/lj_err.h
@@ -23,7 +23,7 @@ LJ_DATA const char *lj_err_allmsg;
 LJ_FUNC GCstr *lj_err_str(lua_State *L, ErrMsg em);
 LJ_FUNCA_NORET void LJ_FASTCALL lj_err_throw(lua_State *L, int errcode);
 LJ_FUNC_NORET void lj_err_mem(lua_State *L);
-LJ_FUNC_NORET void lj_err_run(lua_State *L);
+LJ_FUNCA_NORET void LJ_FASTCALL lj_err_run(lua_State *L);
 LJ_FUNC_NORET void lj_err_msg(lua_State *L, ErrMsg em);
 LJ_FUNC_NORET void lj_err_lex(lua_State *L, GCstr *src, const char *tok,
 			      BCLine line, ErrMsg em, va_list argp);
diff --git a/src/lj_trace.c b/src/lj_trace.c
index 68a657a7..bae4ba14 100644
--- a/src/lj_trace.c
+++ b/src/lj_trace.c
@@ -802,8 +802,8 @@ typedef struct ExitDataCP {
 static TValue *trace_exit_cp(lua_State *L, lua_CFunction dummy, void *ud)
 {
   ExitDataCP *exd = (ExitDataCP *)ud;
-  cframe_errfunc(L->cframe) = -1;  /* Inherit error function. */
-  /* Always catch error here. */
+  /* Always catch error here and don't call error function. */
+  cframe_errfunc(L->cframe) = 0;
   cframe_nres(L->cframe) = -2*LUAI_MAXSTACK*(int)sizeof(TValue);
   exd->pc = lj_snap_restore(exd->J, exd->exptr);
   UNUSED(dummy);
diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
index 21f7fecb..a29292f1 100644
--- a/src/vm_arm.dasc
+++ b/src/vm_arm.dasc
@@ -2247,9 +2247,8 @@ static void build_subroutines(BuildCtx *ctx)
   |  b <2
   |
   |9:  // Rethrow error from the right C frame.
-  |  rsb CARG2, CARG1, #0
   |  mov CARG1, L
-  |  bl extern lj_err_throw		// (lua_State *L, int errcode)
+  |  bl extern lj_err_run		// (lua_State *L)
   |.endif
   |
   |//-----------------------------------------------------------------------
diff --git a/src/vm_mips.dasc b/src/vm_mips.dasc
index ec57d789..93c772ff 100644
--- a/src/vm_mips.dasc
+++ b/src/vm_mips.dasc
@@ -2512,9 +2512,8 @@ static void build_subroutines(BuildCtx *ctx)
   |.  addu RA, RA, BASE
   |
   |9:  // Rethrow error from the right C frame.
-  |  load_got lj_err_throw
-  |  negu CARG2, CRET1
-  |  call_intern lj_err_throw		// (lua_State *L, int errcode)
+  |  load_got lj_err_run
+  |  call_intern lj_err_run		// (lua_State *L)
   |.  move CARG1, L
   |.endif
   |
diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
index 3f48b7ff..980176a2 100644
--- a/src/vm_ppc.dasc
+++ b/src/vm_ppc.dasc
@@ -2707,9 +2707,8 @@ static void build_subroutines(BuildCtx *ctx)
   |  bctr
   |
   |9:  // Rethrow error from the right C frame.
-  |  neg CARG2, CARG1
   |  mr CARG1, L
-  |  bl extern lj_err_throw		// (lua_State *L, int errcode)
+  |  bl extern lj_err_run		// (lua_State *L)
   |.endif
   |
   |//-----------------------------------------------------------------------
diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
index 92140cec..22f69edf 100644
--- a/src/vm_x86.dasc
+++ b/src/vm_x86.dasc
@@ -3040,10 +3040,8 @@ static void build_subroutines(BuildCtx *ctx)
   |  jmp <2
   |
   |9:  // Rethrow error from the right C frame.
-  |  neg RD
   |  mov FCARG1, L:RB
-  |  mov FCARG2, RD
-  |  call extern lj_err_throw@8		// (lua_State *L, int errcode)
+  |  call extern lj_err_run@4		// (lua_State *L)
   |.endif
   |
   |//-----------------------------------------------------------------------
diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
index 82ce6a8f..5cb487c3 100644
--- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
+++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
@@ -4,11 +4,27 @@ local tap = require('tap')
 -- error is raised on restoration from the snapshot.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/603.
 local test = tap.test('lj-603-err-snap-restore.test.lua')
-test:plan(1)
+test:plan(2)
 
+-- XXX: This is fragile. We need a specific amount of Lua stack
+-- slots is used to the error on restoration from a snapshot is
+-- raised and error handler isn't called according to the new
+-- behaviour. With another amount of used stack slots another one
+-- error may be raised (`LJ_ERR_STKOV` ("stack overflow") during
+-- growing stack while trying to push error message,
+-- `LJ_ERR_ERRERR` ("error in error handling"), etc.).
+-- This amount is suited well for GC64 and not GC64 mode.
+-- luacheck: no unused
+local _, _, _, _, _, _
+
+local handler_is_called = false
 local recursive_f
 local function errfunc()
   xpcall(recursive_f, errfunc)
+  -- Since this error is occured on snapshot restoration and may
+  -- handled by compiler itself, we shouldn't bother a user with
+  -- it.
+  handler_is_called = true
 end
 
 -- A recursive call to itself leads to trace with up-recursion.
@@ -22,6 +38,11 @@ end
 recursive_f()
 
 test:ok(true)
+-- Disabled on *BSD due to #4819.
+-- XXX: The different amount of stack slots is in-use for
+-- Tarantool at start, so just skip test for it.
+-- luacheck: no global
+test:ok(jit.os == 'BSD' or _TARANTOOL or not handler_is_called)
 
 -- XXX: Don't use `os.exit()` here intense. When error on snap
 -- restoration is raised, `err_unwind()` doesn't stop on correct
-- 
2.34.1