* [Tarantool-patches] [PATCH v2 luajit 2/3] jit: abort trace recording and execution for C API
2020-04-15 0:34 [Tarantool-patches] [PATCH v2 luajit 0/3] Trace abort on FFI sandwich or mode change Igor Munkin
2020-04-15 0:34 ` [Tarantool-patches] [PATCH v2 luajit 1/3] test: add auxillary module for testing Igor Munkin
@ 2020-04-15 0:34 ` Igor Munkin
2020-04-15 0:34 ` [Tarantool-patches] [PATCH v2 luajit 3/3] jit: abort trace execution on JIT mode change Igor Munkin
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Igor Munkin @ 2020-04-15 0:34 UTC (permalink / raw)
To: Vladislav Shpilevoy, Sergey Ostanevich; +Cc: tarantool-patches
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 -> C 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 when the
execution re-enters LuaJIT VM. If re-entrancy is detected while running
mcode the platform finishes its execution with EXIT_FAILURE code and
calls panic routine prior to the exit.
Relates to tarantool/tarantool#4427
Co-authored-by: Vyacheslav Egorov <vegorov@google.com>
Co-authored-by: Sergey Ostanevich <sergos@tarantool.org>
Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
---
src/lj_api.c | 44 ++++++++++++++----
src/lj_errmsg.h | 1 +
test/gh-4427-ffi-sandwich.skipcond | 7 +++
test/gh-4427-ffi-sandwich.test.lua | 49 ++++++++++++++++++++
test/gh-4427-ffi-sandwich/CMakeLists.txt | 1 +
test/gh-4427-ffi-sandwich/libsandwich.c | 59 ++++++++++++++++++++++++
6 files changed, 152 insertions(+), 9 deletions(-)
create mode 100644 test/gh-4427-ffi-sandwich.skipcond
create mode 100755 test/gh-4427-ffi-sandwich.test.lua
create mode 100644 test/gh-4427-ffi-sandwich/CMakeLists.txt
create mode 100644 test/gh-4427-ffi-sandwich/libsandwich.c
diff --git a/src/lj_api.c b/src/lj_api.c
index a5e2465..c9b5d22 100644
--- a/src/lj_api.c
+++ b/src/lj_api.c
@@ -76,6 +76,18 @@ static GCtab *getcurrenv(lua_State *L)
return fn->c.gct == ~LJ_TFUNC ? tabref(fn->c.env) : tabref(L->env);
}
+static void jit_secure_call(lua_State *L, TValue *base, int nres) {
+ global_State *g = G(L);
+ /* Forbid Lua world re-entrancy while running the trace */
+ 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)
@@ -300,7 +312,7 @@ LUA_API int lua_equal(lua_State *L, int idx1, int idx2)
return (int)(uintptr_t)base;
} else {
L->top = base+2;
- lj_vm_call(L, base, 1+1);
+ jit_secure_call(L, base, 1+1);
L->top -= 2+LJ_FR2;
return tvistruecond(L->top+1+LJ_FR2);
}
@@ -323,7 +335,7 @@ LUA_API int lua_lessthan(lua_State *L, int idx1, int idx2)
return (int)(uintptr_t)base;
} else {
L->top = base+2;
- lj_vm_call(L, base, 1+1);
+ jit_secure_call(L, base, 1+1);
L->top -= 2+LJ_FR2;
return tvistruecond(L->top+1+LJ_FR2);
}
@@ -775,7 +787,7 @@ LUA_API void lua_concat(lua_State *L, int n)
}
n -= (int)(L->top - top);
L->top = top+2;
- lj_vm_call(L, top, 1+1);
+ jit_secure_call(L, top, 1+1);
L->top -= 1+LJ_FR2;
copyTV(L, L->top-1, L->top+LJ_FR2);
} while (--n > 0);
@@ -795,7 +807,7 @@ LUA_API void lua_gettable(lua_State *L, int idx)
v = lj_meta_tget(L, t, L->top-1);
if (v == NULL) {
L->top += 2;
- lj_vm_call(L, L->top-2, 1+1);
+ jit_secure_call(L, L->top-2, 1+1);
L->top -= 2+LJ_FR2;
v = L->top+1+LJ_FR2;
}
@@ -811,7 +823,7 @@ LUA_API void lua_getfield(lua_State *L, int idx, const char *k)
v = lj_meta_tget(L, t, &key);
if (v == NULL) {
L->top += 2;
- lj_vm_call(L, L->top-2, 1+1);
+ jit_secure_call(L, L->top-2, 1+1);
L->top -= 2+LJ_FR2;
v = L->top+1+LJ_FR2;
}
@@ -966,7 +978,7 @@ LUA_API void lua_settable(lua_State *L, int idx)
TValue *base = L->top;
copyTV(L, base+2, base-3-2*LJ_FR2);
L->top = base+3;
- lj_vm_call(L, base, 0+1);
+ jit_secure_call(L, base, 0+1);
L->top -= 3+LJ_FR2;
}
}
@@ -987,7 +999,7 @@ LUA_API void lua_setfield(lua_State *L, int idx, const char *k)
TValue *base = L->top;
copyTV(L, base+2, base-3-2*LJ_FR2);
L->top = base+3;
- lj_vm_call(L, base, 0+1);
+ jit_secure_call(L, base, 0+1);
L->top -= 2+LJ_FR2;
}
}
@@ -1118,7 +1130,7 @@ LUA_API void lua_call(lua_State *L, int nargs, int nresults)
{
api_check(L, L->status == LUA_OK || L->status == LUA_ERRERR);
api_checknelems(L, nargs+1);
- lj_vm_call(L, api_call_base(L, nargs), nresults+1);
+ jit_secure_call(L, api_call_base(L, nargs), nresults+1);
}
LUA_API int lua_pcall(lua_State *L, int nargs, int nresults, int errfunc)
@@ -1136,6 +1148,13 @@ LUA_API int lua_pcall(lua_State *L, int nargs, int nresults, int errfunc)
api_checkvalidindex(L, o);
ef = savestack(L, o);
}
+ /* Forbid Lua world re-entrancy while running the trace */
+ 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 */
status = lj_vm_pcall(L, api_call_base(L, nargs), nresults+1, ef);
if (status) hook_restore(g, oldh);
return status;
@@ -1160,6 +1179,13 @@ LUA_API int lua_cpcall(lua_State *L, lua_CFunction func, void *ud)
uint8_t oldh = hook_save(g);
int status;
api_check(L, L->status == LUA_OK || L->status == LUA_ERRERR);
+ /* Forbid Lua world re-entrancy while running the trace */
+ 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 */
status = lj_vm_cpcall(L, func, ud, cpcall);
if (status) hook_restore(g, oldh);
return status;
@@ -1172,7 +1198,7 @@ LUALIB_API int luaL_callmeta(lua_State *L, int idx, const char *field)
if (LJ_FR2) setnilV(top++);
copyTV(L, top++, index2adr(L, idx));
L->top = top;
- lj_vm_call(L, top-1, 1+1);
+ jit_secure_call(L, top-1, 1+1);
return 1;
}
return 0;
diff --git a/src/lj_errmsg.h b/src/lj_errmsg.h
index 060a9f8..1580385 100644
--- a/src/lj_errmsg.h
+++ b/src/lj_errmsg.h
@@ -112,6 +112,7 @@ ERRDEF(NOJIT, "no JIT compiler for this architecture (yet)")
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")
/* Lexer/parser errors. */
ERRDEF(XMODE, "attempt to load chunk with wrong mode")
diff --git a/test/gh-4427-ffi-sandwich.skipcond b/test/gh-4427-ffi-sandwich.skipcond
new file mode 100644
index 0000000..2a2ec4d
--- /dev/null
+++ b/test/gh-4427-ffi-sandwich.skipcond
@@ -0,0 +1,7 @@
+import platform
+
+# Disabled on FreeBSD due to #4819.
+if platform.system() == 'FreeBSD':
+ self.skip = 1
+
+# vim: set ft=python:
diff --git a/test/gh-4427-ffi-sandwich.test.lua b/test/gh-4427-ffi-sandwich.test.lua
new file mode 100755
index 0000000..9d5e50f
--- /dev/null
+++ b/test/gh-4427-ffi-sandwich.test.lua
@@ -0,0 +1,49 @@
+#!/usr/bin/env tarantool
+
+if #arg == 0 then
+ require('utils').selfrun(arg, {
+ {
+ arg = {
+ 1, -- hotloop (arg[1])
+ 1, -- trigger (arg[2])
+ },
+ res = tostring(3), -- hotloop + trigger + 1
+ msg = 'Trace is aborted',
+ },
+ {
+ arg = {
+ 1, -- hotloop (arg[1])
+ 2, -- trigger (arg[2])
+ },
+ res = 'Lua VM re-entrancy is detected while executing the trace',
+ msg = 'Trace is recorded',
+ },
+ })
+end
+
+local cfg = {
+ hotloop = arg[1] or 1,
+ trigger = arg[2] or 1,
+}
+
+local ffi = require('ffi')
+local ffisandwich = ffi.load('libsandwich')
+ffi.cdef('int increment(struct sandwich *state, int i)')
+
+-- Save the current coroutine and set the value to trigger
+-- <increment> call the Lua routine instead of C implementation.
+local sandwich = require('libsandwich')(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))
+
+local res
+for i = 0, cfg.trigger + cfg.hotloop do
+ res = ffisandwich.increment(sandwich, i)
+end
+-- Check the resulting value if panic didn't occur earlier.
+print(res)
diff --git a/test/gh-4427-ffi-sandwich/CMakeLists.txt b/test/gh-4427-ffi-sandwich/CMakeLists.txt
new file mode 100644
index 0000000..995c6bb
--- /dev/null
+++ b/test/gh-4427-ffi-sandwich/CMakeLists.txt
@@ -0,0 +1 @@
+build_lualib(libsandwich libsandwich.c)
diff --git a/test/gh-4427-ffi-sandwich/libsandwich.c b/test/gh-4427-ffi-sandwich/libsandwich.c
new file mode 100644
index 0000000..029758b
--- /dev/null
+++ b/test/gh-4427-ffi-sandwich/libsandwich.c
@@ -0,0 +1,59 @@
+#include <lua.h>
+#include <lauxlib.h>
+
+struct sandwich {
+ lua_State *L; /* Coroutine saved for a Lua call */
+ int ref; /* Anchor to the Lua function to be run */
+ int trigger; /* Trigger for switching to Lua call */
+};
+
+int increment(struct sandwich *state, int i)
+{
+ if (i < state->trigger)
+ return i + 1;
+
+ /* Sandwich is triggered and Lua increment function is called */
+ lua_pushnumber(state->L, state->ref);
+ lua_gettable(state->L, LUA_REGISTRYINDEX);
+ lua_pushnumber(state->L, i);
+ lua_call(state->L, 1, 1);
+ return lua_tonumber(state->L, -1);
+}
+
+#define STRUCT_SANDWICH_MT "struct sandwich"
+
+static int init(lua_State *L)
+{
+ struct sandwich *state = lua_newuserdata(L, sizeof(struct sandwich));
+
+ luaL_getmetatable(L, STRUCT_SANDWICH_MT);
+ lua_setmetatable(L, -2);
+
+ /* Lua increment function to be called when sandwich is triggered */
+ if (luaL_dostring(L, "return function(i) return i + 1 end"))
+ luaL_error(L, "failed to translate Lua increment function");
+
+ state->ref = luaL_ref(L, LUA_REGISTRYINDEX);
+ state->L = L;
+ state->trigger = lua_tonumber(L, 1);
+ return 1;
+}
+
+static int fin(lua_State *L)
+{
+ struct sandwich *state = luaL_checkudata(L, 1, STRUCT_SANDWICH_MT);
+
+ /* Release the anchored increment function */
+ luaL_unref(L, LUA_REGISTRYINDEX, state->ref);
+ return 0;
+}
+
+LUA_API int luaopen_libsandwich(lua_State *L)
+{
+ luaL_newmetatable(L, STRUCT_SANDWICH_MT);
+ lua_pushcfunction(L, fin);
+ lua_setfield(L, -2, "__gc");
+
+ lua_pushcfunction(L, init);
+ return 1;
+}
--
2.25.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [Tarantool-patches] [PATCH v2 luajit 3/3] jit: abort trace execution on JIT mode change
2020-04-15 0:34 [Tarantool-patches] [PATCH v2 luajit 0/3] Trace abort on FFI sandwich or mode change Igor Munkin
2020-04-15 0:34 ` [Tarantool-patches] [PATCH v2 luajit 1/3] test: add auxillary module for testing Igor Munkin
2020-04-15 0:34 ` [Tarantool-patches] [PATCH v2 luajit 2/3] jit: abort trace recording and execution for C API Igor Munkin
@ 2020-04-15 0:34 ` Igor Munkin
2020-04-19 16:16 ` [Tarantool-patches] [PATCH v2 luajit 0/3] Trace abort on FFI sandwich or " Vladislav Shpilevoy
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Igor Munkin @ 2020-04-15 0:34 UTC (permalink / raw)
To: Vladislav Shpilevoy, Sergey Ostanevich; +Cc: tarantool-patches
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. If the call occurs the platform finishes its execution with
EXIT_FAILURE code and calls panic routine prior to the exit.
Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
---
src/lj_dispatch.c | 6 ++++
src/lj_errmsg.h | 1 +
test/lj-flush-on-trace.skipcond | 7 ++++
test/lj-flush-on-trace.test.lua | 48 +++++++++++++++++++++++++++
test/lj-flush-on-trace/CMakeLists.txt | 1 +
test/lj-flush-on-trace/libflush.c | 31 +++++++++++++++++
6 files changed, 94 insertions(+)
create mode 100644 test/lj-flush-on-trace.skipcond
create mode 100755 test/lj-flush-on-trace.test.lua
create mode 100644 test/lj-flush-on-trace/CMakeLists.txt
create mode 100644 test/lj-flush-on-trace/libflush.c
diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c
index 5d6795f..b694d8f 100644
--- a/src/lj_dispatch.c
+++ b/src/lj_dispatch.c
@@ -240,6 +240,12 @@ int luaJIT_setmode(lua_State *L, int idx, int mode)
{
global_State *g = G(L);
int mm = mode & LUAJIT_MODE_MASK;
+ /* Forbid JIT state change while running the trace */
+ 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.skipcond b/test/lj-flush-on-trace.skipcond
new file mode 100644
index 0000000..2a2ec4d
--- /dev/null
+++ b/test/lj-flush-on-trace.skipcond
@@ -0,0 +1,7 @@
+import platform
+
+# Disabled on FreeBSD due to #4819.
+if platform.system() == 'FreeBSD':
+ self.skip = 1
+
+# vim: set ft=python:
diff --git a/test/lj-flush-on-trace.test.lua b/test/lj-flush-on-trace.test.lua
new file mode 100755
index 0000000..0b3ccf4
--- /dev/null
+++ b/test/lj-flush-on-trace.test.lua
@@ -0,0 +1,48 @@
+#!/usr/bin/env tarantool
+
+if #arg == 0 then
+ require('utils').selfrun(arg, {
+ {
+ arg = {
+ 1, -- hotloop (arg[1])
+ 1, -- trigger (arg[2])
+ },
+ res = 'OK',
+ msg = 'Trace is aborted',
+ },
+ {
+ arg = {
+ 1, -- hotloop (arg[1])
+ 2, -- trigger (arg[2])
+ },
+ res = 'JIT mode change is detected while executing the trace',
+ msg = 'Trace is recorded',
+ },
+ })
+end
+
+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
+-- <flush> call the Lua routine instead of 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')
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 <lua.h>
+#include <luajit.h>
+
+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;
+}
--
2.25.0
^ permalink raw reply [flat|nested] 8+ messages in thread