

<HTML><BODY><div><div>Hi!</div><div> </div><div>See my answer below.</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Вторник, 21 июля 2020, 14:54 +03:00 от Nikita Pettik <korablev@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><style type="text/css"></style><div><div id="style_15953324421805520229_BODY">On 21 Jul 14:05, Ilya Kosarev wrote:<br>> In case lua_gettop() called from encode_lua_call() returns negative<br>> value, we will segfault in iproto_reply_error() with empty diag, as far<br>> as it is unexpected error path not covered with diagnostics. Thus<br>> corresponding sane check with assert is introduced.<br>><br>> Closes #4649<br>> ---<br>> Branch: <a href="https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4649-sane-check-on-lua_gettop" target="_blank">https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4649-sane-check-on-lua_gettop</a><br>> Issue: <a href="https://github.com/tarantool/tarantool/issues/4649" target="_blank">https://github.com/tarantool/tarantool/issues/4649</a><br>><br>> Changes in v2:<br>> - added reasoning in comment<br>><br>> src/box/lua/call.c | 6 ++++++<br>> 1 file changed, 6 insertions(+)<br>><br>> diff --git a/src/box/lua/call.c b/src/box/lua/call.c<br>> index ca871e077..516276a9f 100644<br>> --- a/src/box/lua/call.c<br>> +++ b/src/box/lua/call.c<br>> @@ -361,6 +361,12 @@ encode_lua_call(lua_State *L)<br>><br>> struct luaL_serializer *cfg = luaL_msgpack_default;<br>> int size = lua_gettop(port->L);<br>> + /*<br>> + * lua_gettop() might return negative value in case the internal state<br>> + * of the given Lua coroutine is seriously broken. In case of such<br>> + * behavior execution has to be aborted immediately.<br>> + */<br>> + assert(size >= 0);<br><br>As I said, assuming lua_gettop() does return negative index under no<br>circumstances and your assumption is false, what other asserts or<br>checks can you suggest? I mean this problem appears extremely rare,<br>so we'd better provide extra checks just in case (so that we locate<br>problem with ease next time). Could you elaborate on this?</div></div></div></div></blockquote></div><div>After last careful review finally using proper coredump</div><div>as far as i see it is the last option for empty diag segfault</div><div>mentioned in gh-4649. For some reasons it wasn’t previously</div><div>detected.</div><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>> for (int i = 1; i <= size; ++i)<br>> luamp_encode(port->L, cfg, &stream, i);<br>> port->size = size;<br>> --<br>> 2.17.1<br>></div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Ilya Kosarev</div></div></div><div> </div></div></BODY></HTML>