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

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

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

Hi, Sergey,

see the answer below

Sergey

On 3/4/26 12:49, Sergey Kaplun wrote:
> 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.

A new file with test was added (see 
test/tarantool-tests/gh-1402-call_init-regression.test.lua).

The branch was force-pushed.

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

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