<HTML><BODY><div>Hi, Sergey!</div><div>Thanks for the fixes!</div><div>LGTM</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;"><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_16764444430841701864_BODY">Hi, Maxim!<br>Thanks for the review!<br><br>On 15.02.23, Maxim Kokryashkin wrote:<br>><br>> Hi, Sergey!<br>> LGTM, except for a few nits regarding the commit message.<br><br>Fixed your comments, the new commit message is the following:<br><br><br>| ARM64: Fix pcall() error case.<br>|<br>| Reported by Alex Orlenko.<br>|<br>| (cherry picked from commit b4b2dce9fc3ffaaaede39b36d06415311e2aa516)<br>|<br>| The `pcall()` assembler preambule modifies `RC`, which is mapped to<br>| `x28` (the value is N_args * 8), during the check of the amount of the<br>| given arguments. So, this wrong value being used in the `fff_fallback`<br>| routine leads to a crash on error throwing, because the Lua stack is<br>| incorrectly filled and can't be unwound.<br>|<br>| This patch adds the additional comparison before taking the fallback<br>| branch and modifies `RC` only after this branch.<br>|<br>| Sergey Kaplun:<br>| * added the description and the test for the problem<br>|<br>| Part of tarantool/tarantool#8069<br><br>Branch is force-pushed.<br><br>> <br>> > <br>> >>From: Mike Pall <mike><br>> >><br>> >>Reported by Alex Orlenko.<br>> >><br>> >>(cherry picked from commit b4b2dce9fc3ffaaaede39b36d06415311e2aa516)<br>> >><br>> >>The `pcall()` assembler preambule modifies `RC` (`x28`) (N args * 8)<br>> >The «`RC` (`x28`) (N args * 8)» expression is hard to percieve. I suggest<br>> >reformulating it in a way like «modifies `RC`, which is mapped to `x28`, so<br>> >it has value ...». Feel free to ignore.<br>> >>during the check of the amount of the given arguments. So, this wrong<br>> >>value using in the `fff_fallback` routine leading to a crash on the<br>> >Typo: s/using/being used/<br>> >Typo: s/leading/leads<br>> >Typo: s/on the/on<br>> >>error throwing, because the Lua stack is filled incorrect and can't be<br>> >Typo: s/is filled incorrect/is incorrectly filled/<br>> >>unwound.<br>> >><br>> >>This patch adds the additional comparison before taking the fallback<br>> >>branch and modifies `RC` only after this branch.<br>> >><br>> >>Sergey Kaplun:<br>> >>* added the description and the test for the problem<br>> >><br>> >>Part of tarantool/tarantool#8069<br>> >>---<br><br><snipped><br><br>> >--<br>> >Best regards,<br>> >Maxim Kokryashkin<br>> > <br><br>--<br>Best regards,<br>Sergey Kaplun</div></div></div></div></blockquote><div> </div></div></blockquote></BODY></HTML>