

<HTML><BODY><div>Hi! Thanks for the comments. </div><div> </div><div>I agree that it is strange to add this test, as long as it does not test</div><div>anything, so I propose to remove it.</div><div> </div><div>Best regards,</div><div>Maxim Kokryashkin</div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16339662080987608959_BODY">Hi, Maxim!<br><br>Thanks for the patch!<br><br>Please consider my comments below.<br><br>On 30.09.21, Maxim Kokryashkin wrote:<br>> The first fiber in Tarantool has only 512Kb of the stack which is not enough to<br>> handle such a deep call chain.<br>> The test is adapted to Tarantool by decreasing the string length.<br>><br>> Closes tarantool/tarantool#5782<br>> Part of tarantool/tarantool#5845<br>> Part of tarantool/tarantool#4473<br><br>Looks like it should be 5870 instead 4473. Also, 5845 is already<br>closed.<br><br>> ---<br><br>Please show the Tarantool branch as well, to show that problem is gone.<br><br>> GitHub branch: <a href="https://github.com/tarantool/luajit/tree/fckxorg/gh-5782-adapt-deep-nest-gsub-PUC-Rio" target="_blank">https://github.com/tarantool/luajit/tree/fckxorg/gh-5782-adapt-deep-nest-gsub-PUC-Rio</a><br>> Issue: <a href="https://github.com/tarantool/tarantool/issues/5782" target="_blank">https://github.com/tarantool/tarantool/issues/5782</a><br>><br>> test/PUC-Rio-Lua-5.1-tests/pm.lua | 9 ++++-----<br>> 1 file changed, 4 insertions(+), 5 deletions(-)<br>><br>> diff --git a/test/PUC-Rio-Lua-5.1-tests/pm.lua b/test/PUC-Rio-Lua-5.1-tests/pm.lua<br>> index e364ff9d..7da3ef4a 100644<br>> --- a/test/PUC-Rio-Lua-5.1-tests/pm.lua<br>> +++ b/test/PUC-Rio-Lua-5.1-tests/pm.lua<br>> @@ -206,12 +206,11 @@ function rev (s)<br>> return string.gsub(s, "(.)(.+)", function (c,s1) return rev(s1)..c end)<br>> end<br>><br>> -local x = string.rep('012345', 10)<br>> --- FIXME: The first Tarantool's fiber has only 512Kb of stack.<br>> --- It is not enough for this recursive call.<br>> +-- This test is adapted to match the stack size (512Kb) of the first fiber in<br>> +-- Tarantool.<br>> -- See also <a href="https://github.com/tarantool/tarantool/issues/5782" target="_blank">https://github.com/tarantool/tarantool/issues/5782</a>.<br>> --- The test is disabled for Tarantool binary.<br>> --- assert(rev(rev(x)) == x)<br>> +local x = string.rep('01234', 10)<br>> +assert(rev(rev(x)) == x)<br><br>The patch is looks OK to me, but the main problem is still here:<br>LuaJIT is badly managing C stack overflow. The same chunk rases an error<br>for Lua 5.1, but crashes for LuaJIT.<br><br>| $ luajit -e 'local function rev (s) return string.gsub(s, "(.)(.+)", function (c,s1) return rev(s1)..c end) end local x = string.rep("0", 1000) rev(x)'<br>| Segmentation fault<br><br>| $ lua -e 'local function rev (s) return string.gsub(s, "(.)(.+)", function (c,s1) return rev(s1)..c end) end local x = string.rep("0", 1000) rev(x)'<br>| lua: C stack overflow<br>| stack traceback:<br>| [C]: in function 'gsub'<br>| (command line):1: in function 'rev'<br>| (command line):1: in function <(command line):1><br>| [C]: in function 'gsub'<br>| (command line):1: in function 'rev'<br>| (command line):1: in function <(command line):1><br>| [C]: in function 'gsub'<br>| (command line):1: in function 'rev'<br>| (command line):1: in function <(command line):1><br>| [C]: in function 'gsub'<br>| ...<br>| [C]: in function 'gsub'<br>| (command line):1: in function 'rev'<br>| (command line):1: in function <(command line):1><br>| [C]: in function 'gsub'<br>| (command line):1: in function 'rev'<br>| (command line):1: in function <(command line):1><br>| [C]: in function 'gsub'<br>| (command line):1: in function 'rev'<br>| (command line):1: in main chunk<br>| [C]: ?<br><br>So I suppose it is strange to add test that tests nothing.<br><br>Thoughts?<br><br>><br>><br>> -- gsub with tables<br>> --<br>> 2.33.0<br>><br><br>--<br>Best regards,<br>Sergey Kaplun</div></div></div></div></blockquote><div> </div></div></blockquote></BODY></HTML>