Re: [Tarantool-patches] [PATCH luajit] Fix overflow check in unpack().

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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


Hi, Sergey!
Thanks for the fixes!
LGTM
 
--
Best regards,
Maxim Kokryashkin
 
  
>Среда, 7 сентября 2022, 14:59 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
> 
>Hi, Maxim!
>
>Thanks for the review!
>
>On 06.09.22, Maxim Kokryashkin wrote:
>>
>> Hi, Sergey!
>> Thanks for the patch!
>> Please consider my comments below:
>>  
>> >from Sergey Kaplun via Tarantool-patches < tarantool-patches@dev.tarantool.org >:
>> > 
>> >From: Mike Pall <mike>
>> >
>> >Thanks to HybridDog.
>> >
>> >When build with optimization compiler may throw away overflow check in
>> >`unpack()` base library function.
>> Typo: s/build with optimization/built with optimization,
>> Also,  I think we should mention the specific optimization that causes the mentioned behavior
>> since it is not mentioned in both the LuaJIT’s issue and the original Lua issue.
>
>Fixed.
>The new commit message is the following:
>I mention the -fstrict-overflow flag as the crucial one (obviously
>some more are needed).
>
>| Fix overflow check in unpack().
>|
>| Thanks to HybridDog.
>|
>| (cherry picked from commit 179cf2eb84fef2b9a524469c3c8cc49363b8fb10)
>|
>| When built with -O2 optimization flag (includes -fstrict-overflow)
>| compiler throws away overflow check in `unpack()` base library function.
>|
>| This patch prevents aforementioned error by comparing the unsigned
>| amount of values to unpack with `LUAI_MAXCSTACK` instead of 0.
>|
>| Sergey Kaplun:
>| * added the description and the test for the problem
>|
>| Part of tarantool/tarantool#7230
>
>Branch is force-pushed.
>
>> >
>> >This patch prevents aforementioned error by comparing the unsigned
>> >amount of values to unpack with `LUAI_MAXCSTACK` instead of 0.
>> >
>> >Sergey Kaplun:
>> >* added the description and the test for the problem
>> >
>> >Part of tarantool/tarantool#7230
>> >---
>> >
>> >Issue/PR:
>> >*  https://github.com/LuaJIT/LuaJIT/pull/574
>> >*  https://github.com/tarantool/tarantool/issues/7230
>> >Branch:  https://github.com/tarantool/luajit/tree/skaplun/lj-574-overflow-unpack-full-ci
>> >PR:  https://github.com/tarantool/tarantool/pull/7596
>> >
>> > src/lib_base.c | 6 ++++--
>> > test/tarantool-tests/lj-574-overflow-unpack.test.lua | 12 ++++++++++++
>
><snipped>
>
>> >--
>> >2.34.1
>> --
>> Best regards,
>> Maxim Kokryashkin
>
>--
>Best regards,
>Sergey Kaplun
 

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