Hi!
 
See my answer below.
Вторник, 21 июля 2020, 14:54 +03:00 от Nikita Pettik <korablev@tarantool.org>:
 
On 21 Jul 14:05, Ilya Kosarev wrote:
> In case lua_gettop() called from encode_lua_call() returns negative
> value, we will segfault in iproto_reply_error() with empty diag, as far
> as it is unexpected error path not covered with diagnostics. Thus
> corresponding sane check with assert is introduced.
>
> Closes #4649
> ---
> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4649-sane-check-on-lua_gettop
> Issue: https://github.com/tarantool/tarantool/issues/4649
>
> Changes in v2:
> - added reasoning in comment
>
> src/box/lua/call.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
> index ca871e077..516276a9f 100644
> --- a/src/box/lua/call.c
> +++ b/src/box/lua/call.c
> @@ -361,6 +361,12 @@ encode_lua_call(lua_State *L)
>
> struct luaL_serializer *cfg = luaL_msgpack_default;
> int size = lua_gettop(port->L);
> + /*
> + * lua_gettop() might return negative value in case the internal state
> + * of the given Lua coroutine is seriously broken. In case of such
> + * behavior execution has to be aborted immediately.
> + */
> + assert(size >= 0);

As I said, assuming lua_gettop() does return negative index under no
circumstances and your assumption is false, what other asserts or
checks can you suggest? I mean this problem appears extremely rare,
so we'd better provide extra checks just in case (so that we locate
problem with ease next time). Could you elaborate on this?
After last careful review finally using proper coredump
as far as i see it is the last option for empty diag segfault
mentioned in gh-4649. For some reasons it wasn’t previously
detected.

> for (int i = 1; i <= size; ++i)
> luamp_encode(port->L, cfg, &stream, i);
> port->size = size;
> --
> 2.17.1
>
 
 
--
Ilya Kosarev