[tarantool-patches] [PATCH v3 0/4] Dump lua frames for a fiber traceback

[tarantool-patches] [PATCH v3 0/4] Dump lua frames for a fiber traceback

[Georgy Kirichenko]

Lua frames are now dumped as well as plain C-frames for a
fiber backtrace. The main patch follows preparation fixes:
 - disable unwind handler inlining;
 - enable backtrace for an active coroutine;
 - reuse existing lua state for triggers;

Georgy Kirichenko (4):
  fiber: do not inline coro unwind function
  Proper unwind for currently executing fiber
  Use fiber lua state for triggers if possible
  Show names of Lua functions in backtraces

 src/backtrace.cc        | 24 ++++++++++++--
 src/box/lua/space.cc    | 12 +++----
 src/fiber.h             | 32 ++++++++----------
 src/lua/fiber.c         | 73 ++++++++++++++++++++++++++++++++++++++---
 src/lua/trigger.c       | 19 ++++++++---
 src/lua/trigger.h       |  2 +-
 test/app/fiber.result   | 36 ++++++++++++++++++++
 test/app/fiber.test.lua | 13 ++++++++
 8 files changed, 175 insertions(+), 36 deletions(-)

-- 
2.19.1
https://github.com/tarantool/tarantool/issues/3538
https://github.com/tarantool/tarantool/compare/g.kirichenko/gh-3538-lua-backtrace

Changes in v3:
 - Fix lua tracing for lua-triggers
 - Fixes according to review

Changes in v2:
 - Rebased against the latest 1.10
 - Segmentation fault fixed

[tarantool-patches] [PATCH v3 1/4] fiber: do not inline coro unwind function

[Georgy Kirichenko]

Do not inline coro_unwcontext because the unwind handler expects
for a separate stack frame.
---
 src/backtrace.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backtrace.cc b/src/backtrace.cc
index 2512bc045..d6be97621 100644
--- a/src/backtrace.cc
+++ b/src/backtrace.cc
@@ -210,7 +210,7 @@ unw_getcontext_f(unw_context_t *unw_context, void *stack)
  * @param @unw_context unwind context to store execution state.
  * @param @coro_ctx fiber context to unwind.
  */
-static void
+static void NOINLINE
 coro_unwcontext(unw_context_t *unw_context, struct coro_context *coro_ctx)
 {
 #if __amd64
-- 
2.19.1

[tarantool-patches] [PATCH v3 2/4] Proper unwind for currently executing fiber

[Georgy Kirichenko]

Each yielded fiber has a preserved coro state stored in a corresponding
variable however an executing fiber has a volatile state placed in CPU
registers (stack pointer, instruction pointer and non-volatile registers)
and corresponding context-storing variable value is invalid.
For already yielded fiber we have to use a special asm-written handler to make
a temporary switch to the preserved state and capture executing context what
is not needed for executing fiber.
After the patch for the executing fiber NULL is passed to the backtrace
function as coro context and then backtrace function could decide should
it use special context-switching handler or might just use unw_getcontext
from the unwind library.
---
 src/backtrace.cc | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/backtrace.cc b/src/backtrace.cc
index d6be97621..7aa175edb 100644
--- a/src/backtrace.cc
+++ b/src/backtrace.cc
@@ -363,12 +363,32 @@ __asm__ volatile(
 #endif
 }
 
+/*
+ * Call `cb' callback for each `coro_ctx' contained frame or the current
+ * executed coroutine if `coro_ctx' is NULL. A coro_context is a structure
+ * created on each coroutine yield to store execution context so for an on-CPU
+ * coroutine there is no valid coro_context could be defined and NULL is
+ * passed.
+*/
 void
 backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx)
 {
 	unw_cursor_t unw_cur;
 	unw_context_t unw_ctx;
-	coro_unwcontext(&unw_ctx, coro_ctx);
+	if (coro_ctx == NULL) {
+		/*
+		 * Current executing coroutine and simple unw_getcontext
+		 * should function.
+		 */
+		unw_getcontext(&unw_ctx);
+	} else {
+		/*
+		 * Execution context is stored in the coro_ctx
+		 * so use special context-switching handler to
+		 * capture an unwind context.
+		 */
+		coro_unwcontext(&unw_ctx, coro_ctx);
+	}
 	unw_init_local(&unw_cur, &unw_ctx);
 	int frame_no = 0;
 	unw_word_t sp = 0, old_sp = 0, ip, offset;
-- 
2.19.1

[tarantool-patches] [PATCH v3 3/4] Use fiber lua state for triggers if possible

[Georgy Kirichenko]

Lua trigger invocation reuses a fiber lua state if exists instead of
creating of new one for each new invocation. This is needed for a lua
stack reconstruction during backtracing.

Relates: #3538
---
 src/box/lua/space.cc | 12 ++++++------
 src/fiber.h          | 32 ++++++++++++++------------------
 src/lua/trigger.c    | 19 ++++++++++++++-----
 src/lua/trigger.h    |  2 +-
 4 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 25b7e36da..df698810b 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -73,20 +73,20 @@ lbox_push_txn_stmt(struct lua_State *L, void *event)
 }
 
 static int
