[Tarantool-patches] [PATCH v3] lua: assert in lua_gettop() in case of negative stack size

[Ilya Kosarev <i.kosarev@tarantool.org>]

According to user report (details provided in tarantool/tarantool#4649)
lua_gettop() returns negative value in encode_lua_call(). Such behavior
is not expected and violates Lua reference manual. Thus corresponding
sane check with assert is introduced.

Relates to tarantool/tarantool#4649
---
Branch: https://github.com/tarantool/luajit/tree/i.kosarev/gh-4649-assert-in-lua_gettop
Issue: https://github.com/tarantool/tarantool/issues/4649

Changes in v2:
- added reasoning in comment

Changes in v3:
- moved assert to LuaJIT itself

(gdb) bt
#0  0x00007f0012bbf387 in raise () from .../lib64/libc.so.6
#1  0x00007f0012bc0a78 in abort () from .../lib64/libc.so.6
#2  0x000000000041108e in sig_fatal_cb (signo=11, siginfo=0x7ef54107f8f0, context=0x7ef54107f7c0) at .../src/main.cc:258
#3  <signal handler called>
#4  0x00007f0012cf8691 in __strlen_sse2_pminub () from .../lib64/libc.so.6
#5  0x000000000061b18c in iproto_reply_error (out=0x7f00098692e8, e=0x0, sync=54211898, schema_version=495) at .../src/box/xrow.c:434
#6  0x00000000004168c2 in tx_reply_error (msg=0x7f0009816d80) at .../src/box/iproto.cc:1465
#7  0x0000000000416ec6 in tx_process_call (m=0x7f0009816d80) at .../src/box/iproto.cc:1649
#8  0x0000000000520e5a in cmsg_deliver (msg=0x7f0009816d80) at .../src/cbus.c:375
#9  0x0000000000521e34 in fiber_pool_f (ap=0x7ef5448c1728) at .../src/fiber_pool.c:64
#10 0x0000000000410a7a in fiber_cxx_invoke(fiber_func, typedef __va_list_tag __va_list_tag *) (f=0x521ca1 <fiber_pool_f>, ap=0x7ef5448c1728) at .../src/fiber.h:678
#11 0x000000000051af4e in fiber_loop (data=0x0) at .../src/fiber.c:713
#12 0x00000000006abae3 in coro_init () at .../third_party/coro/coro.c:110
(gdb) fr 7
#7  0x0000000000416ec6 in tx_process_call (m=0x7f0009816d80) at .../src/box/iproto.cc:1649
1649	in .../src/box/iproto.cc
(gdb) p rc
$29 = 0
(gdb) p count
$30 = -1
(gdb) p msg->header.type
$31 = 10
(gdb) p port->vtab->dump_msgpack
$32 = (int (*)(port *, obuf *)) 0x4e759f <port_lua_dump>

static void
tx_process_call(struct cmsg *m)
{
	struct iproto_msg *msg = tx_accept_msg(m);
	if (tx_check_schema(msg->header.schema_version))
		goto error;

	/*
	 * CALL/EVAL should copy its arguments so we can discard
	 * input on yield to avoid stalling other connections by
	 * a long polling request.
	 */
	struct trigger fiber_on_yield;
	trigger_create(&fiber_on_yield, tx_process_call_on_yield, msg, NULL);
	trigger_add(&fiber()->on_yield, &fiber_on_yield);

	int rc;
	struct port port;

	switch (msg->header.type) {
	case IPROTO_CALL:
	case IPROTO_CALL_16:
		rc = box_process_call(&msg->call, &port);
		break;
	case IPROTO_EVAL:
		rc = box_process_eval(&msg->call, &port);
		break;
	default:
		unreachable();
	}

	trigger_clear(&fiber_on_yield);

	if (rc != 0)
		goto error;

	/*
	 * Add all elements returned by the function to iproto.
	 *
	 * To allow clients to understand a complex return from
	 * a procedure, we are compatible with SELECT protocol,
	 * and return the number of return values first, and
	 * then each return value as a tuple.
	 *
	 * (!) Please note that a save point for output buffer
	 * must be taken only after finishing executing of Lua
	 * function because Lua can yield and leave the
	 * buffer in inconsistent state (a parallel request
	 * from the same connection will break the protocol).
	 */

	int count;
	struct obuf *out;
	struct obuf_svp svp;

	out = msg->connection->tx.p_obuf;
	if (iproto_prepare_select(out, &svp) != 0) {
		port_destroy(&port);
		goto error;
	}

	if (msg->header.type == IPROTO_CALL_16)
		count = port_dump_msgpack_16(&port, out);
	else
		count = port_dump_msgpack(&port, out);
	port_destroy(&port);
	if (count < 0) {
		obuf_rollback_to_svp(out, &svp);
		goto error;
	}

	iproto_reply_select(out, &svp, msg->header.sync,
			    ::schema_version, count);
	iproto_wpos_create(&msg->wpos, out);
	return;
error:
	tx_reply_error(msg);
}

As we can see, we fail to the error path through count == -1. It comes
from port_lua_dump() calling port_lua_do_dump() with encode_lua_call().

static int
port_lua_dump(struct port *base, struct obuf *out)
{
	return port_lua_do_dump(base, out, encode_lua_call);
}

encode_lua_call() actually sets port->size equal to lua_gettop(), being
returned by port_lua_do_dump().

 src/lj_api.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/lj_api.c b/src/lj_api.c
index c9b5d22..5ca2c57 100644
--- a/src/lj_api.c
+++ b/src/lj_api.c
@@ -135,7 +135,12 @@ LUA_API const lua_Number *lua_version(lua_State *L)
 
 LUA_API int lua_gettop(lua_State *L)
 {
-  return (int)(L->top - L->base);
+  int stack_size = (int)(L->top - L->base);
+  /*
+   * Stack size has to be non-negative according to Lua reference manual.
+   */
+  assert(stack_size >= 0);
+  return stack_size;
 }
 
 LUA_API void lua_settop(lua_State *L, int idx)
-- 
2.17.1