Re: [Tarantool-patches] [PATCH v2] lua: assert on lua_gettop() negative return value

[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?

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