Re: [Tarantool-patches] [PATCH luajit 2/3][v2] LJ_FR2: Fix stack checks in vararg calls.

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

Sergey,

On 16.02.26, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for review!
> 
> On 2/11/26 11:30, Sergey Kaplun via Tarantool-patches wrote:
> > On 10.12.25, Sergey Bronnikov wrote:

<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
> >> @@ -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;
> > Let's add an additional test for this part of code (since we don't have
> > any). It may be taken from [1]. It doesn't fail now, but we may cover
> > this branch more precise.
> 
> Don't get what do you mean.
> 
> true branch in gc32 is covered by the following tests:
> 
> test/LuaJIT-tests
> test/PUC-Rio-Lua-5.1-tests
> test/tarantool-c-tests/lj-1087-vm-handler-call.c_test
> test/tarantool-tests/fix-ff-select-recording.test.lua
> test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
> test/tarantool-tests/fix-slot-check-for-mm-record.test.lua
> test/tarantool-tests/fix-slots-overflow-for-varg-record.test.lua
> test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
> test/tarantool-tests/lj-1024-varg-maxslot.test.lua
> test/tarantool-tests/lj-1025-tsetm-maxslot.test.lua
> test/tarantool-tests/lj-1026-arm64-invalid-hrefk-offset-check.test.lua
> test/tarantool-tests/lj-1046-fix-bc-varg-recording.test.lua
> test/tarantool-tests/lj-1164-record-meta-concat-varg-pcall.test.lua
> test/tarantool-tests/lj-1295-bad-renames-for-sunk-values.test.lua
> test/tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua
> test/tarantool-tests/lj-704-bc-varg-use-def.test.lua

Just the true branch isn't enough. We need the true branch when the
stack needs to be reallocated, like in the [1]. When I check this issue
(with 1 removed) none of our tests catches the incorrect behaviour. You
may refer to the test like lj-1402-vararg-realloc-check.test.lua. The
comment in the test should clarify that this is to avoid regressions in
the future.

<snipped>

> > [1]:https://github.com/LuaJIT/LuaJIT/issues/1402#issue-3569942423
> >

-- 
Best regards,
Sergey Kaplun