Re: [Tarantool-patches] [PATCH luajit 1/2] jit: abort trace recording and execution for C API

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

Thanks for the patch, see below.
On 27 мар 13:47, Igor Munkin wrote:
> JIT recording semantics assumes FFI calls are atomic regarding the
> LuaJIT VM: if the execution exited Lua world through FFI it is not
> re-entering Lua world again.
> 
> However, there are ways to break this atomicity via FFI: one can
> re-enter LuaJIT VM via Lua C API inside the C routine called via FFI. As
> a result the following stack mix is created:
> | Lua-FFI -> С routine -> Lua-C API -> Lua VM
> This sort of re-entrancy is explicitly not supported by LuaJIT compiler
> machinery. @mraleph named such layout an "FFI sandwich" and I'm strictly
> against eating it.
> 
> E.g. there is a trigger machinery in Tarantool calling Lua functions
> from C space. As a result of FFI function call with a trigger underneath
> the "FFI sandwich" is created.
> 
> This changeset introduces the mechanism for Lua-C API callbacks similar
> to the one existing for Lua-FFI: trace recording is aborted whe the
                                                              when
> execution re-enters LuaJIT VM. Sandwich detection while mcode execution
> leads to platform panic.
Could you elaborate here in comment on this a little: it means this 
particular Lua call to be aborted, fail for the fiber Lua state, or for 
the whole Tarantool?

> 
> Relates to tarantool/tarantool#4427
> 
> Co-authored-by: Vyacheslav Egorov <vegorov@google.com>
> Co-authored-by: Sergey Ostanevich <sergos@tarantool.org>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  src/lj_api.c                             | 35 ++++++++++----
>  src/lj_errmsg.h                          |  1 +
>  test/gh-4427-ffi-sandwich/CMakeLists.txt |  1 +
>  test/gh-4427-ffi-sandwich/libsandwich.c  | 59 ++++++++++++++++++++++++
>  test/gh-4427-ffi-sandwich/test.lua       | 26 +++++++++++
>  5 files changed, 113 insertions(+), 9 deletions(-)
>  create mode 100644 test/gh-4427-ffi-sandwich/CMakeLists.txt
>  create mode 100644 test/gh-4427-ffi-sandwich/libsandwich.c
>  create mode 100644 test/gh-4427-ffi-sandwich/test.lua
> 
> diff --git a/src/lj_api.c b/src/lj_api.c
> index a5e2465..c1f53e0 100644
> --- a/src/lj_api.c
> +++ b/src/lj_api.c
> @@ -76,6 +76,17 @@ static GCtab *getcurrenv(lua_State *L)
>    return fn->c.gct == ~LJ_TFUNC ? tabref(fn->c.env) : tabref(L->env);
>  }
>  
> +static void call(lua_State *L, TValue *base, int nres) {
I would recommend to name the function in a more descriptive way, like
sane_call. 

> +  global_State *g = G(L);
> +  if (tvref(g->jit_base)) {
> +    setstrV(L, L->top++, lj_err_str(L, LJ_ERR_JITCALL));
> +    if (g->panic) g->panic(L);
> +    exit(EXIT_FAILURE);
> +  }
> +  lj_trace_abort(g);  /* Never record across Lua VM entrance */
> +  lj_vm_call(L, base, nres);
> +}
> +
>  /* -- Miscellaneous API functions ----------------------------------------- */
>  
>  LUA_API int lua_status(lua_State *L)

Also I have a question on perf impact - have you measured any
benchmarks - Lua only, or as part of Tarantool?

Regards,
Sergos