Re: [Tarantool-patches] [PATCH luajit] Fix frame for on-trace out-of-memory error.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Max,

Thanks for the patch! LGTM


P.S. I've made an attempt to reproduce the original issue without a 
portion of Lua C code

and build userdata with calling "newproxy()", but attempt is failed, so  
seems Lua C is required here.

Also I'm upset that test triggers an OOM but we cannot reliably distinct 
that

triggered OOM is exactly that OOM that we want. Discussed this verbally,

perhaps such places should be closed with other tests


Sergey


On 7/24/23 17:41, Maxim Kokryashkin wrote:
> Reported by ruidong007.
>
> (cherry-picked from commit 2d8300c1944f3a62c10f0829e9b7847c5a6f0482)
>
> When an on-trace OOM error is triggered from a frame that is
> child in regard to `jit_base`, and `L->base` is not updated
> correspondingly (FUNCC, for example), it is possible to
> encounter an inconsistent Lua stack in the error handler.
>
> This patch adds a fixup for OOM errors on trace that always
> sets the Lua stack base to `jit_base`, so the stack is
> now consistent.
>
> Part of tarantool/tarantool#8825
> ---
> PR: https://github.com/tarantool/tarantool/pull/8909
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/lj-1004-oom-error-frame

LJ issue: https://github.com/LuaJIT/LuaJIT/issues/1004


>   src/lj_err.c                                  |  4 ++++
>   test/tarantool-tests/CMakeLists.txt           |  1 +
>   .../lj-1004-oom-error-frame.test.lua          | 24 +++++++++++++++++++
>   .../lj-1004-oom-error-frame/CMakeLists.txt    |  1 +
>   .../lj-1004-oom-error-frame/testoomframe.c    | 17 +++++++++++++
>   5 files changed, 47 insertions(+)
>   create mode 100644 test/tarantool-tests/lj-1004-oom-error-frame.test.lua
>   create mode 100644 test/tarantool-tests/lj-1004-oom-error-frame/CMakeLists.txt
>   create mode 100644 test/tarantool-tests/lj-1004-oom-error-frame/testoomframe.c
>
> diff --git a/src/lj_err.c b/src/lj_err.c
> index 9903d273..09729791 100644
> --- a/src/lj_err.c
> +++ b/src/lj_err.c
> @@ -802,6 +802,10 @@ LJ_NOINLINE void lj_err_mem(lua_State *L)
>   {
>     if (L->status == LUA_ERRERR+1)  /* Don't touch the stack during lua_open. */
>       lj_vm_unwind_c(L->cframe, LUA_ERRMEM);
> +  if (LJ_HASJIT) {
> +    TValue *base = tvref(G(L)->jit_base);
> +    if (base) L->base = base;
> +  }
>     if (curr_funcisL(L)) L->top = curr_topL(L);
>     setstrV(L, L->top++, lj_err_str(L, LJ_ERR_ERRMEM));
>     lj_err_throw(L, LUA_ERRMEM);
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 6218f76a..93230677 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -66,6 +66,7 @@ add_subdirectory(lj-416-xor-before-jcc)
>   add_subdirectory(lj-601-fix-gc-finderrfunc)
>   add_subdirectory(lj-727-lightuserdata-itern)
>   add_subdirectory(lj-flush-on-trace)
> +add_subdirectory(lj-1004-oom-error-frame)
>   
>   # The part of the memory profiler toolchain is located in tools
>   # directory, jit, profiler, and bytecode toolchains are located
> diff --git a/test/tarantool-tests/lj-1004-oom-error-frame.test.lua b/test/tarantool-tests/lj-1004-oom-error-frame.test.lua
> new file mode 100644
> index 00000000..fd167d14
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1004-oom-error-frame.test.lua
> @@ -0,0 +1,24 @@
> +local tap = require('tap')
> +local test  = tap.test('lj-1004-oom-error-frame'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +  ['Test requires GC64 mode disabled'] = require('ffi').abi('gc64'),
> +})
> +
> +test:plan(1)
> +
> +local testoomframe = require('testoomframe')
> +
> +local anchor = {}
> +local function extra_frame(val)
> +  table.insert(anchor, val)
> +end
> +
> +local function chomp()
> +  while true do
> +    extra_frame(testoomframe.allocate_userdata())
> +  end
> +end
> +
> +local st, _ = pcall(chomp)
> +test:ok(st == false, 'on-trace error handled successfully')
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/tarantool-tests/lj-1004-oom-error-frame/CMakeLists.txt b/test/tarantool-tests/lj-1004-oom-error-frame/CMakeLists.txt
> new file mode 100644
> index 00000000..3bca5df8
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1004-oom-error-frame/CMakeLists.txt
> @@ -0,0 +1 @@
> +BuildTestCLib(testoomframe testoomframe.c)
> diff --git a/test/tarantool-tests/lj-1004-oom-error-frame/testoomframe.c b/test/tarantool-tests/lj-1004-oom-error-frame/testoomframe.c
> new file mode 100644
> index 00000000..13071b4e
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1004-oom-error-frame/testoomframe.c
> @@ -0,0 +1,17 @@
> +#include <lua.h>
> +#include <lauxlib.h>
> +
> +static int allocate_userdata(lua_State *L) {
> +	lua_newuserdata(L, 16);
> +	return 1;
> +}
> +
> +static const struct luaL_Reg testoomframe[] = {
> +	{"allocate_userdata", allocate_userdata},
> +	{NULL, NULL}
> +};
> +
> +LUA_API int luaopen_testoomframe(lua_State *L) {
> +	luaL_register(L, "testoomframe", testoomframe);
> +	return 1;
> +}