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

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

[Sergey Kaplun via Tarantool-patches]

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 also https://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.
+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}")
-- 
2.53.0

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

[Sergey Bronnikov via Tarantool-patches]

[-- 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 --]

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

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!

Thanks for the review!
See my answer below.

On 04.03.26, Sergey Bronnikov wrote:
> 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)
> >

<snipped>

> > +-- 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.

This is done to be consistent witht the OOM abbreviation above.
If you don't insist, I would leave them as is.

> > +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)

<snipped>

-- 
Best regards,
Sergey Kaplun