From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6C8F04696C3 for ; Sat, 28 Mar 2020 22:36:20 +0300 (MSK) Date: Sat, 28 Mar 2020 22:36:18 +0300 From: Sergey Ostanevich Message-ID: <20200328193618.GB328@tarantool.org> References: <6a753febbdde86642257c57f7e3a3b1700317ca3.1585304087.git.imun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6a753febbdde86642257c57f7e3a3b1700317ca3.1585304087.git.imun@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH luajit 2/2] jit: abort trace execution on JIT mode change List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org Hi! Thanks for the patch, LGTM. Sergos. On 27 мар 13:47, Igor Munkin wrote: > Current luaJIT_setmode implementation aborts trace recording but nothing > prevents calling it on already compiled trace. E.g. if one conditionally > calls an FFI function having luaJIT_setmode with LUAJIT_MODE_FLUSH mode > underneath, the trace being executed can be purged and the return > address is invalidated as a result (since the mcode is released). > > This changeset prohibits luaJIT_setmode call while mcode is being > executed leading to platform panic if the call occurs. > > Signed-off-by: Igor Munkin > --- > src/lj_dispatch.c | 5 +++++ > src/lj_errmsg.h | 1 + > test/lj-flush-on-trace/CMakeLists.txt | 1 + > test/lj-flush-on-trace/libflush.c | 31 +++++++++++++++++++++++++++ > test/lj-flush-on-trace/test.lua | 25 +++++++++++++++++++++ > 5 files changed, 63 insertions(+) > create mode 100644 test/lj-flush-on-trace/CMakeLists.txt > create mode 100644 test/lj-flush-on-trace/libflush.c > create mode 100644 test/lj-flush-on-trace/test.lua > > diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c > index 5d6795f..b3448c8 100644 > --- a/src/lj_dispatch.c > +++ b/src/lj_dispatch.c > @@ -240,6 +240,11 @@ int luaJIT_setmode(lua_State *L, int idx, int mode) > { > global_State *g = G(L); > int mm = mode & LUAJIT_MODE_MASK; > + if (tvref(g->jit_base)) { > + setstrV(L, L->top++, lj_err_str(L, LJ_ERR_JITMODE)); > + if (g->panic) g->panic(L); > + exit(EXIT_FAILURE); > + } > lj_trace_abort(g); /* Abort recording on any state change. */ > /* Avoid pulling the rug from under our own feet. */ > if ((g->hookmask & HOOK_GC)) > diff --git a/src/lj_errmsg.h b/src/lj_errmsg.h > index 1580385..de7b867 100644 > --- a/src/lj_errmsg.h > +++ b/src/lj_errmsg.h > @@ -113,6 +113,7 @@ ERRDEF(NOJIT, "JIT compiler permanently disabled by build option") > #endif > ERRDEF(JITOPT, "unknown or malformed optimization flag " LUA_QS) > ERRDEF(JITCALL, "Lua VM re-entrancy is detected while executing the trace") > +ERRDEF(JITMODE, "JIT mode change is detected while executing the trace") > > /* Lexer/parser errors. */ > ERRDEF(XMODE, "attempt to load chunk with wrong mode") > diff --git a/test/lj-flush-on-trace/CMakeLists.txt b/test/lj-flush-on-trace/CMakeLists.txt > new file mode 100644 > index 0000000..a90452d > --- /dev/null > +++ b/test/lj-flush-on-trace/CMakeLists.txt > @@ -0,0 +1 @@ > +build_lualib(libflush libflush.c) > diff --git a/test/lj-flush-on-trace/libflush.c b/test/lj-flush-on-trace/libflush.c > new file mode 100644 > index 0000000..177409a > --- /dev/null > +++ b/test/lj-flush-on-trace/libflush.c > @@ -0,0 +1,31 @@ > +#include > +#include > + > +struct flush { > + lua_State *L; /* Coroutine saved to change JIT mode */ > + int trigger; /* Trigger for flushing all traces */ > +}; > + > +void flush(struct flush *state, int i) > +{ > + if (i < state->trigger) > + return; > + > + /* Trace flushing is triggered */ > + (void)luaJIT_setmode(state->L, 0, LUAJIT_MODE_ENGINE|LUAJIT_MODE_FLUSH); > +} > + > +static int init(lua_State *L) > +{ > + struct flush *state = lua_newuserdata(L, sizeof(struct flush)); > + > + state->L = L; > + state->trigger = lua_tonumber(L, 1); > + return 1; > +} > + > +LUA_API int luaopen_libflush(lua_State *L) > +{ > + lua_pushcfunction(L, init); > + return 1; > +} > diff --git a/test/lj-flush-on-trace/test.lua b/test/lj-flush-on-trace/test.lua > new file mode 100644 > index 0000000..ff6e0b6 > --- /dev/null > +++ b/test/lj-flush-on-trace/test.lua > @@ -0,0 +1,25 @@ > +local cfg = { > + hotloop = arg[1] or 1, > + trigger = arg[2] or 1, > +} > + > +local ffi = require('ffi') > +local ffiflush = ffi.load('libflush') > +ffi.cdef('void flush(struct flush *state, int i)') > + > +-- Save the current coroutine and set the value to trigger ipp > +-- call the Lua routine instead of pure C implementation. > +local flush = require('libflush')(cfg.trigger) > + > +-- Depending on trigger and hotloop values the following contexts > +-- are possible: > +-- * if trigger <= hotloop -> trace recording is aborted > +-- * if trigger > hotloop -> trace is recorded but execution > +-- leads to panic > +jit.opt.start("3", string.format("hotloop=%d", cfg.hotloop)) > + > +for i = 0, cfg.trigger + cfg.hotloop do > + ffiflush.flush(flush, i) > +end > +-- Panic didn't occur earlier. > +print('OK') > -- > 2.25.0 >