<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi, Max!</p>
<p><br>
</p>
<p>Thanks for review! Added more comments to the test and commit
message.</p>
<p>New changes force-pushed to the branch. Please take a look.<br>
</p>
<p><br>
</p>
<p>S.<br>
</p>
<div class="moz-cite-prefix">On 6/7/23 14:35, Maxim Kokryashkin
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1686137725.949044142@f485.i.mail.ru">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div>Hi, Sergey and Sergey!</div>
<div> </div>
<div>
<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_16860561301802465383_BODY">Hi, Sergey!<br>
Thanks for the patch!<br>
Please, consider my comments below.<br>
<br>
On 30.05.23, Sergey Bronnikov wrote:<br>
> From: Sergey Bronnikov <<a
href="/compose?To=sergeyb@tarantool.org"
moz-do-not-send="true">sergeyb@tarantool.org</a>><br>
><br>
> Contributed by XmiliaH.<br>
><br>
> (cherry-picked from commit
93a65d3cc263aef2d2feb3d7ff2206aca3bee17e)<br>
><br>
> After emitting bytecode instruction BC_FNEW
fixup is not required,</div>
</div>
</div>
</div>
</blockquote>
<div>Typo: s/bytecode/the bytecode</div>
</div>
</blockquote>
</blockquote>
<p>Fixed, thanks!</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:1686137725.949044142@f485.i.mail.ru">
<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>
<div class="js-helper js-readmsg-msg">
<div>
<div>> because FuncState will set a flag
PROTO_CHILD that will trigger emitting<br>
> a pair of instructions BC_UCLO and BC_RET (see
<src/lj_parse.c:2355>)<br>
> and BC_RET will close all upvalues from base
equal to 0.<br>
<br>
This part describes why replacing UCLO with FNEW is
good enough and<br>
better than just deleting<br>
| case BC_UCLO: return;<br>
But the original problem is that some of BC_RET are
not fixup-ed, due to<br>
early return, if UCLO is obtained before, those
leads to VM<br>
inconsistency after return from the function.
Please, mention this too.</div>
</div>
</div>
</div>
</blockquote>
<div>Agree here, it is hard to get what the patch is about
from that description,</div>
<div>without diving into the changes.</div>
</div>
</blockquote>
</blockquote>
<p>Added more details.</p>
<p><br>
</p>
<snipped><br>
<blockquote type="cite"
cite="mid:1686137725.949044142@f485.i.mail.ru">
<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>
<div class="js-helper js-readmsg-msg">
<div>
<div>Also, before the patch I got the following
assertion in JIT:<br>
<br>
| LUA_PATH="src/?.lua;;" src/luajit -Ohotloop=1 -e '<br>
|<br>
| local function missing_uclo()<br>
| while true do -- luacheck: ignore<br>
| local f<br>
| if false then break end<br>
| while true do<br>
| if f then<br>
| return f<br>
| end<br>
| f = function()<br>
| return f<br>
| end<br>
| end<br>
| end<br>
| end<br>
| f = missing_uclo()<br>
| print(f())<br>
| f = missing_uclo()<br>
| print(f())<br>
| '<br>
| 3.1002202036551<br>
| luajit:
/home/burii/reviews/luajit/lj-819-missing-uclo/src/lj_record.c:135:
rec_check_slots: Assertion `((((((tr))>>24)
& IRT_TYPE) - (TRef)(IRT_NUM) <= (TRef)<br>
| (IRT_INT-IRT_NUM)))' failed.<br>
| Aborted<br>
<br>
I don't sure that we should test this particular
failure too, since the<br>
origin of the problem is the incorrect emitted
bytecode.<br>
<br>
Thoughts?</div>
</div>
</div>
</div>
</blockquote>
<div>We should not, because it is most likely caused by the
issue</div>
<div>that was fixed in the LuaJIT/LuaJIT@5c46f477.</div>
</div>
</blockquote>
</blockquote>
<p><br>
</p>
<p>assert in rec_check_slots could be for many reasons, so I added a
testcase for compiler too.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:1686137725.949044142@f485.i.mail.ru">
<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>
<div class="js-helper js-readmsg-msg">
<div>
<div><br>
> --<br>
> 2.34.1<br>
><br>
<br>
--<br>
Best regards,<br>
Sergey Kaplun</div>
</div>
</div>
</div>
</blockquote>
<div>
<div>--<br>
Best regards,</div>
<div>Maxim Kokryashkin</div>
</div>
<div> </div>
</div>
</blockquote>
</blockquote>
</body>
</html>