[Tarantool-patches] [PATCH v3] lua: assert in lua_gettop() in case of negative stack size
Ilya Kosarev
i.kosarev at tarantool.org
Wed Jul 22 01:32:42 MSK 2020
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
More information about the Tarantool-patches
mailing list