Re: [Tarantool-patches] [PATCH luajit] Handle OOM error on stack resize in coroutine.resume and lua_checkstack.

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

[-- Attachment #1: Type: text/plain, Size: 7560 bytes --]

H, Sergey,

thanks for the patch! LGTM

Sergey

On 3/13/26 14:24, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Peter Cawley.
>
> (cherry picked from commit a5d2f70c73e406beb617afa829a7af5b8c1d842c)
>
> If the OOM or stack overflow error is obtained during
> `lj_state_growstack()` in the `coroutine.resume()` it may raise an error
> leading to the incorrect error message and unexpected behaviour for
> this corner case. Also, `lua_checkstack()` may raise an OOM error
> leading to panic on an unprotected lua_State, for example.
>
> This patch handles these cases by protecting `lj_state_growstack()`.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#12134
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1066-err-coroutine-resume
> Related issues:
> *https://github.com/LuaJIT/LuaJIT/issues/1066
> *https://github.com/tarantool/tarantool/issues/12134
>
>   src/lib_base.c                                |  5 +-
>   src/lj_api.c                                  |  7 +-
>   src/lj_state.c                                | 12 +++
>   src/lj_state.h                                |  1 +
>   .../lj-1066-err-lua-checkstack.test.c         | 84 +++++++++++++++++++
>   .../lj-1066-err-coroutine-resume.test.lua     | 27 ++++++
>   6 files changed, 134 insertions(+), 2 deletions(-)
>   create mode 100644 test/tarantool-c-tests/lj-1066-err-lua-checkstack.test.c
>   create mode 100644 test/tarantool-tests/lj-1066-err-coroutine-resume.test.lua
>
> diff --git a/src/lib_base.c b/src/lib_base.c
> index 23728d6c..a5b907de 100644
> --- a/src/lib_base.c
> +++ b/src/lib_base.c
> @@ -606,7 +606,10 @@ static int ffh_resume(lua_State *L, lua_State *co, int wrap)
>       setstrV(L, L->base-LJ_FR2, lj_err_str(L, em));
>       return FFH_RES(2);
>     }
> -  lj_state_growstack(co, (MSize)(L->top - L->base));
> +  if (lj_state_cpgrowstack(co, (MSize)(L->top - L->base)) != LUA_OK) {
> +    cTValue *msg = --co->top;
> +    lj_err_callermsg(L, strVdata(msg));
> +  }
>     return FFH_RETRY;
>   }
>   
> diff --git a/src/lj_api.c b/src/lj_api.c
> index 2e915306..16a1da0e 100644
> --- a/src/lj_api.c
> +++ b/src/lj_api.c
> @@ -116,7 +116,12 @@ LUA_API int lua_checkstack(lua_State *L, int size)
>     if (size > LUAI_MAXCSTACK || (L->top - L->base + size) > LUAI_MAXCSTACK) {
>       return 0;  /* Stack overflow. */
>     } else if (size > 0) {
> -    lj_state_checkstack(L, (MSize)size);
> +    int avail = (int)(mref(L->maxstack, TValue) - L->top);
> +    if (size > avail &&
> +	lj_state_cpgrowstack(L, (MSize)(size - avail)) != LUA_OK) {
> +      L->top--;
> +      return 0;  /* Out of memory. */
> +    }
>     }
>     return 1;
>   }
> diff --git a/src/lj_state.c b/src/lj_state.c
> index 8a1acf87..1b698485 100644
> --- a/src/lj_state.c
> +++ b/src/lj_state.c
> @@ -170,6 +170,18 @@ void LJ_FASTCALL lj_state_growstack1(lua_State *L)
>     lj_state_growstack(L, 1);
>   }
>   
> +static TValue *cpgrowstack(lua_State *co, lua_CFunction dummy, void *ud)
> +{
> +  UNUSED(dummy);
> +  lj_state_growstack(co, *(MSize *)ud);
> +  return NULL;
> +}
> +
> +int LJ_FASTCALL lj_state_cpgrowstack(lua_State *L, MSize need)
> +{
> +  return lj_vm_cpcall(L, NULL, &need, cpgrowstack);
> +}
> +
>   /* Allocate basic stack for new state. */
>   static void stack_init(lua_State *L1, lua_State *L)
>   {
> diff --git a/src/lj_state.h b/src/lj_state.h
> index 02a0eafa..cb3581d3 100644
> --- a/src/lj_state.h
> +++ b/src/lj_state.h
> @@ -18,6 +18,7 @@ LJ_FUNC void lj_state_relimitstack(lua_State *L);
>   LJ_FUNC void lj_state_shrinkstack(lua_State *L, MSize used);
>   LJ_FUNCA void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need);
>   LJ_FUNC void LJ_FASTCALL lj_state_growstack1(lua_State *L);
> +LJ_FUNC int LJ_FASTCALL lj_state_cpgrowstack(lua_State *L, MSize need);
>   
>   static LJ_AINLINE void lj_state_checkstack(lua_State *L, MSize need)
>   {
> diff --git a/test/tarantool-c-tests/lj-1066-err-lua-checkstack.test.c b/test/tarantool-c-tests/lj-1066-err-lua-checkstack.test.c
> new file mode 100644
> index 00000000..d1cc8b06
> --- /dev/null
> +++ b/test/tarantool-c-tests/lj-1066-err-lua-checkstack.test.c
> @@ -0,0 +1,84 @@
> +#include "lua.h"
> +#include "luaconf.h"
> +
> +#include "test.h"
> +#include "utils.h"
> +
> +#include <stdlib.h>
> +
> +/* XXX: Still need normal assert for sanity checks. */
> +#undef NDEBUG
> +#include <assert.h>
> +
> +static lua_Alloc old_allocf = NULL;
> +static void *old_alloc_state = NULL;
> +
> +/* Always OOM on reallocation (not on allocation). */
> +static void *allocf_null_realloc(void *ud, void *ptr, size_t osize,
> +				 size_t nsize)
> +{
> +	assert(old_allocf != NULL);
> +	if (ptr != NULL && osize < nsize)
> +		return NULL;
> +	else
> +		return old_allocf(ud, ptr, osize, nsize);
> +}
> +
> +static void enable_allocinject(lua_State *L)
> +{
> +	assert(old_allocf == NULL);
> +	old_allocf = lua_getallocf(L, &old_alloc_state);
> +	lua_setallocf(L, allocf_null_realloc, old_alloc_state);
> +}
> +
> +/* Restore the default allocator function. */
> +static void disable_allocinject(lua_State *L)
> +{
> +	assert(old_allocf != NULL);
> +	lua_setallocf(L, old_allocf, old_alloc_state);
> +	old_allocf = NULL;
> +	old_alloc_state = NULL;
> +}
> +
> +static int checkstack_res = -1;
> +
> +static int test_checkstack(lua_State *L)
> +{
> +	/*
> +	 * There is no stack overflow error, but the OOM error
> +	 * due to stack reallocation, before the patch.
> +	 */
> +	checkstack_res = lua_checkstack(L, LUAI_MAXCSTACK / 2);
> +	return 0;
> +}
> +
> +static int oom_on_lua_checkstack(void *test_state)
> +{
> +	lua_State *L = test_state;
> +	/* Use fresh-new coroutine for stack manipulations. */
> +	lua_State *L1 = lua_newthread(L);
> +
> +	enable_allocinject(L);
> +	/*
> +	 * `L1` should have enough space to `cpcall()` without
> +	 * stack reallocation.
> +	 */
> +	int status = lua_cpcall(L1, test_checkstack, NULL);
> +	disable_allocinject(L);
> +
> +	assert_true(status == LUA_OK);
> +	assert_true(checkstack_res == 0);
> +
> +	return TEST_EXIT_SUCCESS;
> +}
> +
> +int main(void)
> +{
> +	lua_State *L = utils_lua_init();
> +	const struct test_unit tgroup[] = {
> +		test_unit_def(oom_on_lua_checkstack),
> +	};
> +	const int test_result = test_run_group(tgroup, L);
> +	utils_lua_close(L);
> +	return test_result;
> +}
> diff --git a/test/tarantool-tests/lj-1066-err-coroutine-resume.test.lua b/test/tarantool-tests/lj-1066-err-coroutine-resume.test.lua
> new file mode 100644
> index 00000000..3324c351
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1066-err-coroutine-resume.test.lua
> @@ -0,0 +1,27 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT misbehaviour in case of error
> +-- on growing stack for the coroutine during `coroutine.resume()`.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1066.
> +
> +local test = tap.test('lj-1066-err-coroutine-resume')
> +
> +test:plan(2)
> +
> +local function resume_wrap()
> +  local function recurser()
> +    recurser(coroutine.yield())
> +  end
> +  local co = coroutine.create(recurser)
> +  -- Cause the stack overflow and throws an error with incorrect
> +  -- error message before the patch. Use some arguments to obtain
> +  -- the stack overflow faster.
> +  while coroutine.resume(co, 1, 2, 3, 4, 5, 6, 7, 8, 9) do end
> +end
> +
> +local status, errmsg = pcall(resume_wrap)
> +
> +test:is(status, false, 'status is correct')
> +test:like(errmsg, 'stack overflow', 'error message is correct')
> +
> +test:done(true)

[-- Attachment #2: Type: text/html, Size: 8035 bytes --]