[Tarantool-patches] [PATCH luajit 2/2] Call error function on rethrow after trace exit.
Sergey Kaplun
skaplun at tarantool.org
Sun Jul 31 13:58:31 MSK 2022
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 at 8 // (lua_State *L, int errcode)
+ | call extern lj_err_run at 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
More information about the Tarantool-patches
mailing list