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