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

[Igor Munkin <imun@tarantool.org>]

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
execution re-enters LuaJIT VM. Sandwich detection while mcode execution
leads to platform panic.

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) {
+  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)
@@ -300,7 +311,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);
+      call(L, base, 1+1);
       L->top -= 2+LJ_FR2;
       return tvistruecond(L->top+1+LJ_FR2);
     }
@@ -323,7 +334,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);
+      call(L, base, 1+1);
       L->top -= 2+LJ_FR2;
       return tvistruecond(L->top+1+LJ_FR2);
     }
@@ -775,7 +786,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);
+      call(L, top, 1+1);
       L->top -= 1+LJ_FR2;
       copyTV(L, L->top-1, L->top+LJ_FR2);
     } while (--n > 0);
@@ -795,7 +806,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);
+    call(L, L->top-2, 1+1);
     L->top -= 2+LJ_FR2;
     v = L->top+1+LJ_FR2;
   }
@@ -811,7 +822,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);
+    call(L, L->top-2, 1+1);
     L->top -= 2+LJ_FR2;
     v = L->top+1+LJ_FR2;
   }
@@ -966,7 +977,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);
+    call(L, base, 0+1);
     L->top -= 3+LJ_FR2;
   }
 }
@@ -987,7 +998,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);
+    call(L, base, 0+1);
     L->top -= 2+LJ_FR2;
   }
 }
@@ -1118,7 +1129,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);
+  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 +1147,12 @@ LUA_API int lua_pcall(lua_State *L, int nargs, int nresults, int errfunc)
     api_checkvalidindex(L, o);
     ef = savestack(L, o);
   }
+  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;
@@ -1172,7 +1189,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);
+    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/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..2a24fb2
--- /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 ipp(struct sandwich *state, int i)
+{
+	if (i < state->trigger)
+		return i + 1;
+
+	/* Sandwich is triggered and Lua ipp 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 ipp 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 ipp 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 ipp 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;
+}
diff --git a/test/gh-4427-ffi-sandwich/test.lua b/test/gh-4427-ffi-sandwich/test.lua
new file mode 100644
index 0000000..3ccaee5
--- /dev/null
+++ b/test/gh-4427-ffi-sandwich/test.lua
@@ -0,0 +1,26 @@
+local cfg = {
+  hotloop = arg[1] or 1,
+  trigger = arg[2] or 1,
+}
+
+local ffi = require('ffi')
+local ffisandwich = ffi.load('libsandwich')
+ffi.cdef('int ipp(struct sandwich *state, int i)')
+
+-- Save the current coroutine and set the value to trigger ipp
+-- call the Lua routine instead of pure 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.ipp(sandwich, i)
+end
+-- Check the resulting value if panic didn't occur earlier.
+print(res)
-- 
2.25.0