<HTML><BODY><div>Hi!</div><div>Thanks for the patch!</div><div>LGTM, except for a few nits regarding the commit message.</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_16889862731928372732_BODY">From: Mike Pall <mike><br><br>Analyzed by Sergey Kaplun.<br><br>(cherry-picked from commit 94ada59628dd6ce5d6d2dad1d35a68ad30127f53)<br><br>While recording BC_VARG `J->maxslot` isn't shrunk to the effective stack</div></div></div></div></blockquote><div>Typo: s/shrunk/shrinking</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>top. This leads to dead value stored in the JIT slots and the following</div></div></div></div></blockquote><div>Typo: s/value/values/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>assertion failure for these slots check in `rec_check_slots()`. Note,<br>that `rec_varg()` modifies `maxslot` only under the condition that<br>`maxslot` should be increased, but the dead values are left for the<br>opposite case.<br><br>This patch removes the condition inside `rec_varg()` only for the case<br>when varargs are not defined on trace (`framedepth` is 0), but the<br>similar issue still occurs for the case when vararg are defined on the</div></div></div></div></blockquote><div>Typo: s/vararg/varagrs/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>trace.<br><br>Sergey Kaplun:<br>* added the description and the test for the problem<br><br>Part of tarantool/tarantool#8825<br>---<br> src/lj_record.c | 3 +--<br> .../lj-1024-varg-maxslot.test.lua | 23 +++++++++++++++++++<br> 2 files changed, 24 insertions(+), 2 deletions(-)<br> create mode 100644 test/tarantool-tests/lj-1024-varg-maxslot.test.lua<br><br>diff --git a/src/lj_record.c b/src/lj_record.c<br>index a90cba77..112524d3 100644<br>--- a/src/lj_record.c<br>+++ b/src/lj_record.c<br>@@ -1812,8 +1812,7 @@ static void rec_varg(jit_State *J, BCReg dst, ptrdiff_t nresults)<br> }<br> for (i = nvararg; i < nresults; i++)<br> J->base[dst+i] = TREF_NIL;<br>- if (dst + (BCReg)nresults > J->maxslot)<br>- J->maxslot = dst + (BCReg)nresults;<br>+ J->maxslot = dst + (BCReg)nresults;<br> } else if (select_detect(J)) { /* y = select(x, ...) */<br> TRef tridx = J->base[dst-1];<br> TRef tr = TREF_NIL;<br>diff --git a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua<br>new file mode 100644<br>index 00000000..14270595<br>--- /dev/null<br>+++ b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua<br>@@ -0,0 +1,23 @@<br>+local tap = require('tap')<br>+local test = tap.test('lj-noticket-varg-usedef'):skipcond({<br>+ ['Test requires JIT enabled'] = not jit.status(),<br>+})<br>+<br>+test:plan(1)<br>+<br>+jit.opt.start('hotloop=1')<br>+<br>+local counter = 0<br>+-- luacheck: ignore<br>+local anchor<br>+while counter < 3 do<br>+ counter = counter + 1<br>+ -- BC_VARG 5 1 0. `...` is nil (argument for the script).<br>+ -- luacheck: ignore<br>+ -- XXX: some condition to use several slots on the Lua stack.<br>+ anchor = 1 >= 1, ...<br>+end<br>+<br>+test:ok(true, 'BC_VARG recording 0th frame depth')<br>+<br>+os.exit(test:check() and 0 or 1)<br>--<br>2.34.1</div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div></div></blockquote></BODY></HTML>