-lbox_pop_txn_stmt(struct lua_State *L, void *event)
+lbox_pop_txn_stmt(struct lua_State *L, int nret, void *event)
 {
 	struct txn_stmt *stmt = txn_current_stmt((struct txn *) event);
 
-	if (lua_gettop(L) < 1) {
+	if (nret < 1) {
 		/* No return value - nothing to do. */
 		return 0;
 	}
-
-	struct tuple *result = luaT_istuple(L, 1);
-	if (result == NULL && !lua_isnil(L, 1) && !luaL_isnull(L, 1)) {
+	int top = lua_gettop(L) - nret + 1;
+	struct tuple *result = luaT_istuple(L, top);
+	if (result == NULL && !lua_isnil(L, top) && !luaL_isnull(L, top)) {
 		/* Invalid return value - raise error. */
 		diag_set(ClientError, ER_BEFORE_REPLACE_RET,
-			 lua_typename(L, lua_type(L, 1)));
+			 lua_typename(L, lua_type(L, top)));
 		return -1;
 	}
 
diff --git a/src/fiber.h b/src/fiber.h
index bb5343fdc..d4ed1193a 100644
--- a/src/fiber.h
+++ b/src/fiber.h
@@ -398,24 +398,20 @@ struct fiber {
 		struct session *session;
 		struct credentials *credentials;
 		struct txn *txn;
-		union {
-			/**
-			 * Fields used by a fiber created in Lua:
-			 * Lua stack and the optional
-			 * fiber.storage Lua reference.
-			 */
-			struct {
-				struct lua_State *stack;
-				int ref;
-			} lua;
-			/**
-			 * Fields used by a fiber created to
-			 * process an iproto request.
-			 */
-			struct {
-				uint64_t sync;
-			} net;
-		};
+		/**
+		 * Lua stack and the optional
+		 * fiber.storage Lua reference.
+		 */
+		struct {
+			struct lua_State *stack;
+			int ref;
+		} lua;
+		/**
+		 * Iproto sync.
+		 */
+		struct {
+			uint64_t sync;
+		} net;
 	} storage;
 	/** An object to wait for incoming message or a reader. */
 	struct ipc_wait_pad *wait_pad;
diff --git a/src/lua/trigger.c b/src/lua/trigger.c
index c758e47ea..2c2ede212 100644
--- a/src/lua/trigger.c
+++ b/src/lua/trigger.c
@@ -80,16 +80,25 @@ lbox_trigger_run(struct trigger *ptr, void *event)
 	 * invocation, and in future we plan to hack into Lua
 	 * C API to fix this.
 	 */
-	struct lua_State *L = lua_newthread(tarantool_L);
-	int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
+	struct lua_State *L;
+	int coro_ref;
+	if (fiber()->storage.lua.stack == NULL) {
+		L = lua_newthread(tarantool_L);
+		coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
+	} else {
+		L = fiber()->storage.lua.stack;
+		coro_ref = LUA_REFNIL;
+	}
+	int top = lua_gettop(L);
 	lua_rawgeti(L, LUA_REGISTRYINDEX, trigger->ref);
-	int top = trigger->push_event(L, event);
-	if (luaT_call(L, top, LUA_MULTRET)) {
+	int nargs = trigger->push_event(L, event);
+	if (luaT_call(L, nargs, LUA_MULTRET)) {
 		luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref);
 		diag_raise();
 	}
+	int nret = lua_gettop(L) - top;
 	if (trigger->pop_event != NULL &&
-	    trigger->pop_event(L, event) != 0) {
+	    trigger->pop_event(L, nret, event) != 0) {
 		luaL_unref(tarantool_L, LUA_REGISTRYINDEX, coro_ref);
 		diag_raise();
 	}
diff --git a/src/lua/trigger.h b/src/lua/trigger.h
index 8901f5dea..1d1dcce35 100644
--- a/src/lua/trigger.h
+++ b/src/lua/trigger.h
@@ -52,7 +52,7 @@ typedef int
  * error will be raised for the caller.
  */
 typedef int
-(*lbox_pop_event_f)(struct lua_State *L, void *event);
+(*lbox_pop_event_f)(struct lua_State *L, int nret, void *event);
 
 /**
  * Create a Lua trigger, replace an existing one,
-- 
2.19.1

[tarantool-patches] [PATCH v3 4/4] Show names of Lua functions in backtraces

[Georgy Kirichenko]

Trace corresponding Lua state as well as normal C stack frames while
fiber backtracing. This might be useful for debugging purposes.

Fixes: #3538
---
 src/lua/fiber.c         | 73 ++++++++++++++++++++++++++++++++++++++---
 test/app/fiber.result   | 36 ++++++++++++++++++++
 test/app/fiber.test.lua | 13 ++++++++
 3 files changed, 118 insertions(+), 4 deletions(-)

diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 147add89b..7ed341924 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -178,20 +178,80 @@ lbox_fiber_id(struct lua_State *L)
 	return 1;
 }
 
+/**
+ * Lua fiber traceback context.
+ */
+struct lua_fiber_tb_ctx {
+	/* Lua stack to push values. */
+	struct lua_State *L;
+	/* Lua stack to trace. */
+	struct lua_State *R;
+	/* Current Lua frame. */
+	int lua_frame;
+	/* Count of traced frames (both C and Lua). */
+	int tb_frame;
+};
+
 #ifdef ENABLE_BACKTRACE
+static void
+dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
+{
+	char buf[512];
+	snprintf(buf, sizeof(buf), "%s in %s at line %i",
+		 ar->name != NULL ? ar->name : "(unnamed)",
+		 ar->source, ar->currentline);
+	lua_pushnumber(L, tb_frame);
+	lua_newtable(L);
+	lua_pushstring(L, "L");
+	lua_pushstring(L, buf);
+	lua_settable(L, -3);
+	lua_settable(L, -3);
+}
+
 static int
 fiber_backtrace_cb(int frameno, void *frameret, const char *func, size_t offset, void *cb_ctx)
 {
+	struct lua_fiber_tb_ctx *tb_ctx = (struct lua_fiber_tb_ctx *)cb_ctx;
+	struct lua_State *L = tb_ctx->L;
+	if (strstr(func, "lj_BC_FUNCC") == func) {
+		/* We are in the LUA vm. */
+		lua_Debug ar;
+		while (tb_ctx->R && lua_getstack(tb_ctx->R, tb_ctx->lua_frame, &ar) > 0) {
+			/* Skip all following C-frames. */
+			lua_getinfo(tb_ctx->R, "Sln", &ar);
+			if (*ar.what != 'C')
+				break;
+			if (ar.name != NULL) {
+				/* Dump frame if it is a C built-in call. */
+				tb_ctx->tb_frame++;
+				dump_lua_frame(L, &ar, tb_ctx->tb_frame);
+			}
+			tb_ctx->lua_frame++;
+		}
+		while (tb_ctx->R && lua_getstack(tb_ctx->R, tb_ctx->lua_frame, &ar) > 0) {
+			/* Trace Lua frame. */
+			lua_getinfo(tb_ctx->R, "Sln", &ar);
+			if (*ar.what == 'C') {
+				break;
+			}
+			tb_ctx->tb_frame++;
+			dump_lua_frame(L, &ar, tb_ctx->tb_frame);
+			tb_ctx->lua_frame++;
+		}
+	}
 	char buf[512];
 	int l = snprintf(buf, sizeof(buf), "#%-2d %p in ", frameno, frameret);
 	if (func)
 		snprintf(buf + l, sizeof(buf) - l, "%s+%zu", func, offset);
 	else
 		snprintf(buf + l, sizeof(buf) - l, "?");
-	struct lua_State *L = (struct lua_State*)cb_ctx;
-	lua_pushnumber(L, frameno + 1);
+	tb_ctx->tb_frame++;
+	lua_pushnumber(L, tb_ctx->tb_frame);
+	lua_newtable(L);
+	lua_pushstring(L, "C");
 	lua_pushstring(L, buf);
 	lua_settable(L, -3);
+	lua_settable(L, -3);
 	return 0;
 }
 #endif
@@ -229,10 +289,15 @@ lbox_fiber_statof(struct fiber *f, void *cb_ctx, bool backtrace)
 
 	if (backtrace) {
 #ifdef ENABLE_BACKTRACE
+		struct lua_fiber_tb_ctx tb_ctx;
+		tb_ctx.L = L;
+		tb_ctx.R = f->storage.lua.stack;
+		tb_ctx.lua_frame = 0;
+		tb_ctx.tb_frame = 0;
 		lua_pushstring(L, "backtrace");
 		lua_newtable(L);
-		if (f != fiber())
-			backtrace_foreach(fiber_backtrace_cb, &f->ctx, L);
+		backtrace_foreach(fiber_backtrace_cb,
+				  f != fiber() ? &f->ctx : NULL, &tb_ctx);
 		lua_settable(L, -3);
 #endif /* ENABLE_BACKTRACE */
 	}
diff --git a/test/app/fiber.result b/test/app/fiber.result
index 77144e661..59aa8d5c6 100644
--- a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -849,6 +849,42 @@ f2:cancel()
 f3:cancel()
 ---
 ...
+function sf1() loop() end
+---
+...
+function sf2() sf1() end
+---
+...
+function sf3() sf2() end
+---
+...
+f1 = fiber.create(sf3)
+---
+...
+info = fiber.info()
+---
+...
+backtrace = info[f1:id()].backtrace
+---
+...
+bt_str = ''
+---
+...
+for _, b in pairs(backtrace) do bt_str = bt_str .. (b['L'] or '') end
+---
+...
+bt_str:find('sf1') ~= nil
+---
+- true
+...
+bt_str:find('loop') ~= nil
+---
+- true
+...
+bt_str:find('sf3') ~= nil
+---
+- true
+...
 -- # gh-666: nulls in output
 --
 getmetatable(fiber.info())
diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua
index 98a136090..b9d82ef05 100644
--- a/test/app/fiber.test.lua
+++ b/test/app/fiber.test.lua
@@ -334,6 +334,19 @@ f1:cancel()
 f2:cancel()
 f3:cancel()
 
+function sf1() loop() end
+function sf2() sf1() end
+function sf3() sf2() end
+f1 = fiber.create(sf3)
+
+info = fiber.info()
+backtrace = info[f1:id()].backtrace
+bt_str = ''
+for _, b in pairs(backtrace) do bt_str = bt_str .. (b['L'] or '') end
+bt_str:find('sf1') ~= nil
+bt_str:find('loop') ~= nil
+bt_str:find('sf3') ~= nil
+
 -- # gh-666: nulls in output
 --
 getmetatable(fiber.info())
-- 
2.19.1

Re: [tarantool-patches] [PATCH v3 0/4] Dump lua frames for a fiber traceback

[Vladimir Davydov]

On Wed, Oct 31, 2018 at 01:49:09PM +0300, Georgy Kirichenko wrote:
> Lua frames are now dumped as well as plain C-frames for a
> fiber backtrace. The main patch follows preparation fixes:
>  - disable unwind handler inlining;
>  - enable backtrace for an active coroutine;
>  - reuse existing lua state for triggers;
> 
> Georgy Kirichenko (4):
>   fiber: do not inline coro unwind function
>   Proper unwind for currently executing fiber
>   Use fiber lua state for triggers if possible
>   Show names of Lua functions in backtraces

Pushed to 1.10-features