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

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

Hi, Max


see my comments below

On 9/5/23 13:39, 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 the trace that always
> sets the Lua stack base to `jit_base`, so the stack is
> now consistent.
>
> Part of tarantool/tarantool#8825
> ---


<snipped>

> +local testoomframe = require('testoomframe')
> +
> +local anchor_memory = {} -- luacheck: no unused
> +local function eatchunks(size)
> +  while true do
> +    anchor_memory[ffi.new('char[?]', size)] = 1


Why ffi.new() is a key, not a value?

> +  end
> +end
> +
> +pcall(eatchunks, 512 * 1024 * 1024)

Why exactly this size is used?


> +
> +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, err = pcall(chomp)
> +test:ok(st == false, 'on-trace error handled successfully')
> +test:like(err, 'not enough memory', 'error is OOM')
> +test:done(true)
> 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..a54eac63
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1004-oom-error-frame/testoomframe.c
> @@ -0,0 +1,17 @@
> +#include <lua.h>
> +#include <lauxlib.h>

Test uses headers provided by systems instead of headers provided by 
LuaJIT-under-test. It is expected?

--- a/test/tarantool-tests/lj-1004-oom-error-frame/testoomframe.c
+++ b/test/tarantool-tests/lj-1004-oom-error-frame/testoomframe.c
@@ -1,5 +1,5 @@
-#include <lua.h>
-#include <lauxlib.h>
+#include "lua.h"
+#include "lauxlib.h"

  static int allocate_userdata(lua_State *L) {
         lua_newuserdata(L, 1);

> +
> +static int allocate_userdata(lua_State *L) {
> +	lua_newuserdata(L, 1);
> +	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;
> +}