Re: [Tarantool-patches] [PATCH luajit] Fix reporting of an error during error handling.

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

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

Hi, Sergey!

thanks for the patch! LGTM with minor comments below.

Sergey

On 3/2/26 11:23, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Sergey Kaplun.
>
> (cherry picked from commit 54a162688ed25902122077149df9b456bc5a763e)
>
> For now, any non-ERRRUN error in the error handling preserves its own
> error message. Thus, instead of the 'error in error handling', the
> caller gets an 'out of memory' or 'stack overflow' error.
>
> This patch adds the corresponding status check in the custom error
> handlers. The "error in error handling" string is also added to the
> preallocated strings to avoid OOM during error message allocation.
>
> 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-1381-fix-errmsg-in-err-handler
> Related issues:
> *https://github.com/LuaJIT/LuaJIT/issues/1381
> *https://github.com/tarantool/tarantool/issues/12134
>
>   src/lj_err.c                                  | 10 ++++
>   src/lj_state.c                                |  1 +
>   ...lj-1381-fix-errmsg-in-err-handler.test.lua | 50 +++++++++++++++++++
>   test/tarantool-tests/utils/CMakeLists.txt     |  1 +
>   4 files changed, 62 insertions(+)
>   create mode 100644 test/tarantool-tests/lj-1381-fix-errmsg-in-err-handler.test.lua
>
> diff --git a/src/lj_err.c b/src/lj_err.c
> index 80dca847..51baee8d 100644
> --- a/src/lj_err.c
> +++ b/src/lj_err.c
> @@ -775,9 +775,17 @@ LJ_NOINLINE GCstr *lj_err_str(lua_State *L, ErrMsg em)
>     return lj_str_newz(L, err2msg(em));
>   }
>   
> +LJ_NORET LJ_NOINLINE static void lj_err_err(lua_State *L)
> +{
> +  setstrV(L, L->top++, lj_err_str(L, LJ_ERR_ERRERR));
> +  lj_err_throw(L, LUA_ERRERR);
> +}
> +
>   /* Out-of-memory error. */
>   LJ_NOINLINE void lj_err_mem(lua_State *L)
>   {
> +  if (L->status == LUA_ERRERR)
> +    lj_err_err(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) {
> @@ -874,6 +882,8 @@ LJ_NOINLINE void LJ_FASTCALL lj_err_run(lua_State *L)
>   /* Stack overflow error. */
>   void LJ_FASTCALL lj_err_stkov(lua_State *L)
>   {
> +  if (L->status == LUA_ERRERR)
> +    lj_err_err(L);
>     lj_debug_addloc(L, err2msg(LJ_ERR_STKOV), L->base-1, NULL);
>     lj_err_run(L);
>   }
> diff --git a/src/lj_state.c b/src/lj_state.c
> index 053e5ec9..8e46fa6d 100644
> --- a/src/lj_state.c
> +++ b/src/lj_state.c
> @@ -201,6 +201,7 @@ static TValue *cpluaopen(lua_State *L, lua_CFunction dummy, void *ud)
>     lj_meta_init(L);
>     lj_lex_init(L);
>     fixstring(lj_err_str(L, LJ_ERR_ERRMEM));  /* Preallocate memory error msg. */
> +  fixstring(lj_err_str(L, LJ_ERR_ERRERR));  /* Preallocate err in err msg. */
>     g->gc.threshold = 4*g->gc.total;
>   #if LJ_HASFFI
>     lj_ctype_initfin(L);
> diff --git a/test/tarantool-tests/lj-1381-fix-errmsg-in-err-handler.test.lua b/test/tarantool-tests/lj-1381-fix-errmsg-in-err-handler.test.lua
> new file mode 100644
> index 00000000..a789683e
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1381-fix-errmsg-in-err-handler.test.lua
> @@ -0,0 +1,50 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT incorrect error message for the
> +-- errors in the error handler.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1381.
> +
> +local test = tap.test('lj-1381-fix-errmsg-in-err-handler')
> +
> +local allocinject = require('allocinject')
> +
> +test:plan(6)
> +
> +-- Disable JIT to avoid multiple invocation of the error handler.
> +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1382.
> +jit.off()
> +
> +-- OOM on the creation of ERRERR message.
> +coroutine.wrap(function()
> +  allocinject.enable_null_alloc()
> +  local st, msg = xpcall(error, error)
> +  allocinject.disable()
> +test:ok(not st, 'OOM ERRERR incorrect status')
> +  -- Prevent preallocated error message.
> +test:ok(msg:match('error in ' .. 'error handling'),
> +          'OOM ERRERR incorrect errmsg: ' .. msg)
> +end)()
> +
> +-- OOM in the error handler.
> +coroutine.wrap(function()
> +  local function errmem() local _ = {} end
> +  allocinject.enable_null_alloc()
> +  local st, msg = xpcall(error, errmem)
> +  allocinject.disable()
> +test:ok(not st, 'OOM incorrect status')
> +  -- Prevent preallocated error message.
> +test:ok(msg:match('error in ' .. 'error handling'),
> +          'OOM incorrect errmsg: ' .. msg)
> +end)()
> +
> +-- STKOV in the error handler.
Why abbreviation is used? May be "stack overflow"? Feel free to ignore.
> +coroutine.wrap(function()
> +  local function stkov() stkov() end
> +  local st, msg = xpcall(error, stkov)
> +test:ok(not st, 'STKOV incorrect status')
> +  -- Prevent preallocated error message.
> +test:ok(msg:match('error in ' .. 'error handling'),
> +          'STKOV incorrect errmsg: ' .. msg)
> +end)()
> +
> +test:done(true)
> diff --git a/test/tarantool-tests/utils/CMakeLists.txt b/test/tarantool-tests/utils/CMakeLists.txt
> index a16424ec..ec792b62 100644
> --- a/test/tarantool-tests/utils/CMakeLists.txt
> +++ b/test/tarantool-tests/utils/CMakeLists.txt
> @@ -5,5 +5,6 @@ list(APPEND tests
>     lj-1247-fin-tab-rehashing-on-trace.test.lua
>     lj-1249-loadfile-fd-leak.test.lua
>     lj-1298-oom-on-concat-recording.test.lua
> +  lj-1381-fix-errmsg-in-err-handler.test.lua
>   )
>   BuildTestCLib(allocinject allocinject.c "${tests}")

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