Hi, Sergey, thanks for review! Please consider my three comments below. Sergey On 10/27/25 11:16, Sergey Kaplun wrote: > Hi, Sergey! > Thanks for the fixes! > Please consider my comments below. > Also, please send the next version via v2 series to simplify the > review. > > On 23.09.25, Sergey Bronnikov wrote: >> Hi, Sergey, >> >> thanks for review! Please see my comments below. >> >> Sergey >> >> On 9/1/25 16:07, Sergey Kaplun via Tarantool-patches wrote: >>> Hi, Sergey! >>> Thanks for the patch! >>> Please consider my comments below. >>> >>> On 27.08.25, Sergey Bronnikov wrote: > > >>>> Sergey Bronnikov: >>>> * added the description and the test for the problem >>>> >>>> Part of tarantool/tarantool#11691 >>>> --- >>>> src/lj_def.h | 2 +- >>>> src/lj_dispatch.c | 2 +- >>>> src/vm_arm64.dasc | 1 + >>>> src/vm_mips64.dasc | 1 + >>>> ...048-fix-stack-checks-vararg-calls.test.lua | 56 +++++++++++++++++++ >>>> 5 files changed, 60 insertions(+), 2 deletions(-) >>>> 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 >>> >>> >>>> 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; >>> I can't see the test related to this change. Not `prober_1()` nor >>> `prober_2()` lead to the assertion failure for x86_64 or aarch64 without >>> it. The LJ_FR2 check was added for consistency with non-gc64 flavor, the commit message was updated >    A fixup for a number of required slots in `call_init()` was added >   for consistency with non-gc64 flavor. Also, there is an issue [1] about inconsistencies in stack checks. 1. https://github.com/LuaJIT/LuaJIT/issues/1402 >> Please check again. Both testcases trigger segfault on AArch64 (odroid). > Double checked: > | root@odroid:/home/skaplun/lj-1048-review# git diff > | diff --git a/src/lj_dispatch.c b/src/lj_dispatch.c > | index 431cb3c2..a44a5adf 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+LJ_FR2+gotparams; > | + if ((pt->flags & PROTO_VARARG)) need += 1+gotparams; > | lj_state_checkstack(L, (MSize)need); > | numparams -= gotparams; > | return numparams >= 0 ? numparams : 0; > | Test project /home/skaplun/lj-1048-review > | Start 118: test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua > | 1/1 Test #118: test/tarantool-tests/lj-1048-fix-stack-checks-vararg-calls.test.lua ... Passed 3.38 sec > | > | 100% tests passed, 0 tests failed out of 1 > | > | Label Time Summary: > | tarantool-tests = 3.38 sec*proc (1 test) > | > | Total Test time (real) = 3.42 sec > > > >>>> +-- patch. >>>> +local function prober_1(...) -- luacheck: no unused >>>> + pcall(pcall, pcall, pcall, pcall, pcall, pcall, pcall, pcall, pairs, {}) >>>> +end >>> Why do we want to use probber_1 here? Why is this different from the >>> second example? Only because of the metamethods? `prober_1` triggers the issue by using recursive (p)call > Still need an explanation. > >>> If we want to keep it, please describe why we need at least 9 pcall-s. >> As I got right, exactly this number of pcall's is needed to trigger a >> stack overflow. > Yes, but why 9 is minimum number of pcall's when the issue is reproduced? > > The number depends on a previous value of LJ_STACK_EXTRA. LJ_STACK_EXTRA, is an "overlay" on top of the stack, and for a buffer overflow you need at least 8 + 1 frames to write slots above this "overlay". These pcalls generates additional frames.