* [Tarantool-patches] [PATCH luajit 0/3][v3] Fix stack overflow in pcall/xpcall
@ 2026-03-12 9:05 Sergey Bronnikov via Tarantool-patches
2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 1/3][v3] MIPS64: Fix xpcall() error case Sergey Bronnikov via Tarantool-patches
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2026-03-12 9:05 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun
The proposed patches fixes stack overflow in pcall/xpcall.
Related issues:
- https://github.com/LuaJIT/LuaJIT/issues/1048
- https://github.com/LuaJIT/LuaJIT/issues/1402
- https://github.com/tarantool/tarantool/issues/12134
Git branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-fix-stack-checks-in-vararg-calls
Changes in v2:
- Added patch for MIPS64
- Fixed issues reported on review
Changes in v3:
- Added a new file with test gh-1402-call_init-regression.test.lua
- Adjusted N_ITERATIONS in the last patch with appropriate comment
- Fixups after commentaries from Sergey Kaplun
Mike Pall (3):
MIPS64: Fix xpcall() error case.
LJ_FR2: Fix stack checks in vararg calls.
Add stack check to pcall/xpcall.
src/lj_def.h | 2 +-
src/lj_dispatch.c | 2 +-
src/luajit-gdb.py | 2 +-
src/vm_arm.dasc | 7 ++
src/vm_arm64.dasc | 9 ++
src/vm_mips.dasc | 10 ++-
src/vm_mips64.dasc | 16 +++-
src/vm_ppc.dasc | 9 ++
src/vm_x64.dasc | 6 ++
src/vm_x86.dasc | 6 ++
.../gh-1402-call_init-regression.test.lua | 36 ++++++++
...048-fix-stack-checks-vararg-calls.test.lua | 86 +++++++++++++++++++
12 files changed, 184 insertions(+), 7 deletions(-)
create mode 100644 test/tarantool-tests/gh-1402-call_init-regression.test.lua
create mode 100644 test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [Tarantool-patches] [PATCH luajit 1/3][v3] MIPS64: Fix xpcall() error case. 2026-03-12 9:05 [Tarantool-patches] [PATCH luajit 0/3][v3] Fix stack overflow in pcall/xpcall Sergey Bronnikov via Tarantool-patches @ 2026-03-12 8:49 ` Sergey Bronnikov via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls Sergey Bronnikov via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 3/3][v3] Add stack check to pcall/xpcall Sergey Bronnikov via Tarantool-patches 2 siblings, 0 replies; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2026-03-12 8:49 UTC (permalink / raw) To: tarantool-patches, Sergey Kaplun From: Mike Pall <mike> Thanks to François Perrad and Stefan Pejic. (cherry picked from commit ea7071d3c30b6432bfe6f8a9d263e0285cec25e3) The patch fixes `xpcall()` segfaults on MIPS64 architecture. The similar patch for ARM64 has been backported (with the test) previously, see the commit af889e4608e6eca495dd85e6161d8bcd7d3628e6 ("ARM64: Fix xpcall() error case (really)."). Sergey Bronnikov: * added the description Part of tarantool/tarantool#12134 --- src/vm_mips64.dasc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc index 44fba36c..da187a7a 100644 --- a/src/vm_mips64.dasc +++ b/src/vm_mips64.dasc @@ -1440,15 +1440,16 @@ static void build_subroutines(BuildCtx *ctx) |. nop | |.ffunc xpcall - | daddiu NARGS8:RC, NARGS8:RC, -16 + | daddiu NARGS8:TMP0, NARGS8:RC, -16 | ld CARG1, 0(BASE) | ld CARG2, 8(BASE) - | bltz NARGS8:RC, ->fff_fallback + | bltz NARGS8:TMP0, ->fff_fallback |. lbu TMP1, DISPATCH_GL(hookmask)(DISPATCH) | gettp AT, CARG2 | daddiu AT, AT, -LJ_TFUNC | bnez AT, ->fff_fallback // Traceback must be a function. |. move TMP2, BASE + | move NARGS8:RC, NARGS8:TMP0 | daddiu BASE, BASE, 24 | // Remember active hook before pcall. | srl TMP3, TMP3, HOOK_ACTIVE_SHIFT -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls. 2026-03-12 9:05 [Tarantool-patches] [PATCH luajit 0/3][v3] Fix stack overflow in pcall/xpcall Sergey Bronnikov via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 1/3][v3] MIPS64: Fix xpcall() error case Sergey Bronnikov via Tarantool-patches @ 2026-03-12 8:49 ` Sergey Bronnikov via Tarantool-patches 2026-03-12 9:36 ` Sergey Kaplun via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 3/3][v3] Add stack check to pcall/xpcall Sergey Bronnikov via Tarantool-patches 2 siblings, 1 reply; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2026-03-12 8:49 UTC (permalink / raw) To: tarantool-patches, Sergey Kaplun From: Mike Pall <mike> Thanks to Peter Cawley. (cherry picked from commit d1a2fef8a8f53b0055ee041f7f63d83a27444ffa) Stack overflow can cause a segmentation fault in a vararg function on ARM64 and MIPS64 in LJ_FR2 mode. This happens because the stack check in BC_IFUNCV is off by one on these platforms without the patch. The original stack check for ARM64 and MIPS64 was incorrect: | RA == BASE + (RD=NARGS)*8 + framesize * 8 >= maxstack while the stack check on x86_64 is correct and therefore is not affected by the problem: | RA == BASE + (RD=NARGS+1)*8 + framesize * 8 +8 > maxstack The patch partially fixes the aforementioned issue by bumping LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a vararg function as the __newindex metamethod. A fixup for a number of required slots in `call_init()` was added for consistency with non-GC64 flavor. The check is too strict, so this can't lead to any crash. This patch also corrects the number of redzone slots in luajit-gdb.py to match the updated LJ_STACK_EXTRA and adds the test <gh-1402-call_init-regression.test.lua> that will help to avoid a regression in the future, see details in [1]. Sergey Bronnikov: * added the description and the test for the problem Part of tarantool/tarantool#12134 1. https://github.com/LuaJIT/LuaJIT/issues/1402 --- src/lj_def.h | 2 +- src/lj_dispatch.c | 2 +- src/luajit-gdb.py | 2 +- src/vm_arm64.dasc | 1 + src/vm_mips64.dasc | 1 + .../gh-1402-call_init-regression.test.lua | 36 +++++++++++++ ...048-fix-stack-checks-vararg-calls.test.lua | 53 +++++++++++++++++++ 7 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 test/tarantool-tests/gh-1402-call_init-regression.test.lua create mode 100644 test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua diff --git a/src/lj_def.h b/src/lj_def.h index a5bca6b0..7e4f251e 100644 --- a/src/lj_def.h +++ b/src/lj_def.h @@ -69,7 +69,7 @@ typedef unsigned int uintptr_t; #define LJ_MAX_UPVAL 60 /* Max. # of upvalues. */ #define LJ_MAX_IDXCHAIN 100 /* __index/__newindex chain limit. */ -#define LJ_STACK_EXTRA (5+2*LJ_FR2) /* Extra stack space (metamethods). */ +#define LJ_STACK_EXTRA (5+3*LJ_FR2) /* Extra stack space (metamethods). */ #define LJ_NUM_CBPAGE 1 /* Number of FFI callback pages. */ diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c index a44a5adf..431cb3c2 100644 --- a/src/lj_dispatch.c +++ b/src/lj_dispatch.c @@ -453,7 +453,7 @@ static int call_init(lua_State *L, GCfunc *fn) int numparams = pt->numparams; int gotparams = (int)(L->top - L->base); int need = pt->framesize; - if ((pt->flags & PROTO_VARARG)) need += 1+gotparams; + if ((pt->flags & PROTO_VARARG)) need += 1+LJ_FR2+gotparams; lj_state_checkstack(L, (MSize)need); numparams -= gotparams; return numparams >= 0 ? numparams : 0; diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py index 0ae2a6e0..dab07b35 100644 --- a/src/luajit-gdb.py +++ b/src/luajit-gdb.py @@ -542,7 +542,7 @@ def dump_stack(L, base=None, top=None): top = top or L['top'] stack = mref('TValue *', L['stack']) maxstack = mref('TValue *', L['maxstack']) - red = 5 + 2 * LJ_FR2 + red = 5 + 3 * LJ_FR2 dump = [ '{padding} Red zone: {nredslots: >2} slots {padding}'.format( diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc index 6600e226..5ef37243 100644 --- a/src/vm_arm64.dasc +++ b/src/vm_arm64.dasc @@ -3779,6 +3779,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) | add TMP2, BASE, RC | add LFUNC:CARG3, CARG3, TMP0, lsl #47 | add RA, RA, RC + | sub CARG1, CARG1, #8 | add TMP0, RC, #16+FRAME_VARG | str LFUNC:CARG3, [TMP2], #8 // Store (tagged) copy of LFUNC. | ldr KBASE, [PC, #-4+PC2PROTO(k)] diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc index da187a7a..6c2975b4 100644 --- a/src/vm_mips64.dasc +++ b/src/vm_mips64.dasc @@ -5268,6 +5268,7 @@ static void build_ins(BuildCtx *ctx, BCOp op, int defop) | settp LFUNC:RB, TMP0 | daddu TMP0, RA, RC | sd LFUNC:RB, 0(TMP1) // Store (tagged) copy of LFUNC. + | daddiu TMP2, TMP2, -8 | daddiu TMP3, RC, 16+FRAME_VARG | sltu AT, TMP0, TMP2 | ld KBASE, -4+PC2PROTO(k)(PC) diff --git a/test/tarantool-tests/gh-1402-call_init-regression.test.lua b/test/tarantool-tests/gh-1402-call_init-regression.test.lua new file mode 100644 index 00000000..b20f9e39 --- /dev/null +++ b/test/tarantool-tests/gh-1402-call_init-regression.test.lua @@ -0,0 +1,36 @@ +local tap = require('tap') + +-- A test file to demonstrate a probably quite strict stack +-- check for vararg functions in call_init. +-- See also https://github.com/LuaJIT/LuaJIT/issues/1402 +local test = tap.test('gh-1402-call_init-regression.test.lua'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +local function vararg(...) -- luacheck: no unused + -- None. +end + +-- Make compilation aggressive. +jit.opt.start("hotloop=1") + +local function caller() + -- luacheck: push no unused + local _, _, _, _, _, _, _, _, _, _ + local _, _, _, _, _, _, _, _, _, _ + local _, _, _, _, _, _, _, _, _, _ + -- luacheck: pop + local n = 1 + while n < 3 do + vararg() + n = n + 1 + end +end + +pcall(coroutine.wrap(caller)) + +test:ok(true, 'no assertion for vararg functions in call_init') + +test:done(true) diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua new file mode 100644 index 00000000..3a8ad63d --- /dev/null +++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua @@ -0,0 +1,53 @@ +local tap = require('tap') + +-- A test file to demonstrate a crash due to Lua stack +-- out-of-bounds access, see below testcase descriptions. +-- See also https://github.com/LuaJIT/LuaJIT/issues/1048. +local test = tap.test('lj-1048-fix-stack-checks-vararg-calls') + +test:plan(2) + +-- The test case demonstrates a segmentation fault due to stack +-- overflow by recursive calling `pcall()`. The functions are +-- vararg because the stack check in BC_IFUNCV is off by one on +-- ARM64 and MIPS64 without the patch. +local function prober_1(...) -- luacheck: no unused + -- Any fast function can be used as metamethod, but `type` is + -- convenient here because it works fast and can be used with + -- any data type. Lua function cannot be used since it + -- will check the stack on each invocation. We need to check + -- using of the correct value LJ_STACK_EXTRA slots + -- (5+3*LJ_FR2) = 8 for GC64 mode. + pcall(pcall, pcall, pcall, pcall, pcall, pcall, pcall, pcall, type, 0) +end + +local function looper(prober, n, ...) + prober(...) + return looper(prober, n + 1, n, ...) +end + +pcall(coroutine.wrap(looper), prober_1, 0) + +test:ok(true, 'no stack overflow with recursive pcall') + +-- The testcase demonstrate a segmentation fault due to stack +-- overflow when `pcall()` is used as `__newindex` metamethod. +-- The function is vararg because stack check in BC_IFUNCV is off +-- by one on ARM64 and MIPS64 without the patch. + +-- Any fast function can be used as metamethod, but `type` is +-- convenient here because it works fast and can be used with +-- any data type. Lua function cannot be used since it +-- will check the stack on each invocation. +local t = setmetatable({}, { __newindex = pcall, __call = type }) + +local function prober_2(...) -- luacheck: no unused + -- Invokes `pcall(t, t, t)`. + t[t] = t +end + +pcall(coroutine.wrap(looper), prober_2, 0) + +test:ok(true, 'no stack overflow with metamethod') + +test:done(true) -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls. 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls Sergey Bronnikov via Tarantool-patches @ 2026-03-12 9:36 ` Sergey Kaplun via Tarantool-patches 2026-03-12 12:25 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 1 reply; 8+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2026-03-12 9:36 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches Hi, Sergey! Thanks for the patch! LGTM, after fixing my nits below. Please add the iterational diff for the fixes. On 12.03.26, Sergey Bronnikov wrote: > From: Mike Pall <mike> > > Thanks to Peter Cawley. > > (cherry picked from commit d1a2fef8a8f53b0055ee041f7f63d83a27444ffa) > > Stack overflow can cause a segmentation fault in a vararg > function on ARM64 and MIPS64 in LJ_FR2 mode. This happens > because the stack check in BC_IFUNCV is off by one on these > platforms without the patch. The original stack check > for ARM64 and MIPS64 was incorrect: > > | RA == BASE + (RD=NARGS)*8 + framesize * 8 >= maxstack > > while the stack check on x86_64 is correct and therefore is > not affected by the problem: > > | RA == BASE + (RD=NARGS+1)*8 + framesize * 8 +8 > maxstack Typo: s/ +8/ + 8/ > > The patch partially fixes the aforementioned issue by bumping > LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a > vararg function as the __newindex metamethod. > > A fixup for a number of required slots in `call_init()` was added > for consistency with non-GC64 flavor. The check is too strict, so > this can't lead to any crash. > > This patch also corrects the number of redzone slots in > luajit-gdb.py to match the updated LJ_STACK_EXTRA and adds the test luajit_lldb.py should be updated as well. > <gh-1402-call_init-regression.test.lua> that will help to avoid gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue tracker. > a regression in the future, see details in [1]. Just mention details here like the following: | The patch partially fixes the aforementioned issue by bumping | LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a | vararg function as the __newindex metamethod. | | A fixup for a number of required slots in `call_init()` was added for | consistency with the non-GC64 flavor. The check is too strict (if | comparing the corresponding checks in the VM BC_IFUNCV), so this can't | lead to any crash. To avoid possible regression in the future the | corresponding test is added. | | This patch also corrects the number of redzone slots in luajit-gdb.py | and luajit_lldb.py to match the updated LJ_STACK_EXTRA. > > Sergey Bronnikov: > * added the description and the test for the problem > > Part of tarantool/tarantool#12134 > > 1. https://github.com/LuaJIT/LuaJIT/issues/1402 Please, don't mention the issue during backporting, to avoid messing the issue tracker. > --- > src/lj_def.h | 2 +- > src/lj_dispatch.c | 2 +- > src/luajit-gdb.py | 2 +- > src/vm_arm64.dasc | 1 + > src/vm_mips64.dasc | 1 + > .../gh-1402-call_init-regression.test.lua | 36 +++++++++++++ gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue tracker. > ...048-fix-stack-checks-vararg-calls.test.lua | 53 +++++++++++++++++++ > 7 files changed, 94 insertions(+), 3 deletions(-) > create mode 100644 test/tarantool-tests/gh-1402-call_init-regression.test.lua > create mode 100644 test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua > > diff --git a/src/lj_def.h b/src/lj_def.h > index a5bca6b0..7e4f251e 100644 > --- a/src/lj_def.h > +++ b/src/lj_def.h <snipped> > diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c > index a44a5adf..431cb3c2 100644 > --- a/src/lj_dispatch.c > +++ b/src/lj_dispatch.c <snipped> > diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py > index 0ae2a6e0..dab07b35 100644 > --- a/src/luajit-gdb.py > +++ b/src/luajit-gdb.py <snipped> > diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc > index 6600e226..5ef37243 100644 > --- a/src/vm_arm64.dasc > +++ b/src/vm_arm64.dasc <snipped> > diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc > index da187a7a..6c2975b4 100644 > --- a/src/vm_mips64.dasc > +++ b/src/vm_mips64.dasc <snipped> > diff --git a/test/tarantool-tests/gh-1402-call_init-regression.test.lua b/test/tarantool-tests/gh-1402-call_init-regression.test.lua Please, avoid _ in the file names, lets name it like: lj-1402-vararg-stkov-check-gc64.test.lua Same for the name of the test. > new file mode 100644 > index 00000000..b20f9e39 > --- /dev/null > +++ b/test/tarantool-tests/gh-1402-call_init-regression.test.lua > @@ -0,0 +1,36 @@ > +local tap = require('tap') > + > +-- A test file to demonstrate a probably quite strict stack > +-- check for vararg functions in call_init. This is not about quite strict stack check. We need this to test the behaviour of the LuaJIT while recording the vararg function. Let's rephrase like the following: | -- The test file to verify correctness of stack size check during | -- recording of vararg functions. The test file to verify correctness of stack size check during recording of vararg functions. > +-- See also https://github.com/LuaJIT/LuaJIT/issues/1402 > +local test = tap.test('gh-1402-call_init-regression.test.lua'):skipcond({ gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue tracker. > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +test:plan(1) > + > +local function vararg(...) -- luacheck: no unused Let's use this comment before the vararg declaration. It helps with the _ below as well. > + -- None. > +end > + > +-- Make compilation aggressive. Excess comment. It's quite general approach in our tests. > +jit.opt.start("hotloop=1") Typo: s/"/'/g > + Please add the following comment: | -- This function utilizes the exact amount of stack slots | -- to cause the stack reallocation during `call_init()` in the | -- GC64 mode. > +local function caller() > + -- luacheck: push no unused Lets drop this luacheck suppression, see the comment above. > + local _, _, _, _, _, _, _, _, _, _ > + local _, _, _, _, _, _, _, _, _, _ > + local _, _, _, _, _, _, _, _, _, _ > + -- luacheck: pop > + local n = 1 > + while n < 3 do > + vararg() > + n = n + 1 > + end > +end > + > +pcall(coroutine.wrap(caller)) The pcall is excess lets do it without it: | coroutine.wrap(caller)() > + > +test:ok(true, 'no assertion for vararg functions in call_init') Just mention 'no assertion failure' (this assertion isn't in the `call_init()`, but during recording in `rec_check_slots()`). > + > +test:done(true) > diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua > new file mode 100644 > index 00000000..3a8ad63d > --- /dev/null > +++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua <snipped> > -- > 2.43.0 > -- Best regards, Sergey Kaplun ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls. 2026-03-12 9:36 ` Sergey Kaplun via Tarantool-patches @ 2026-03-12 12:25 ` Sergey Bronnikov via Tarantool-patches 2026-03-12 12:47 ` Sergey Kaplun via Tarantool-patches 0 siblings, 1 reply; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2026-03-12 12:25 UTC (permalink / raw) To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 10443 bytes --] Hi, Sergey, thanks for review! See comments below. The branch was force-pushed. Sergey On 3/12/26 12:36, Sergey Kaplun via Tarantool-patches wrote: > Hi, Sergey! > Thanks for the patch! > > LGTM, after fixing my nits below. > Please add the iterational diff for the fixes. > > On 12.03.26, Sergey Bronnikov wrote: >> From: Mike Pall <mike> >> >> Thanks to Peter Cawley. >> >> (cherry picked from commit d1a2fef8a8f53b0055ee041f7f63d83a27444ffa) >> >> Stack overflow can cause a segmentation fault in a vararg >> function on ARM64 and MIPS64 in LJ_FR2 mode. This happens >> because the stack check in BC_IFUNCV is off by one on these >> platforms without the patch. The original stack check >> for ARM64 and MIPS64 was incorrect: >> >> | RA == BASE + (RD=NARGS)*8 + framesize * 8 >= maxstack >> >> while the stack check on x86_64 is correct and therefore is >> not affected by the problem: >> >> | RA == BASE + (RD=NARGS+1)*8 + framesize * 8 +8 > maxstack > Typo: s/ +8/ + 8/ Fixed, thanks! >> The patch partially fixes the aforementioned issue by bumping >> LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a >> vararg function as the __newindex metamethod. >> >> A fixup for a number of required slots in `call_init()` was added >> for consistency with non-GC64 flavor. The check is too strict, so >> this can't lead to any crash. >> >> This patch also corrects the number of redzone slots in >> luajit-gdb.py to match the updated LJ_STACK_EXTRA and adds the test > luajit_lldb.py should be updated as well. Right, fixed: --- a/src/luajit_lldb.py +++ b/src/luajit_lldb.py @@ -833,7 +833,7 @@ def dump_stack(L, base=None, top=None): top = top or L.top stack = mref(TValuePtr, L.stack) maxstack = mref(TValuePtr, L.maxstack) - red = 5 + 2 * LJ_FR2 + red = 5 + 3 * LJ_FR2 dump = [ '{padding} Red zone: {nredslots: >2} slots {padding}'.format( > >> <gh-1402-call_init-regression.test.lua> that will help to avoid > gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue > tracker. Ah, right, I've overlooked it is a LuaJIT issue, not Tarantool. Thanks! Renamed. >> a regression in the future, see details in [1]. > Just mention details here like the following: > > | The patch partially fixes the aforementioned issue by bumping > | LJ_STACK_EXTRA by 1 to give a space to the entire frame link for a > | vararg function as the __newindex metamethod. > | > | A fixup for a number of required slots in `call_init()` was added for > | consistency with the non-GC64 flavor. The check is too strict (if > | comparing the corresponding checks in the VM BC_IFUNCV), so this can't > | lead to any crash. To avoid possible regression in the future the > | corresponding test is added. > | > | This patch also corrects the number of redzone slots in luajit-gdb.py > | and luajit_lldb.py to match the updated LJ_STACK_EXTRA. > Updated. >> Sergey Bronnikov: >> * added the description and the test for the problem >> >> Part of tarantool/tarantool#12134 >> >> 1.https://github.com/LuaJIT/LuaJIT/issues/1402 > Please, don't mention the issue during backporting, to avoid messing the > issue tracker. > >> --- >> src/lj_def.h | 2 +- >> src/lj_dispatch.c | 2 +- >> src/luajit-gdb.py | 2 +- >> src/vm_arm64.dasc | 1 + >> src/vm_mips64.dasc | 1 + >> .../gh-1402-call_init-regression.test.lua | 36 +++++++++++++ > gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue > tracker. renamed > >> ...048-fix-stack-checks-vararg-calls.test.lua | 53 +++++++++++++++++++ >> 7 files changed, 94 insertions(+), 3 deletions(-) >> create mode 100644 test/tarantool-tests/gh-1402-call_init-regression.test.lua >> create mode 100644 test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua >> >> diff --git a/src/lj_def.h b/src/lj_def.h >> index a5bca6b0..7e4f251e 100644 >> --- a/src/lj_def.h >> +++ b/src/lj_def.h > <snipped> > >> diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c >> index a44a5adf..431cb3c2 100644 >> --- a/src/lj_dispatch.c >> +++ b/src/lj_dispatch.c > <snipped> > >> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py >> index 0ae2a6e0..dab07b35 100644 >> --- a/src/luajit-gdb.py >> +++ b/src/luajit-gdb.py > <snipped> > >> diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc >> index 6600e226..5ef37243 100644 >> --- a/src/vm_arm64.dasc >> +++ b/src/vm_arm64.dasc > <snipped> > >> diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc >> index da187a7a..6c2975b4 100644 >> --- a/src/vm_mips64.dasc >> +++ b/src/vm_mips64.dasc > <snipped> > >> diff --git a/test/tarantool-tests/gh-1402-call_init-regression.test.lua b/test/tarantool-tests/gh-1402-call_init-regression.test.lua > Please, avoid _ in the file names, lets name it like: > > lj-1402-vararg-stkov-check-gc64.test.lua > > Same for the name of the test. ok, renamed once again > >> new file mode 100644 >> index 00000000..b20f9e39 >> --- /dev/null >> +++ b/test/tarantool-tests/gh-1402-call_init-regression.test.lua >> @@ -0,0 +1,36 @@ >> +local tap = require('tap') >> + >> +-- A test file to demonstrate a probably quite strict stack >> +-- check for vararg functions in call_init. > This is not about quite strict stack check. We need this to test the > behaviour of the LuaJIT while recording the vararg function. Let's > rephrase like the following: > > | -- The test file to verify correctness of stack size check during > | -- recording of vararg functions. Updated. > > The test file to verify correctness of stack size check during recording of vararg functions. >> +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1402 >> +local test = tap.test('gh-1402-call_init-regression.test.lua'):skipcond({ > gh- prefix is for the Tarantool issue tracker, use lj- for LuaJIT issue > tracker. renamed --- a/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua +++ b/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua @@ -1,15 +1,16 @@ local tap = require('tap') --- A test file to demonstrate a probably quite strict stack --- check for vararg functions in call_init. +-- The test file to verify correctness of stack size check during +-- recording of vararg functions. -- See also https://github.com/LuaJIT/LuaJIT/issues/1402 -local test = tap.test('gh-1402-call_init-regression.test.lua'):skipcond({ +local test = tap.test('lj-1402-vararg-stkov-check-gc64.test.lua'):skipcond({ ['Test requires JIT enabled'] = not jit.status(), }) >> + ['Test requires JIT enabled'] = not jit.status(), >> +}) >> + >> +test:plan(1) >> + >> +local function vararg(...) -- luacheck: no unused > Let's use this comment before the vararg declaration. > It helps with the _ below as well. Updated: --- a/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua +++ b/test/tarantool-tests/lj-1402-vararg-stkov-check-gc64.test.lua @@ -1,15 +1,16 @@ local tap = require('tap') --- A test file to demonstrate a probably quite strict stack --- check for vararg functions in call_init. +-- The test file to verify correctness of stack size check during +-- recording of vararg functions. -- See also https://github.com/LuaJIT/LuaJIT/issues/1402 -local test = tap.test('gh-1402-call_init-regression.test.lua'):skipcond({ +local test = tap.test('lj-1402-vararg-stkov-check-gc64.test.lua'):skipcond({ ['Test requires JIT enabled'] = not jit.status(), }) test:plan(1) -local function vararg(...) -- luacheck: no unused +-- luacheck: no unused +local function vararg(...) -- None. end > >> + -- None. >> +end >> + >> +-- Make compilation aggressive. > Excess comment. It's quite general approach in our tests. Updated: test:plan(1) -local function vararg(...) -- luacheck: no unused +-- luacheck: no unused +local function vararg(...) -- None. end >> +jit.opt.start("hotloop=1") > Typo: s/"/'/g > >> + end --- Make compilation aggressive. -jit.opt.start("hotloop=1") +jit.opt.start('hotloop=1') local function caller() -- luacheck: push no unused > Please add the following comment: > > | -- This function utilizes the exact amount of stack slots > | -- to cause the stack reallocation during `call_init()` in the > | -- GC64 mode. --- Make compilation aggressive. -jit.opt.start("hotloop=1") +jit.opt.start('hotloop=1') +-- This function utilizes the exact amount of stack slots to cause +-- the stack reallocation during `call_init()` in the GC64 mode. local function caller() -- luacheck: push no unused local _, _, _, _, _, _, _, _, _, _ >> +local function caller() >> + -- luacheck: push no unused > Lets drop this luacheck suppression, see the comment above. Updated: +-- This function utilizes the exact amount of stack slots to cause +-- the stack reallocation during `call_init()` in the GC64 mode. local function caller() - -- luacheck: push no unused local _, _, _, _, _, _, _, _, _, _ local _, _, _, _, _, _, _, _, _, _ local _, _, _, _, _, _, _, _, _, _ - -- luacheck: pop local n = 1 while n < 3 do vararg() >> + local _, _, _, _, _, _, _, _, _, _ >> + local _, _, _, _, _, _, _, _, _, _ >> + local _, _, _, _, _, _, _, _, _, _ >> + -- luacheck: pop >> + local n = 1 >> + while n < 3 do >> + vararg() >> + n = n + 1 >> + end >> +end >> + >> +pcall(coroutine.wrap(caller)) > The pcall is excess lets do it without it: > | coroutine.wrap(caller)() > @@ -29,7 +29,7 @@ local function caller() end end -pcall(coroutine.wrap(caller)) +coroutine.wrap(caller)() test:ok(true, 'no assertion for vararg functions in call_init') >> + >> +test:ok(true, 'no assertion for vararg functions in call_init') > Just mention 'no assertion failure' (this assertion isn't in the > `call_init()`, but during recording in `rec_check_slots()`). Updated: -test:ok(true, 'no assertion for vararg functions in call_init') +test:ok(true, 'no assertion') test:done(true) >> + >> +test:done(true) >> diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua >> new file mode 100644 >> index 00000000..3a8ad63d >> --- /dev/null >> +++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua > <snipped> > >> -- >> 2.43.0 >> [-- Attachment #2: Type: text/html, Size: 17530 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls. 2026-03-12 12:25 ` Sergey Bronnikov via Tarantool-patches @ 2026-03-12 12:47 ` Sergey Kaplun via Tarantool-patches 0 siblings, 0 replies; 8+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2026-03-12 12:47 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: Sergey Bronnikov, tarantool-patches Hi, Sergey! Thanks for the fixes! LGTM! -- Best regards, Sergey Kaplun ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Tarantool-patches] [PATCH luajit 3/3][v3] Add stack check to pcall/xpcall. 2026-03-12 9:05 [Tarantool-patches] [PATCH luajit 0/3][v3] Fix stack overflow in pcall/xpcall Sergey Bronnikov via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 1/3][v3] MIPS64: Fix xpcall() error case Sergey Bronnikov via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls Sergey Bronnikov via Tarantool-patches @ 2026-03-12 8:49 ` Sergey Bronnikov via Tarantool-patches 2026-03-12 10:16 ` Sergey Kaplun via Tarantool-patches 2 siblings, 1 reply; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2026-03-12 8:49 UTC (permalink / raw) To: tarantool-patches, Sergey Kaplun From: Mike Pall <mike> Analyzed by Peter Cawley. (cherry picked from commit a4c1640432a9d8a60624cdc8065b15078c228e36) The patch adds the stack check to fast functions `pcall()` and `xpcall()`. Sergey Bronnikov: * added the description and the test for the problem Part of tarantool/tarantool#12134 --- src/vm_arm.dasc | 7 ++++ src/vm_arm64.dasc | 8 +++++ src/vm_mips.dasc | 10 +++++- src/vm_mips64.dasc | 14 ++++++-- src/vm_ppc.dasc | 9 +++++ src/vm_x64.dasc | 6 ++++ src/vm_x86.dasc | 6 ++++ ...048-fix-stack-checks-vararg-calls.test.lua | 35 ++++++++++++++++++- 8 files changed, 90 insertions(+), 5 deletions(-) diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc index 7095e660..efe9dcb2 100644 --- a/src/vm_arm.dasc +++ b/src/vm_arm.dasc @@ -1201,8 +1201,11 @@ static void build_subroutines(BuildCtx *ctx) |//-- Base library: catch errors ---------------------------------------- | |.ffunc pcall + | ldr RB, L->maxstack + | add INS, BASE, NARGS8:RC | ldrb RA, [DISPATCH, #DISPATCH_GL(hookmask)] | cmp NARGS8:RC, #8 + | cmphs RB, INS | blo ->fff_fallback | tst RA, #HOOK_ACTIVE // Remember active hook before pcall. | mov RB, BASE @@ -1213,7 +1216,11 @@ static void build_subroutines(BuildCtx *ctx) | b ->vm_call_dispatch | |.ffunc_2 xpcall + | ldr RB, L->maxstack + | add INS, BASE, NARGS8:RC | ldrb RA, [DISPATCH, #DISPATCH_GL(hookmask)] + | cmp RB, INS + | blo ->fff_fallback | checkfunc CARG4, ->fff_fallback // Traceback must be a function. | mov RB, BASE | strd CARG12, [BASE, #8] // Swap function and traceback. diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc index 5ef37243..074c1f31 100644 --- a/src/vm_arm64.dasc +++ b/src/vm_arm64.dasc @@ -1166,6 +1166,10 @@ static void build_subroutines(BuildCtx *ctx) |//-- Base library: catch errors ---------------------------------------- | |.ffunc pcall + | ldr TMP1, L->maxstack + | add TMP2, BASE, NARGS8:RC + | cmp TMP1, TMP2 + | blo ->fff_fallback | cmp NARGS8:RC, #8 | ldrb TMP0w, GL->hookmask | blo ->fff_fallback @@ -1185,6 +1189,10 @@ static void build_subroutines(BuildCtx *ctx) | b ->vm_call_dispatch | |.ffunc xpcall + | ldr TMP1, L->maxstack + | add TMP2, BASE, NARGS8:RC + | cmp TMP1, TMP2 + | blo ->fff_fallback | ldp CARG1, CARG2, [BASE] | ldrb TMP0w, GL->hookmask | subs NARGS8:TMP1, NARGS8:RC, #16 diff --git a/src/vm_mips.dasc b/src/vm_mips.dasc index 32caabf7..69d09d52 100644 --- a/src/vm_mips.dasc +++ b/src/vm_mips.dasc @@ -1382,9 +1382,13 @@ static void build_subroutines(BuildCtx *ctx) |//-- Base library: catch errors ---------------------------------------- | |.ffunc pcall + | lw TMP1, L->maxstack + | addu TMP2, BASE, NARGS8:RC | lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH) | beqz NARGS8:RC, ->fff_fallback - | move TMP2, BASE + |. sltu AT, TMP1, TMP2 + | bnez AT, ->fff_fallback + |. move TMP2, BASE | addiu BASE, BASE, 8 | // Remember active hook before pcall. | srl TMP3, TMP3, HOOK_ACTIVE_SHIFT @@ -1394,8 +1398,12 @@ static void build_subroutines(BuildCtx *ctx) |. addiu NARGS8:RC, NARGS8:RC, -8 | |.ffunc xpcall + | lw TMP1, L->maxstack + | addu TMP2, BASE, NARGS8:RC | sltiu AT, NARGS8:RC, 16 | lw CARG4, 8+HI(BASE) + | sltu TMP1, TMP1, TMP2 + | or AT, AT, TMP1 | bnez AT, ->fff_fallback |. lw CARG3, 8+LO(BASE) | lw CARG1, LO(BASE) diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc index 6c2975b4..4e60ee07 100644 --- a/src/vm_mips64.dasc +++ b/src/vm_mips64.dasc @@ -1418,8 +1418,12 @@ static void build_subroutines(BuildCtx *ctx) |//-- Base library: catch errors ---------------------------------------- | |.ffunc pcall + | ld TMP1, L->maxstack + | daddu TMP2, BASE, NARGS8:RC + | sltu AT, TMP1, TMP2 + | bnez AT, ->fff_fallback + |. lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH) | daddiu NARGS8:RC, NARGS8:RC, -8 - | lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH) | bltz NARGS8:RC, ->fff_fallback |. move TMP2, BASE | daddiu BASE, BASE, 16 @@ -1440,8 +1444,12 @@ static void build_subroutines(BuildCtx *ctx) |. nop | |.ffunc xpcall - | daddiu NARGS8:TMP0, NARGS8:RC, -16 - | ld CARG1, 0(BASE) + | ld TMP1, L->maxstack + | daddu TMP2, BASE, NARGS8:RC + | sltu AT, TMP1, TMP2 + | bnez AT, ->fff_fallback + |. ld CARG1, 0(BASE) + | daddiu NARGS8:RC, NARGS8:RC, -16 | ld CARG2, 8(BASE) | bltz NARGS8:TMP0, ->fff_fallback |. lbu TMP1, DISPATCH_GL(hookmask)(DISPATCH) diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc index 980ad897..f2ea933b 100644 --- a/src/vm_ppc.dasc +++ b/src/vm_ppc.dasc @@ -1755,8 +1755,12 @@ static void build_subroutines(BuildCtx *ctx) |//-- Base library: catch errors ---------------------------------------- | |.ffunc pcall + | lwz TMP1, L->maxstack + | add TMP2, BASE, NARGS8:RC | cmplwi NARGS8:RC, 8 | lbz TMP3, DISPATCH_GL(hookmask)(DISPATCH) + | cmplw cr1, TMP1, TMP2 + | cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt | blt ->fff_fallback | mr TMP2, BASE | la BASE, 8(BASE) @@ -1767,14 +1771,19 @@ static void build_subroutines(BuildCtx *ctx) | b ->vm_call_dispatch | |.ffunc xpcall + | lwz TMP1, L->maxstack + | add TMP2, BASE, NARGS8:RC | cmplwi NARGS8:RC, 16 | lwz CARG3, 8(BASE) + | cmplw cr1, TMP1, TMP2 |.if FPU | lfd FARG2, 8(BASE) + | cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt | lfd FARG1, 0(BASE) |.else | lwz CARG1, 0(BASE) | lwz CARG2, 4(BASE) + | cror 4*cr0+lt, 4*cr0+lt, 4*cr1+lt | lwz CARG4, 12(BASE) |.endif | blt ->fff_fallback diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc index 8b6781a6..c57b76b7 100644 --- a/src/vm_x64.dasc +++ b/src/vm_x64.dasc @@ -1545,6 +1545,9 @@ static void build_subroutines(BuildCtx *ctx) |//-- Base library: catch errors ---------------------------------------- | |.ffunc_1 pcall + | mov L:RB, SAVE_L + | lea RA, [BASE+NARGS:RD*8] + | cmp RA, L:RB->maxstack; ja ->fff_fallback | lea RA, [BASE+16] | sub NARGS:RDd, 1 | mov PCd, 16+FRAME_PCALL @@ -1563,6 +1566,9 @@ static void build_subroutines(BuildCtx *ctx) | jmp ->vm_call_dispatch | |.ffunc_2 xpcall + | mov L:RB, SAVE_L + | lea RA, [BASE+NARGS:RD*8] + | cmp RA, L:RB->maxstack; ja ->fff_fallback | mov LFUNC:RA, [BASE+8] | checktp_nc LFUNC:RA, LJ_TFUNC, ->fff_fallback | mov LFUNC:RB, [BASE] // Swap function and traceback. diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc index 7c11c78e..36804d11 100644 --- a/src/vm_x86.dasc +++ b/src/vm_x86.dasc @@ -1914,6 +1914,9 @@ static void build_subroutines(BuildCtx *ctx) |//-- Base library: catch errors ---------------------------------------- | |.ffunc_1 pcall + | mov L:RB, SAVE_L + | lea RA, [BASE+NARGS:RD*8] + | cmp RA, L:RB->maxstack; ja ->fff_fallback | lea RA, [BASE+8] | sub NARGS:RD, 1 | mov PC, 8+FRAME_PCALL @@ -1925,6 +1928,9 @@ static void build_subroutines(BuildCtx *ctx) | jmp ->vm_call_dispatch | |.ffunc_2 xpcall + | mov L:RB, SAVE_L + | lea RA, [BASE+NARGS:RD*8] + | cmp RA, L:RB->maxstack; ja ->fff_fallback | cmp dword [BASE+12], LJ_TFUNC; jne ->fff_fallback | mov RB, [BASE+4] // Swap function and traceback. | mov [BASE+12], RB diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua index 3a8ad63d..ad8b151b 100644 --- a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua +++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua @@ -5,7 +5,7 @@ local tap = require('tap') -- See also https://github.com/LuaJIT/LuaJIT/issues/1048. local test = tap.test('lj-1048-fix-stack-checks-vararg-calls') -test:plan(2) +test:plan(5) -- The test case demonstrates a segmentation fault due to stack -- overflow by recursive calling `pcall()`. The functions are @@ -50,4 +50,37 @@ pcall(coroutine.wrap(looper), prober_2, 0) test:ok(true, 'no stack overflow with metamethod') +-- The testcases demonstrates a stack overflow in +-- `pcall()`/xpcall()` triggered using metamethod `__call`. + +t = coroutine.wrap(setmetatable)({}, { __call = pcall }) + +test:ok(true, 'no stack overflow with metamethod __call with pcall()') + +t = coroutine.wrap(setmetatable)({}, { __call = xpcall }) + +test:ok(true, 'no stack overflow with metamethod __call with xpcall()') + +-- The testcase demonstrates a stack overflow in +-- `pcall()`/`xpcall()` similar to the first testcase, but it is +-- triggered using `unpack()`. + +t = {} +local function f() + return pcall(unpack(t)) +end + +-- The problem is only reproduced on LuaJIT GC64 and is best +-- reproduced under Valgrind than AddressSanitizer. The chosen +-- value was found experimentally and always results in an attempt +-- to write beyond the allocated memory. +local N_ITERATIONS = 200 + +for i = 1, N_ITERATIONS do + t[i], t[i + 1], t[i + 2] = pcall, type, {} + coroutine.wrap(f)() +end + +test:ok(true, 'no stack overflow with unpacked pcalls') + test:done(true) -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit 3/3][v3] Add stack check to pcall/xpcall. 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 3/3][v3] Add stack check to pcall/xpcall Sergey Bronnikov via Tarantool-patches @ 2026-03-12 10:16 ` Sergey Kaplun via Tarantool-patches 0 siblings, 0 replies; 8+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2026-03-12 10:16 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches Hi, Sergey! Thanks for the patch! Please, fix my comments below. Don't forget to add the corresponding iterative changes. On 12.03.26, Sergey Bronnikov wrote: > From: Mike Pall <mike> > > Analyzed by Peter Cawley. > > (cherry picked from commit a4c1640432a9d8a60624cdc8065b15078c228e36) > > The patch adds the stack check to fast functions `pcall()` and > `xpcall()`. Please add more verbose description: | (cherry picked from commit a4c1640432a9d8a60624cdc8065b15078c228e36) | | The `pcall()` and `xpcall()` calls in GC64 mode require 2 slots. This | means that all arguments should be moved up during emitting of the frame | link to the stack. Hence, this may cause stack overflow without the | corresponding check. | | This patch adds the corresponding checks to the VM. Non-GC64 VMs are | updated as well for the consistency. > > Sergey Bronnikov: > * added the description and the test for the problem > > Part of tarantool/tarantool#12134 > --- > src/vm_arm.dasc | 7 ++++ > src/vm_arm64.dasc | 8 +++++ > src/vm_mips.dasc | 10 +++++- > src/vm_mips64.dasc | 14 ++++++-- > src/vm_ppc.dasc | 9 +++++ > src/vm_x64.dasc | 6 ++++ > src/vm_x86.dasc | 6 ++++ > ...048-fix-stack-checks-vararg-calls.test.lua | 35 ++++++++++++++++++- > 8 files changed, 90 insertions(+), 5 deletions(-) > > diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc > index 7095e660..efe9dcb2 100644 > --- a/src/vm_arm.dasc > +++ b/src/vm_arm.dasc <snipped> > diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc > index 5ef37243..074c1f31 100644 > --- a/src/vm_arm64.dasc > +++ b/src/vm_arm64.dasc <snipped> > diff --git a/src/vm_mips.dasc b/src/vm_mips.dasc > index 32caabf7..69d09d52 100644 > --- a/src/vm_mips.dasc > +++ b/src/vm_mips.dasc <snipped> > diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc > index 6c2975b4..4e60ee07 100644 > --- a/src/vm_mips64.dasc > +++ b/src/vm_mips64.dasc > @@ -1418,8 +1418,12 @@ static void build_subroutines(BuildCtx *ctx) > |//-- Base library: catch errors ---------------------------------------- > | > |.ffunc pcall > + | ld TMP1, L->maxstack > + | daddu TMP2, BASE, NARGS8:RC > + | sltu AT, TMP1, TMP2 > + | bnez AT, ->fff_fallback > + |. lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH) > | daddiu NARGS8:RC, NARGS8:RC, -8 > - | lbu TMP3, DISPATCH_GL(hookmask)(DISPATCH) > | bltz NARGS8:RC, ->fff_fallback > |. move TMP2, BASE > | daddiu BASE, BASE, 16 > @@ -1440,8 +1444,12 @@ static void build_subroutines(BuildCtx *ctx) > |. nop > | > |.ffunc xpcall > - | daddiu NARGS8:TMP0, NARGS8:RC, -16 This neglets the first patch in the series. See the comment below. > - | ld CARG1, 0(BASE) > + | ld TMP1, L->maxstack > + | daddu TMP2, BASE, NARGS8:RC > + | sltu AT, TMP1, TMP2 > + | bnez AT, ->fff_fallback > + |. ld CARG1, 0(BASE) > + | daddiu NARGS8:RC, NARGS8:RC, -16 This line is incorrect. This neglets the 1st patch in the series. It should be | | daddiu NARGS8:TMP0, NARGS8:RC, -16 > | ld CARG2, 8(BASE) > | bltz NARGS8:TMP0, ->fff_fallback > |. lbu TMP1, DISPATCH_GL(hookmask)(DISPATCH) > diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc > index 980ad897..f2ea933b 100644 > --- a/src/vm_ppc.dasc > +++ b/src/vm_ppc.dasc <snipped> > diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc > index 8b6781a6..c57b76b7 100644 > --- a/src/vm_x64.dasc > +++ b/src/vm_x64.dasc <snipped> > diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc > index 7c11c78e..36804d11 100644 > --- a/src/vm_x86.dasc > +++ b/src/vm_x86.dasc <snipped> > diff --git a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua > index 3a8ad63d..ad8b151b 100644 > --- a/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua > +++ b/test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua > @@ -5,7 +5,7 @@ local tap = require('tap') > -- See also https://github.com/LuaJIT/LuaJIT/issues/1048. > local test = tap.test('lj-1048-fix-stack-checks-vararg-calls') > > -test:plan(2) > +test:plan(5) > > -- The test case demonstrates a segmentation fault due to stack > -- overflow by recursive calling `pcall()`. The functions are > @@ -50,4 +50,37 @@ pcall(coroutine.wrap(looper), prober_2, 0) > > test:ok(true, 'no stack overflow with metamethod') > > +-- The testcases demonstrates a stack overflow in > +-- `pcall()`/xpcall()` triggered using metamethod `__call`. > + > +t = coroutine.wrap(setmetatable)({}, { __call = pcall }) I've meant the following: | t = setmetatable({}, { __call = pcall }) | coroutine.wrap(function() t() end)() > + > +test:ok(true, 'no stack overflow with metamethod __call with pcall()') > + > +t = coroutine.wrap(setmetatable)({}, { __call = xpcall }) I've meant the following: | t = setmetatable({}, { __call = xpcall }) | coroutine.wrap(function() t() end)() But this won't work since the second amount of xpcall must be the function. So, this test case is invalid. We must to duplicate the second approach with `xpcall()` This works fine. | LUA_PATH="src/?.lua;;" gdb --args src/luajit -e ' | local t = {} | local function xpcall_wrapper() | return xpcall(unpack(t)) | end | | local N_ITERATIONS = 200 | | for i = 1, N_ITERATIONS do | t[i], t[i + 1], t[i + 2] = xpcall, type, {} | coroutine.wrap(xpcall_wrapper)() | end | ' > + > +test:ok(true, 'no stack overflow with metamethod __call with xpcall()') > + > +-- The testcase demonstrates a stack overflow in > +-- `pcall()`/`xpcall()` similar to the first testcase, but it is > +-- triggered using `unpack()`. > + > +t = {} > +local function f() Lets name it `pcall_wrapper()` > + return pcall(unpack(t)) > +end > + > +-- The problem is only reproduced on LuaJIT GC64 and is best Typo: s/best/better/ > +-- reproduced under Valgrind than AddressSanitizer. The chosen > +-- value was found experimentally and always results in an attempt > +-- to write beyond the allocated memory. > +local N_ITERATIONS = 200 > + > +for i = 1, N_ITERATIONS do > + t[i], t[i + 1], t[i + 2] = pcall, type, {} > + coroutine.wrap(f)() > +end > + > +test:ok(true, 'no stack overflow with unpacked pcalls') > + > test:done(true) > -- > 2.43.0 > -- Best regards, Sergey Kaplun ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-03-12 12:46 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-03-12 9:05 [Tarantool-patches] [PATCH luajit 0/3][v3] Fix stack overflow in pcall/xpcall Sergey Bronnikov via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 1/3][v3] MIPS64: Fix xpcall() error case Sergey Bronnikov via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 2/3][v3] LJ_FR2: Fix stack checks in vararg calls Sergey Bronnikov via Tarantool-patches 2026-03-12 9:36 ` Sergey Kaplun via Tarantool-patches 2026-03-12 12:25 ` Sergey Bronnikov via Tarantool-patches 2026-03-12 12:47 ` Sergey Kaplun via Tarantool-patches 2026-03-12 8:49 ` [Tarantool-patches] [PATCH luajit 3/3][v3] Add stack check to pcall/xpcall Sergey Bronnikov via Tarantool-patches 2026-03-12 10:16 ` Sergey Kaplun via Tarantool-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox