[Tarantool-patches] [PATCH v2 0/4] fiber: introduce creation backtrace

[Tarantool-patches] [PATCH v2 0/4] fiber: introduce creation backtrace

[Egor Elchinov via Tarantool-patches]

From: Egor Elchinov <elchinov.es@gmail.com>

This patchset adds the subtable `backtrace_parent()`
to `fiber.info()` holding backtraces of fiber
creation and introduces `fiber.parent_bt_enable()`
and `fiber.parent_bt_disable()` procedures managing
the runtime ability to collect parent backtraces for
newly created fibers.

Changes in v2:
 - first 3 patches merged together
 - unnecessary changes in `ENABLE_BACKTRACE` usages reverted

https://github.com/tarantool/tarantool/issues/4002
https://github.com/tarantool/tarantool/tree/Egor2001/gh-4002-fiber-creation-backtrace

Egor Elchinov (4):
  fiber: add PoC for fiber creation backtrace
  fiber: add option and PoC for Lua parent backtrace
  fiber: refactor lua backtrace routines
  fiber: refactor C backtrace and add changelog

 .../gh-4002-fiber-creation-backtrace.md       |   8 +
 src/lib/core/backtrace.cc                     | 247 +++++++++++++++---
 src/lib/core/backtrace.h                      |  10 +-
 src/lib/core/crash.c                          |   2 +-
 src/lib/core/fiber.c                          |  35 +++
 src/lib/core/fiber.h                          |  34 +++
 src/lua/fiber.c                               | 223 ++++++++++++++--
 src/lua/fiber.h                               |  19 ++
 .../gh-4002-fiber-creation-backtrace.result   |  78 ++++++
 .../gh-4002-fiber-creation-backtrace.test.lua |  34 +++
 10 files changed, 620 insertions(+), 70 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4002-fiber-creation-backtrace.md
 create mode 100644 test/app/gh-4002-fiber-creation-backtrace.result
 create mode 100644 test/app/gh-4002-fiber-creation-backtrace.test.lua

-- 
2.31.1

[Tarantool-patches] [PATCH v2 1/4] fiber: add PoC for fiber creation backtrace

[Egor Elchinov via Tarantool-patches]

From: Egor Elchinov <elchinov.es@gmail.com>

For now fiber creation backtrace is stored
in the separate subtable of fiber.info
called backtrace_parent for convenience.

Lua stacks of fiber creation
aren't preserved in backtrace yet because
of need to somehow handle parent Lua state
inside the child fiber for this sake.

Backtrace caching and demangling
aren't present yet too as this is a
proof-of-concept implementation.

Due to the limited libunwind API under osx for now
this patch uses backtrace() from <execinfo.h> and
dladdr() from <dlfcn.h> instead of absent
libunwind routines.

Custom backtrace() function was renamed to
backtrace_to_buf() because of the name conflict
with backtrace() routine from <execinfo.h>.

Needed for: #4002
---
 src/lib/core/backtrace.cc                     | 97 ++++++++++++++++++-
 src/lib/core/backtrace.h                      | 10 +-
 src/lib/core/crash.c                          |  2 +-
 src/lib/core/fiber.c                          |  8 ++
 src/lib/core/fiber.h                          | 16 +++
 src/lua/fiber.c                               | 10 ++
 .../gh-4002-fiber-creation-backtrace.result   | 57 +++++++++++
 .../gh-4002-fiber-creation-backtrace.test.lua | 27 ++++++
 8 files changed, 223 insertions(+), 4 deletions(-)
 create mode 100644 test/app/gh-4002-fiber-creation-backtrace.result
 create mode 100644 test/app/gh-4002-fiber-creation-backtrace.test.lua

diff --git a/src/lib/core/backtrace.cc b/src/lib/core/backtrace.cc
index b4048089f..f97c4c0a9 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -46,6 +46,11 @@
 #ifdef ENABLE_BACKTRACE
 #include <libunwind.h>
 
+#ifdef TARGET_OS_DARWIN
+#include <execinfo.h>
+#include <dlfcn.h>
+#endif
+
 #include "small/region.h"
 #include "small/static.h"
 /*
@@ -131,7 +136,7 @@ error:
 }
 
 char *
-backtrace(char *start, size_t size)
+backtrace_to_buf(char *start, size_t size)
 {
 	int frame_no = 0;
 	unw_word_t sp = 0, old_sp = 0, ip, offset;
@@ -432,11 +437,99 @@ out:
 	free(demangle_buf);
 }
 
+/**
+ * Collect up to `limit' IP register values
+ * for frames of the current stack into `ip_buf'.
+ * Must be by far faster than usual backtrace according to the
+ * libunwind doc for unw_backtrace().
+ */
+void NOINLINE
+backtrace_collect_ip(void **ip_buf, int limit)
+{
+	memset(ip_buf, 0, limit * sizeof(*ip_buf));
+#ifndef TARGET_OS_DARWIN
+	unw_backtrace(ip_buf, limit);
+#else
+	backtrace(ip_buf, limit);
+#endif
+}
+
+/**
+ * Call `cb' callback for not more than
+ * first `limit' frames present in the `ip_buf'.
+ *
+ * The implementation uses poorly documented `get_proc_name' callback
+ * from the `unw_accessors_t' to get procedure names via `ip_buf' values.
+ * Although `get_proc_name' is present on most architectures, it's an optional
+ * field, so procedure name is allowed to be absent (NULL) in `cb' call.
+ *
+ * TODO: to add cache and demangling support
+ */
+void
+backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
+		     void *cb_ctx)
+{
+#ifndef TARGET_OS_DARWIN
+	char proc_name[BACKTRACE_NAME_MAX];
+	unw_word_t ip = 0, offset = 0;
+	unw_proc_info_t pi;
+	int frame_no, ret = 0;
+	char *proc = NULL;
+
+	unw_accessors_t *acc = unw_get_accessors(unw_local_addr_space);
+
+	/*
+	 * RIPs collecting comes from inside a helper routine
+	 * so we skip the collector function address itself thus
+	 * start fetching functions with frame number = 1.
+	 */
+	for (frame_no = 1; frame_no < limit && ip_buf[frame_no] != NULL;
+	     frame_no++) {
+		ip = (unw_word_t)ip_buf[frame_no];
+
+		if (acc->get_proc_name == NULL) {
+			ret = unw_get_proc_info_by_ip(unw_local_addr_space,
+						      ip, &pi, NULL);
+			offset = ip - pi.start_ip;
+		} else {
+			ret = acc->get_proc_name(unw_local_addr_space, ip,
+						 proc_name, sizeof(proc_name),
+						 &offset, NULL);
+			proc = proc_name;
+		}
+		if (ret != 0 || cb(frame_no - 1, (void *)ip, proc,
+				   (size_t)offset, cb_ctx) != 0)
+			break;
+	}
+
+	if (ret != 0)
+		say_debug("unwinding error: %s", unw_strerror(ret));
+#else
+	int frame_no, ret = 1;
+	void *ip = NULL;
+	size_t offset = 0;
+	Dl_info dli;
+
+	for (frame_no = 1; frame_no < limit && ip_buf[frame_no] != NULL;
+	     ++frame_no) {
+		ip = ip_buf[frame_no];
+		ret = dladdr(ip, &dli);
+		offset = (char *)ip - (char *)dli.dli_saddr;
+
+		if (cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
+			break;
+	}
+
+	if (ret == 0)
+		say_debug("unwinding error: %i", ret);
+#endif
+}
+
 void
 print_backtrace(void)
 {
 	char *start = (char *)static_alloc(SMALL_STATIC_SIZE);
-	fdprintf(STDERR_FILENO, "%s", backtrace(start, SMALL_STATIC_SIZE));
+	fdprintf(STDERR_FILENO, "%s", backtrace_to_buf(start, SMALL_STATIC_SIZE));
 }
 #endif /* ENABLE_BACKTRACE */
 
diff --git a/src/lib/core/backtrace.h b/src/lib/core/backtrace.h
index e0ae56be4..4552c14e1 100644
--- a/src/lib/core/backtrace.h
+++ b/src/lib/core/backtrace.h
@@ -31,6 +31,7 @@
  * SUCH DAMAGE.
  */
 #include "trivia/config.h"
+#include "trivia/util.h"
 #include <stddef.h>
 
 #if defined(__cplusplus)
@@ -41,7 +42,7 @@ extern "C" {
 #include <coro.h>
 
 char *
-backtrace(char *start, size_t size);
+backtrace_to_buf(char *start, size_t size);
 
 void print_backtrace(void);
 
@@ -55,6 +56,13 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx);
 void
 backtrace_proc_cache_clear(void);
 
+void NOINLINE
+backtrace_collect_ip(void **ip_buf, int limit);
+
+void
+backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
+		     void *cb_ctx);
+
 #endif /* ENABLE_BACKTRACE */
 
 #if defined(__cplusplus)
diff --git a/src/lib/core/crash.c b/src/lib/core/crash.c
index abb7837e6..d1990d68b 100644
--- a/src/lib/core/crash.c
+++ b/src/lib/core/crash.c
@@ -213,7 +213,7 @@ crash_collect(int signo, siginfo_t *siginfo, void *ucontext)
 
 #ifdef ENABLE_BACKTRACE
 	char *start = cinfo->backtrace_buf;
-	backtrace(start, sizeof(cinfo->backtrace_buf));
+	backtrace_to_buf(start, sizeof(cinfo->backtrace_buf));
 #endif
 
 #ifdef HAS_GREG
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 759c7da6a..924ff3c82 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -45,6 +45,11 @@
 
 extern void cord_on_yield(void);
 
+#if ENABLE_BACKTRACE
+#include "backtrace.h" /* fast_trace */
+
+#endif /* ENABLE_BACKTRACE */
+
 #if ENABLE_FIBER_TOP
 #include <x86intrin.h> /* __rdtscp() */
 
@@ -1259,6 +1264,9 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr,
 	fiber->f = f;
 	fiber->fid = cord->next_fid;
 	fiber_set_name(fiber, name);
+#if ENABLE_BACKTRACE
+	backtrace_collect_ip(fiber->parent_bt_ip_buf, FIBER_PARENT_BT_MAX);
+#endif /* ENABLE_BACKTRACE */
 	register_fid(fiber);
 	fiber->csw = 0;
 
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index 8f4e14796..beed58866 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -111,6 +111,18 @@ struct cpu_stat {
 
 #endif /* ENABLE_FIBER_TOP */
 
+#if ENABLE_BACKTRACE
+
+enum {
+	/**
+	 * Maximum entries count to grab
+	 * from the fiber creation backtrace.
+	 */
+	FIBER_PARENT_BT_MAX = 8
+};
+
+#endif /* ENABLE_BACKTRACE */
+
 enum {
 	/** Both limits include terminating 0. */
 	FIBER_NAME_INLINE = 40,
@@ -640,6 +652,10 @@ struct fiber {
 	 */
 	char *name;
 	char inline_name[FIBER_NAME_INLINE];
+#if ENABLE_BACKTRACE
+	/** Fiber creation backtrace chunk. */
+	void *parent_bt_ip_buf[FIBER_PARENT_BT_MAX];
+#endif /* ENABLE_BACKTRACE */
 };
 
 /** Invoke on_stop triggers and delete them. */
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 91898c283..7b21361d4 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -308,6 +308,16 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 		backtrace_foreach(fiber_backtrace_cb,
 				  f != fiber() ? &f->ctx : NULL, &tb_ctx);
 		lua_settable(L, -3);
+
+		tb_ctx.lua_frame = 0;
+		tb_ctx.tb_frame = 0;
+		tb_ctx.R = NULL;
+		lua_pushstring(L, "backtrace_parent");
+		lua_newtable(L);
+		backtrace_foreach_ip(fiber_backtrace_cb,
+				     f->parent_bt_ip_buf,
+				     FIBER_PARENT_BT_MAX, &tb_ctx);
+		lua_settable(L, -3);
 #endif /* ENABLE_BACKTRACE */
 	}
 	return 0;
diff --git a/test/app/gh-4002-fiber-creation-backtrace.result b/test/app/gh-4002-fiber-creation-backtrace.result
new file mode 100644
index 000000000..4934b82d6
--- /dev/null
+++ b/test/app/gh-4002-fiber-creation-backtrace.result
@@ -0,0 +1,57 @@
+-- test-run result file version 2
+yaml = require('yaml')
+ | ---
+ | ...
+fiber = require('fiber')
+ | ---
+ | ...
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+local stack_len = 0
+ | ---
+ | ...
+local parent_stack_len = 0
+ | ---
+ | ...
+
+test_run:cmd('setopt delimiter ";"')
+ | ---
+ | - true
+ | ...
+foo = function()
+    local id = fiber.self():id()
+    local info = fiber.info()[id]
+    local stack = info.backtrace
+    stack_len = stack and #stack or -1
+    local parent_stack = info.backtrace_parent
+    parent_stack_len = parent_stack and #parent_stack or -1
+end;
+ | ---
+ | ...
+
+test_run:cmd('setopt delimiter ""');
+ | ---
+ | - true
+ | ...
+
+local bar,baz
+ | ---
+ | ...
+
+bar = function(n) if n ~= 0 then baz(n-1) else fiber.create(foo) end end
+ | ---
+ | ...
+baz = function(n) bar(n) end
+ | ---
+ | ...
+
+baz(10)
+ | ---
+ | ...
+assert(parent_stack_len > 0 or stack_len == -1)
+ | ---
+ | - true
+ | ...
+
diff --git a/test/app/gh-4002-fiber-creation-backtrace.test.lua b/test/app/gh-4002-fiber-creation-backtrace.test.lua
new file mode 100644
index 000000000..24d41a860
--- /dev/null
+++ b/test/app/gh-4002-fiber-creation-backtrace.test.lua
@@ -0,0 +1,27 @@
+yaml = require('yaml')
+fiber = require('fiber')
+test_run = require('test_run').new()
+
+local stack_len = 0
+local parent_stack_len = 0
+
+test_run:cmd('setopt delimiter ";"')
+foo = function()
+    local id = fiber.self():id()
+    local info = fiber.info()[id]
+    local stack = info.backtrace
+    stack_len = stack and #stack or -1
+    local parent_stack = info.backtrace_parent
+    parent_stack_len = parent_stack and #parent_stack or -1
+end;
+
+test_run:cmd('setopt delimiter ""');
+
+local bar,baz
+
+bar = function(n) if n ~= 0 then baz(n-1) else fiber.create(foo) end end
+baz = function(n) bar(n) end
+
+baz(10)
+assert(parent_stack_len > 0 or stack_len == -1)
+
-- 
2.31.1

[Tarantool-patches] [PATCH v2 2/4] fiber: add option and PoC for Lua parent backtrace

[Egor Elchinov via Tarantool-patches]

From: Egor Elchinov <elchinov.es@gmail.com>

For now Lua backtrace is fully collected
at fiber creation moment and stored in a
dynamically allocated buffer.

This patch also adds option enabling to turn on and off
the collection of backtraces of parent fibers when
creating new fibers.

Needed for: #4002

@TarantoolBot document
Title: new option for fiber parent backtrace

`fiber.parent_bt_enable()` and `fiber.parent_bt_disable()`
turn on and off the ability to trace fiber creation stacks.
If present this option may cause up to 40 per cent
slowdown of fiber creation.
By default parent backtrace is disabled.

Note that fibers created while this option was disabled
lack their parent backtraces in `fiber.info()` forever.
---
 src/lib/core/fiber.c                          |  29 ++-
 src/lib/core/fiber.h                          |  18 ++
 src/lua/fiber.c                               | 200 ++++++++++++++++--
 src/lua/fiber.h                               |  19 ++
 .../gh-4002-fiber-creation-backtrace.result   |  21 ++
 .../gh-4002-fiber-creation-backtrace.test.lua |   7 +
 6 files changed, 280 insertions(+), 14 deletions(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 924ff3c82..dd26e2f13 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -220,6 +220,10 @@ fiber_mprotect(void *addr, size_t len, int prot)
 static __thread bool fiber_top_enabled = false;
 #endif /* ENABLE_FIBER_TOP */
 
+#if ENABLE_BACKTRACE
+static __thread bool fiber_parent_bt_enabled = false;
+#endif /* ENABLE_BACKTRACE */
+
 /**
  * An action performed each time a context switch happens.
  * Used to count each fiber's processing time.
@@ -1265,7 +1269,10 @@ fiber_new_ex(const char *name, const struct fiber_attr *fiber_attr,
 	fiber->fid = cord->next_fid;
 	fiber_set_name(fiber, name);
 #if ENABLE_BACKTRACE
-	backtrace_collect_ip(fiber->parent_bt_ip_buf, FIBER_PARENT_BT_MAX);
+	if (fiber_parent_bt_enabled) {
+		backtrace_collect_ip(fiber->parent_bt_ip_buf,
+				     FIBER_PARENT_BT_MAX);
+	}
 #endif /* ENABLE_BACKTRACE */
 	register_fid(fiber);
 	fiber->csw = 0;
@@ -1414,6 +1421,26 @@ fiber_top_disable(void)
 }
 #endif /* ENABLE_FIBER_TOP */
 
+#if ENABLE_BACKTRACE
+bool
+fiber_parent_bt_is_enabled(void)
+{
+	return fiber_parent_bt_enabled;
+}
+
+void
+fiber_parent_bt_enable(void)
+{
+	fiber_parent_bt_enabled = true;
+}
+
+void
+fiber_parent_bt_disable(void)
+{
+	fiber_parent_bt_enabled = false;
+}
+#endif /* ENABLE_BACKTRACE */
+
 size_t
 box_region_used(void)
 {
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index beed58866..477b68e4c 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -528,6 +528,7 @@ struct txn;
 struct credentials;
 struct lua_State;
 struct ipc_wait_pad;
+struct parent_bt_lua;
 
 struct fiber {
 	coro_context ctx;
@@ -634,6 +635,12 @@ struct fiber {
 			 * Optional fiber.storage Lua reference.
 			 */
 			int ref;
+#if ENABLE_BACKTRACE
+			/**
+			 * Lua parent backtrace (may be NULL)
+			 */
+			struct parent_bt_lua *parent_bt;
+#endif /* ENABLE_BACKTRACE */
 		} lua;
 		/**
 		 * Iproto sync.
@@ -871,6 +878,17 @@ void
 fiber_top_disable(void);
 #endif /* ENABLE_FIBER_TOP */
 
+#if ENABLE_BACKTRACE
+bool
+fiber_parent_bt_is_enabled(void);
+
+void
+fiber_parent_bt_enable(void);
+
+void
+fiber_parent_bt_disable(void);
+#endif /* ENABLE_BACKTRACE */
+
 /** Useful for C unit tests */
 static inline int
 fiber_c_invoke(fiber_func f, va_list ap)
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 7b21361d4..8e96059b4 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -193,6 +193,18 @@ struct lua_fiber_tb_ctx {
 	int tb_frame;
 };
 
+/**
+ * Lua parent traceback context.
+ */
+struct lua_parent_tb_ctx {
+	/* Lua stack to push values. */
+	struct lua_State *L;
+	/* Lua parent backtrace. */
+	struct parent_bt_lua *bt;
+	/* 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)
@@ -209,6 +221,22 @@ dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
 	lua_settable(L, -3);
 }
 
+static void
+dump_parent_lua_frame(struct lua_State *L, const char *name, const char *src,
+		      int currentline, int tb_frame)
+{
+	char buf[512];
+	snprintf(buf, sizeof(buf), "%s in %s at line %i",
+		 name != NULL ? name : "(unnamed)",
+		 src, 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)
 {
@@ -260,7 +288,115 @@ fiber_backtrace_cb(int frameno, void *frameret, const char *func, size_t offset,
 	lua_settable(L, -3);
 	return 0;
 }
-#endif
+
+static int
+fiber_parent_backtrace_cb(int frameno, void *frameret, const char *func,
+			  size_t offset, void *cb_ctx)
+{
+	int lua_frame = 0;
+	struct lua_parent_tb_ctx *tb_ctx = (struct lua_parent_tb_ctx *)cb_ctx;
+	struct parent_bt_lua *bt = tb_ctx->bt;
+	struct lua_State *L = tb_ctx->L;
+	/*
+	 * There is impossible to get func == NULL until
+	 * https://github.com/tarantool/tarantool/issues/5326
+	 * will not resolved, but is possible afterwards.
+	 */
+	if (bt != NULL && func != NULL && strstr(func, "lj_BC_FUNCC") == func) {
+		/* We are in the LUA vm. */
+		while (lua_frame < bt->cnt) {
+			tb_ctx->tb_frame++;
+			dump_parent_lua_frame(L, bt->names[lua_frame],
+					      bt->sources[lua_frame],
+					      bt->lines[lua_frame],
+					      tb_ctx->tb_frame);
+			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, "?");
+	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;
+}
+
+static int
+fiber_parent_bt_init(struct fiber *f, struct lua_State *L)
+{
+	int lua_frame = 0, tb_frame = 0;
+	lua_Debug ar;
+	struct parent_bt_lua *bt = NULL;
+
+	bt = (struct parent_bt_lua *)malloc(sizeof(*bt));
+	if (bt == NULL){
+		diag_set(OutOfMemory, sizeof(*bt), "malloc", "bt");
+		return 1;
+	}
+
+	while (tb_frame < PARENT_BT_LUA_LEN_MAX &&
+	       lua_getstack(L, lua_frame, &ar) > 0) {
+		/* Skip all following C-frames. */
+		lua_getinfo(L, "Sln", &ar);
+		if (*ar.what != 'C')
+			break;
+		if (ar.name != NULL) {
+			/* Dump frame if it is a C built-in call. */
+			bt->lines[tb_frame] = ar.currentline;
+			memset(bt->names[tb_frame], 0,
+			       PARENT_BT_LUA_NAME_MAX);
+			strncpy(bt->names[tb_frame], ar.name,
+				PARENT_BT_LUA_NAME_MAX - 1);
+			memset(bt->sources[tb_frame], 0,
+			       PARENT_BT_LUA_NAME_MAX);
+			strncpy(bt->sources[tb_frame], ar.source,
+				PARENT_BT_LUA_NAME_MAX - 1);
+			tb_frame++;
+		}
+		lua_frame++;
+	}
+	while (tb_frame < PARENT_BT_LUA_LEN_MAX &&
+	       lua_getstack(L, lua_frame, &ar) > 0) {
+		/* Trace Lua frame. */
+		lua_getinfo(L, "Sln", &ar);
+		if (*ar.what == 'C')
+			break;
+		bt->lines[tb_frame] = ar.currentline;
+		memset(bt->names[tb_frame], 0, PARENT_BT_LUA_NAME_MAX);
+		if (ar.name != NULL) {
+			strncpy(bt->names[tb_frame], ar.name,
+				PARENT_BT_LUA_NAME_MAX - 1);
+		} else {
+			strncpy(bt->names[tb_frame], "(unnamed)",
+				PARENT_BT_LUA_NAME_MAX - 1);
+		}
+		memset(bt->sources[tb_frame], 0, PARENT_BT_LUA_NAME_MAX);
+		strncpy(bt->sources[tb_frame], ar.source,
+			PARENT_BT_LUA_NAME_MAX - 1);
+		tb_frame++;
+		lua_frame++;
+	}
+
+	bt->cnt = tb_frame;
+	f->storage.lua.parent_bt = bt;
+
+	return 0;
+}
+
+static void
+fiber_parent_bt_free(struct fiber *f)
+{
+	free(f->storage.lua.parent_bt);
+}
+#endif /* ENABLE_BACKTRACE */
 
 static int
 lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
@@ -299,6 +435,7 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 	if (backtrace) {
 #ifdef ENABLE_BACKTRACE
 		struct lua_fiber_tb_ctx tb_ctx;
+		struct lua_parent_tb_ctx parent_tb_ctx;
 		tb_ctx.L = L;
 		tb_ctx.R = f->storage.lua.stack;
 		tb_ctx.lua_frame = 0;
@@ -309,15 +446,18 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 				  f != fiber() ? &f->ctx : NULL, &tb_ctx);
 		lua_settable(L, -3);
 
-		tb_ctx.lua_frame = 0;
-		tb_ctx.tb_frame = 0;
-		tb_ctx.R = NULL;
-		lua_pushstring(L, "backtrace_parent");
-		lua_newtable(L);
-		backtrace_foreach_ip(fiber_backtrace_cb,
-				     f->parent_bt_ip_buf,
-				     FIBER_PARENT_BT_MAX, &tb_ctx);
-		lua_settable(L, -3);
+		if (fiber_parent_bt_is_enabled()) {
+			parent_tb_ctx.L = L;
+			parent_tb_ctx.bt = f->storage.lua.parent_bt;
+			parent_tb_ctx.tb_frame = 0;
+			lua_pushstring(L, "backtrace_parent");
+			lua_newtable(L);
+			backtrace_foreach_ip(fiber_parent_backtrace_cb,
+					     f->parent_bt_ip_buf,
+					     FIBER_PARENT_BT_MAX,
+					     &parent_tb_ctx);
+			lua_settable(L, -3);
+		}
 #endif /* ENABLE_BACKTRACE */
 	}
 	return 0;
@@ -441,6 +581,22 @@ lbox_do_backtrace(struct lua_State *L)
 	}
 	return true;
 }
+
+static int
+lbox_fiber_parent_bt_enable(struct lua_State *L)
+{
+	(void) L;
+	fiber_parent_bt_enable();
+	return 0;
+}
+
+static int
+lbox_fiber_parent_bt_disable(struct lua_State *L)
+{
+	(void) L;
+	fiber_parent_bt_disable();
+	return 0;
+}
 #endif /* ENABLE_BACKTRACE */
 
 /**
@@ -481,10 +637,14 @@ lua_fiber_run_f(MAYBE_UNUSED va_list ap)
 	 * We can unref child stack here,
 	 * otherwise we have to unref child stack in join
 	 */
-	if (f->flags & FIBER_IS_JOINABLE)
+	if (f->flags & FIBER_IS_JOINABLE) {
 		lua_pushinteger(L, coro_ref);
-	else
+	} else {
+#ifdef ENABLE_BACKTRACE
+		fiber_parent_bt_free(f);
+#endif
 		luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
+	}
 
 	return result;
 }
@@ -506,6 +666,12 @@ fiber_create(struct lua_State *L)
 		luaT_error(L);
 	}
 
+#ifdef ENABLE_BACKTRACE
+	// TODO: error handling
+	if (fiber_parent_bt_is_enabled())
+		fiber_parent_bt_init(f, L);
+#endif
+
 	/* Move the arguments to the new coro */
 	lua_xmove(L, child_L, lua_gettop(L));
 	/* XXX: 'fiber' is leaked if this throws a Lua error. */
@@ -886,8 +1052,12 @@ lbox_fiber_join(struct lua_State *L)
 			lua_xmove(child_L, L, num_ret);
 		}
 	}
-	if (child_L != NULL)
+	if (child_L != NULL) {
+#ifdef ENABLE_BACKTRACE
+		fiber_parent_bt_free(fiber);
+#endif
 		luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
+	}
 	return num_ret + 1;
 }
 
@@ -941,6 +1111,10 @@ static const struct luaL_Reg fiberlib[] = {
 	{"top_enable", lbox_fiber_top_enable},
 	{"top_disable", lbox_fiber_top_disable},
 #endif /* ENABLE_FIBER_TOP */
+#if ENABLE_BACKTRACE
+	{"parent_bt_enable", lbox_fiber_parent_bt_enable},
+	{"parent_bt_disable", lbox_fiber_parent_bt_disable},
+#endif /* ENABLE_BACKTRACE */
 	{"sleep", lbox_fiber_sleep},
 	{"yield", lbox_fiber_yield},
 	{"self", lbox_fiber_self},
diff --git a/src/lua/fiber.h b/src/lua/fiber.h
index e29898706..450840fb0 100644
--- a/src/lua/fiber.h
+++ b/src/lua/fiber.h
@@ -36,6 +36,25 @@ extern "C" {
 
 struct lua_State;
 
+/**
+ * Maximal name length (including '\0')
+ * and backtrace length.
+ */
+enum {
+	PARENT_BT_LUA_NAME_MAX = 64,
+	PARENT_BT_LUA_LEN_MAX = 8
+};
+
+/**
+ * Stores lua parent backtrace for fiber.
+ */
+struct parent_bt_lua {
+	int cnt;
+	char names[PARENT_BT_LUA_LEN_MAX][PARENT_BT_LUA_NAME_MAX];
+	char sources[PARENT_BT_LUA_LEN_MAX][PARENT_BT_LUA_NAME_MAX];
+	int lines[PARENT_BT_LUA_LEN_MAX];
+};
+
 /**
 * Initialize box.fiber system
 */
diff --git a/test/app/gh-4002-fiber-creation-backtrace.result b/test/app/gh-4002-fiber-creation-backtrace.result
index 4934b82d6..6a075911d 100644
--- a/test/app/gh-4002-fiber-creation-backtrace.result
+++ b/test/app/gh-4002-fiber-creation-backtrace.result
@@ -47,6 +47,17 @@ baz = function(n) bar(n) end
  | ---
  | ...
 
+baz(10)
+ | ---
+ | ...
+assert(parent_stack_len == -1)
+ | ---
+ | - true
+ | ...
+
+if stack_len ~= -1 then fiber:parent_bt_enable() end
+ | ---
+ | ...
 baz(10)
  | ---
  | ...
@@ -55,3 +66,13 @@ assert(parent_stack_len > 0 or stack_len == -1)
  | - true
  | ...
 
+if stack_len ~= -1 then fiber:parent_bt_disable() end
+ | ---
+ | ...
+baz(10)
+ | ---
+ | ...
+assert(parent_stack_len == -1)
+ | ---
+ | - true
+ | ...
diff --git a/test/app/gh-4002-fiber-creation-backtrace.test.lua b/test/app/gh-4002-fiber-creation-backtrace.test.lua
index 24d41a860..79a516860 100644
--- a/test/app/gh-4002-fiber-creation-backtrace.test.lua
+++ b/test/app/gh-4002-fiber-creation-backtrace.test.lua
@@ -22,6 +22,13 @@ local bar,baz
 bar = function(n) if n ~= 0 then baz(n-1) else fiber.create(foo) end end
 baz = function(n) bar(n) end
 
+baz(10)
+assert(parent_stack_len == -1)
+
+if stack_len ~= -1 then fiber:parent_bt_enable() end
 baz(10)
 assert(parent_stack_len > 0 or stack_len == -1)
 
+if stack_len ~= -1 then fiber:parent_bt_disable() end
+baz(10)
+assert(parent_stack_len == -1)
-- 
2.31.1

[Tarantool-patches] [PATCH v2 3/4] fiber: refactor lua backtrace routines

[Egor Elchinov via Tarantool-patches]

From: Egor Elchinov <elchinov.es@gmail.com>

Lua backtrace with callback moved to new routine
`lua_backtrace_cb()` similar to the `backtrace_cb()`
for C backtrace.

Needed for: #4002
---
 src/lua/fiber.c | 167 +++++++++++++++++++++---------------------------
 1 file changed, 74 insertions(+), 93 deletions(-)

diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 8e96059b4..d0cdb0ae4 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -205,14 +205,17 @@ struct lua_parent_tb_ctx {
 	int tb_frame;
 };
 
+/** Lua backtrace callback type */
+typedef int (lua_backtrace_cb)(struct lua_Debug *ar, int frame_no, void *cb_ctx);
+
 #ifdef ENABLE_BACKTRACE
 static void
-dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
+dump_lua_frame(struct lua_State *L, const char *name, const char *src, int line,
+	       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);
+		 name != NULL ? name : "(unnamed)", src, line);
 	lua_pushnumber(L, tb_frame);
 	lua_newtable(L);
 	lua_pushstring(L, "L");
@@ -221,20 +224,65 @@ dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
 	lua_settable(L, -3);
 }
 
+static int
+dump_lua_frame_cb(lua_Debug *ar, int tb_frame, void *cb_ctx)
+{
+	(void) tb_frame;
+	struct lua_fiber_tb_ctx *tb_ctx = (struct lua_fiber_tb_ctx *)cb_ctx;
+
+	tb_ctx->tb_frame++;
+	dump_lua_frame(tb_ctx->L, ar->name, ar->source, ar->currentline,
+		       tb_ctx->tb_frame);
+
+	return 0;
+}
+
+static int
+save_lua_frame_cb(lua_Debug *ar, int tb_frame, void *cb_ctx)
+{
+	struct parent_bt_lua *bt = (struct parent_bt_lua *)cb_ctx;
+
+	if (tb_frame >= PARENT_BT_LUA_LEN_MAX)
+		return 1;
+
+	bt->cnt++;
+	bt->lines[tb_frame] = ar->currentline;
+	strncpy(bt->names[tb_frame], ar->name == NULL ? "(unnamed)" : ar->name,
+		PARENT_BT_LUA_NAME_MAX - 1);
+	strncpy(bt->sources[tb_frame], ar->source,
+		PARENT_BT_LUA_NAME_MAX - 1);
+
+	return 0;
+}
+
 static void
-dump_parent_lua_frame(struct lua_State *L, const char *name, const char *src,
-		      int currentline, int tb_frame)
+lua_backtrace_foreach(struct lua_State *L, lua_backtrace_cb cb, void *cb_ctx)
 {
-	char buf[512];
-	snprintf(buf, sizeof(buf), "%s in %s at line %i",
-		 name != NULL ? name : "(unnamed)",
-		 src, 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);
+	int lua_frame = 0, tb_frame = 0;
+	lua_Debug ar;
+	while (lua_getstack(L, lua_frame, &ar) > 0) {
+		/* Skip all following C-frames. */
+		lua_getinfo(L, "Sln", &ar);
+		if (*ar.what != 'C')
+			break;
+		if (ar.name != NULL) {
+			/* Dump frame if it is a C built-in call. */
+			if (cb(&ar, tb_frame, cb_ctx) != 0)
+				return;
+			tb_frame++;
+		}
+		lua_frame++;
+	}
+	while (lua_getstack(L, lua_frame, &ar) > 0) {
+		/* Trace Lua frame. */
+		lua_getinfo(L, "Sln", &ar);
+		if (*ar.what == 'C')
+			break;
+		if (cb(&ar, tb_frame, cb_ctx) != 0)
+			return;
+		tb_frame++;
+		lua_frame++;
+	}
 }
 
 static int
@@ -247,31 +295,9 @@ fiber_backtrace_cb(int frameno, void *frameret, const char *func, size_t offset,
 	 * https://github.com/tarantool/tarantool/issues/5326
 	 * will not resolved, but is possible afterwards.
 	 */
-	if (func != NULL && strstr(func, "lj_BC_FUNCC") == func) {
+	if (func != NULL && tb_ctx->R && 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++;
-		}
+		lua_backtrace_foreach(tb_ctx->R, dump_lua_frame_cb, tb_ctx);
 	}
 	char buf[512];
 	int l = snprintf(buf, sizeof(buf), "#%-2d %p in ", frameno, frameret);
@@ -306,10 +332,10 @@ fiber_parent_backtrace_cb(int frameno, void *frameret, const char *func,
 		/* We are in the LUA vm. */
 		while (lua_frame < bt->cnt) {
 			tb_ctx->tb_frame++;
-			dump_parent_lua_frame(L, bt->names[lua_frame],
-					      bt->sources[lua_frame],
-					      bt->lines[lua_frame],
-					      tb_ctx->tb_frame);
+			dump_lua_frame(L, bt->names[lua_frame],
+				       bt->sources[lua_frame],
+				       bt->lines[lua_frame],
+				       tb_ctx->tb_frame);
 			lua_frame++;
 		}
 	}
@@ -332,60 +358,15 @@ fiber_parent_backtrace_cb(int frameno, void *frameret, const char *func,
 static int
 fiber_parent_bt_init(struct fiber *f, struct lua_State *L)
 {
-	int lua_frame = 0, tb_frame = 0;
-	lua_Debug ar;
 	struct parent_bt_lua *bt = NULL;
-
-	bt = (struct parent_bt_lua *)malloc(sizeof(*bt));
+	bt = (struct parent_bt_lua *)calloc(1, sizeof(*bt));
 	if (bt == NULL){
-		diag_set(OutOfMemory, sizeof(*bt), "malloc", "bt");
+		diag_set(OutOfMemory, sizeof(*bt), "calloc", "bt");
 		return 1;
 	}
 
-	while (tb_frame < PARENT_BT_LUA_LEN_MAX &&
-	       lua_getstack(L, lua_frame, &ar) > 0) {
-		/* Skip all following C-frames. */
-		lua_getinfo(L, "Sln", &ar);
-		if (*ar.what != 'C')
-			break;
-		if (ar.name != NULL) {
-			/* Dump frame if it is a C built-in call. */
-			bt->lines[tb_frame] = ar.currentline;
-			memset(bt->names[tb_frame], 0,
-			       PARENT_BT_LUA_NAME_MAX);
-			strncpy(bt->names[tb_frame], ar.name,
-				PARENT_BT_LUA_NAME_MAX - 1);
-			memset(bt->sources[tb_frame], 0,
-			       PARENT_BT_LUA_NAME_MAX);
-			strncpy(bt->sources[tb_frame], ar.source,
-				PARENT_BT_LUA_NAME_MAX - 1);
-			tb_frame++;
-		}
-		lua_frame++;
-	}
-	while (tb_frame < PARENT_BT_LUA_LEN_MAX &&
-	       lua_getstack(L, lua_frame, &ar) > 0) {
-		/* Trace Lua frame. */
-		lua_getinfo(L, "Sln", &ar);
-		if (*ar.what == 'C')
-			break;
-		bt->lines[tb_frame] = ar.currentline;
-		memset(bt->names[tb_frame], 0, PARENT_BT_LUA_NAME_MAX);
-		if (ar.name != NULL) {
-			strncpy(bt->names[tb_frame], ar.name,
-				PARENT_BT_LUA_NAME_MAX - 1);
-		} else {
-			strncpy(bt->names[tb_frame], "(unnamed)",
-				PARENT_BT_LUA_NAME_MAX - 1);
-		}
-		memset(bt->sources[tb_frame], 0, PARENT_BT_LUA_NAME_MAX);
-		strncpy(bt->sources[tb_frame], ar.source,
-			PARENT_BT_LUA_NAME_MAX - 1);
-		tb_frame++;
-		lua_frame++;
-	}
-
-	bt->cnt = tb_frame;
+	bt->cnt = 0;
+	lua_backtrace_foreach(L, save_lua_frame_cb, bt);
 	f->storage.lua.parent_bt = bt;
 
 	return 0;
@@ -1111,7 +1092,7 @@ static const struct luaL_Reg fiberlib[] = {
 	{"top_enable", lbox_fiber_top_enable},
 	{"top_disable", lbox_fiber_top_disable},
 #endif /* ENABLE_FIBER_TOP */
-#if ENABLE_BACKTRACE
+#ifdef ENABLE_BACKTRACE
 	{"parent_bt_enable", lbox_fiber_parent_bt_enable},
 	{"parent_bt_disable", lbox_fiber_parent_bt_disable},
 #endif /* ENABLE_BACKTRACE */
-- 
2.31.1

[Tarantool-patches] [PATCH v2 4/4] fiber: refactor C backtrace and add changelog

[Egor Elchinov via Tarantool-patches]

From: Egor Elchinov <elchinov.es@gmail.com>

Backtrace cache support and cxx demangling added.
DWARF version and lua test fixed
to support specific platforms.
Changelog added.

Closes: #4002
---
 .../gh-4002-fiber-creation-backtrace.md       |   8 +
 src/lib/core/backtrace.cc                     | 164 +++++++++++++-----
 .../gh-4002-fiber-creation-backtrace.result   |   4 +-
 .../gh-4002-fiber-creation-backtrace.test.lua |   4 +-
 4 files changed, 132 insertions(+), 48 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4002-fiber-creation-backtrace.md

diff --git a/changelogs/unreleased/gh-4002-fiber-creation-backtrace.md b/changelogs/unreleased/gh-4002-fiber-creation-backtrace.md
new file mode 100644
index 000000000..1e1ac177a
--- /dev/null
+++ b/changelogs/unreleased/gh-4002-fiber-creation-backtrace.md
@@ -0,0 +1,8 @@
+## feature/fiber
+
+ * Added new subtable `parent_backtrace` to the `fiber.info()`
+   containing C and Lua backtrace chunks of fiber creation.
+ * Added `fiber.parent_bt_enable()` and `fiber.parent_bt_disable()`
+   options in order to switch on/off the ability to collect
+   parent backtraces for newly created fibers and to
+   show/hide `parent_backtrace` subtables in the `fiber.info()`.
diff --git a/src/lib/core/backtrace.cc b/src/lib/core/backtrace.cc
index f97c4c0a9..c82aa27af 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -47,7 +47,6 @@
 #include <libunwind.h>
 
 #ifdef TARGET_OS_DARWIN
-#include <execinfo.h>
 #include <dlfcn.h>
 #endif
 
@@ -79,19 +78,29 @@ backtrace_proc_cache_clear(void)
 	proc_cache = NULL;
 }
 
-const char *
-get_proc_name(unw_cursor_t *unw_cur, unw_word_t *offset, bool skip_cache)
+int
+backtrace_proc_cache_find(unw_word_t ip, const char **name, unw_word_t *offset)
 {
-	static __thread char proc_name[BACKTRACE_NAME_MAX];
-	unw_word_t ip;
-	unw_get_reg(unw_cur, UNW_REG_IP, &ip);
+	struct proc_cache_entry *entry;
+	mh_int_t k;
 
-	if (skip_cache) {
-		unw_get_proc_name(unw_cur, proc_name, sizeof(proc_name),
-				  offset);
-		return proc_name;
+	if (proc_cache != NULL) {
+		k  = mh_i64ptr_find(proc_cache, ip, NULL);
+		if (k != mh_end(proc_cache)) {
+			entry = (struct proc_cache_entry *)
+				mh_i64ptr_node(proc_cache, k)->val;
+			*offset = entry->offset;
+			*name = entry->name;
+			return 0;
+		}
 	}
 
+	return -1;
+}
+
+int
+backtrace_proc_cache_put(unw_word_t ip, const char *name, unw_word_t offset)
+{
 	struct proc_cache_entry *entry;
 	struct mh_i64ptr_node_t node;
 	mh_int_t k;
@@ -99,39 +108,51 @@ get_proc_name(unw_cursor_t *unw_cur, unw_word_t *offset, bool skip_cache)
 	if (proc_cache == NULL) {
 		region_create(&cache_region, &cord()->slabc);
 		proc_cache = mh_i64ptr_new();
-		if (proc_cache == NULL) {
-			unw_get_proc_name(unw_cur, proc_name, sizeof(proc_name),
-					  offset);
-			goto error;
-		}
+		if (proc_cache == NULL)
+			return -1;
+	}
+
+	size_t size;
+	entry = region_alloc_object(&cache_region, typeof(*entry),
+				    &size);
+	if (entry == NULL)
+		return -1;
+
+	node.key = ip;
+	node.val = entry;
+	entry->offset = offset;
+	snprintf(entry->name, BACKTRACE_NAME_MAX, "%s", name);
+
+	k = mh_i64ptr_put(proc_cache, &node, NULL, NULL);
+	if (k == mh_end(proc_cache)) {
+		size_t used = region_used(&cache_region);
+		region_truncate(&cache_region, used - size);
+		return -1;
 	}
 
-	k  = mh_i64ptr_find(proc_cache, ip, NULL);
-	if (k != mh_end(proc_cache)) {
-		entry = (struct proc_cache_entry *)
-			mh_i64ptr_node(proc_cache, k)->val;
-		snprintf(proc_name, BACKTRACE_NAME_MAX, "%s", entry->name);
-		*offset = entry->offset;
-	}  else {
+	return 0;
+}
+
+const char *
+get_proc_name(unw_cursor_t *unw_cur, unw_word_t *offset, bool skip_cache)
+{
+	static __thread char proc_name[BACKTRACE_NAME_MAX];
+	const char *cache_name;
+	unw_word_t ip;
+
+	if (skip_cache) {
 		unw_get_proc_name(unw_cur, proc_name, sizeof(proc_name),
 				  offset);
-		size_t size;
-		entry = region_alloc_object(&cache_region, typeof(*entry),
-					    &size);
-		if (entry == NULL)
-			goto error;
-		node.key = ip;
-		node.val = entry;
-		snprintf(entry->name, BACKTRACE_NAME_MAX, "%s", proc_name);
-		entry->offset = *offset;
-
-		k = mh_i64ptr_put(proc_cache, &node, NULL, NULL);
-		if (k == mh_end(proc_cache)) {
-			free(entry);
-			goto error;
-		}
+		return proc_name;
 	}
-error:
+
+	unw_get_reg(unw_cur, UNW_REG_IP, &ip);
+	if (backtrace_proc_cache_find(ip, &cache_name, offset) == 0) {
+		return cache_name;
+	}
+
+	unw_get_proc_name(unw_cur, proc_name, sizeof(proc_name), offset);
+	backtrace_proc_cache_put(ip, proc_name, *offset);
 	return proc_name;
 }
 
@@ -450,7 +471,25 @@ backtrace_collect_ip(void **ip_buf, int limit)
 #ifndef TARGET_OS_DARWIN
 	unw_backtrace(ip_buf, limit);
 #else
-	backtrace(ip_buf, limit);
+	/*
+	 * This dumb implementation was chosen because the DARWIN
+	 * lacks unw_backtrace() routine from libunwind and
+	 * usual backtrace() from <execinfo.h> has less capabilities
+	 * than the libunwind version which uses DWARF.
+	 */
+	unw_cursor_t unw_cur;
+	unw_context_t unw_ctx;
+	int frame_no = 0;
+	unw_word_t ip;
+
+	unw_getcontext(&unw_ctx);
+	unw_init_local(&unw_cur, &unw_ctx);
+
+	while (frame_no < limit && unw_step(&unw_cur) > 0) {
+		unw_get_reg(&unw_cur, UNW_REG_IP, &ip);
+		ip_buf[frame_no] = (void *)ip;
+		++frame_no;
+	}
 #endif
 }
 
@@ -469,12 +508,15 @@ void
 backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
 		     void *cb_ctx)
 {
+	int demangle_status;
+	char *demangle_buf = NULL;
+	size_t demangle_buf_len = 0;
 #ifndef TARGET_OS_DARWIN
 	char proc_name[BACKTRACE_NAME_MAX];
 	unw_word_t ip = 0, offset = 0;
 	unw_proc_info_t pi;
 	int frame_no, ret = 0;
-	char *proc = NULL;
+	const char *proc = NULL;
 
 	unw_accessors_t *acc = unw_get_accessors(unw_local_addr_space);
 
@@ -487,21 +529,37 @@ backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
 	     frame_no++) {
 		ip = (unw_word_t)ip_buf[frame_no];
 
-		if (acc->get_proc_name == NULL) {
+		if (backtrace_proc_cache_find(ip, &proc, &offset) == 0) {
+			ret = 0;
+		} else if (acc->get_proc_name == NULL) {
 			ret = unw_get_proc_info_by_ip(unw_local_addr_space,
 						      ip, &pi, NULL);
 			offset = ip - pi.start_ip;
+			proc = NULL;
+			backtrace_proc_cache_put(ip, proc, offset);
 		} else {
 			ret = acc->get_proc_name(unw_local_addr_space, ip,
 						 proc_name, sizeof(proc_name),
 						 &offset, NULL);
 			proc = proc_name;
+			backtrace_proc_cache_put(ip, proc, offset);
+		}
+
+		if (proc != NULL) {
+			char *cxxname = abi::__cxa_demangle(proc, demangle_buf,
+							    &demangle_buf_len,
+							    &demangle_status);
+			if (cxxname != NULL) {
+				demangle_buf = cxxname;
+				proc = cxxname;
+			}
 		}
 		if (ret != 0 || cb(frame_no - 1, (void *)ip, proc,
 				   (size_t)offset, cb_ctx) != 0)
 			break;
 	}
 
+	free(demangle_buf);
 	if (ret != 0)
 		say_debug("unwinding error: %s", unw_strerror(ret));
 #else
@@ -509,17 +567,35 @@ backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
 	void *ip = NULL;
 	size_t offset = 0;
 	Dl_info dli;
+	const char *proc = NULL;
 
 	for (frame_no = 1; frame_no < limit && ip_buf[frame_no] != NULL;
 	     ++frame_no) {
 		ip = ip_buf[frame_no];
-		ret = dladdr(ip, &dli);
-		offset = (char *)ip - (char *)dli.dli_saddr;
+		if (backtrace_proc_cache_find((unw_word_t)ip, &proc, 
+					      &offset) == 0) {
+			ret = 0;
+		} else {
+			ret = dladdr(ip, &dli);
+			offset = (char *)ip - (char *)dli.dli_saddr;
+			proc = dli.dli_sname;
+			backtrace_proc_cache_put((unw_word_t)ip, proc, offset);
+		}
 
-		if (cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
+		if (proc != NULL) {
+			char *cxxname = abi::__cxa_demangle(proc, demangle_buf,
+							    &demangle_buf_len,
+							    &demangle_status);
+			if (cxxname != NULL) {
+				demangle_buf = cxxname;
+				proc = cxxname;
+			}
+		}
+		if (cb(frame_no - 1, ip, proc, offset, cb_ctx) != 0)
 			break;
 	}
 
+	free(demangle_buf);
 	if (ret == 0)
 		say_debug("unwinding error: %i", ret);
 #endif
diff --git a/test/app/gh-4002-fiber-creation-backtrace.result b/test/app/gh-4002-fiber-creation-backtrace.result
index 6a075911d..91b0cb9ce 100644
--- a/test/app/gh-4002-fiber-creation-backtrace.result
+++ b/test/app/gh-4002-fiber-creation-backtrace.result
@@ -9,10 +9,10 @@ test_run = require('test_run').new()
  | ---
  | ...
 
-local stack_len = 0
+stack_len = 0
  | ---
  | ...
-local parent_stack_len = 0
+parent_stack_len = 0
  | ---
  | ...
 
diff --git a/test/app/gh-4002-fiber-creation-backtrace.test.lua b/test/app/gh-4002-fiber-creation-backtrace.test.lua
index 79a516860..3c39bb48e 100644
--- a/test/app/gh-4002-fiber-creation-backtrace.test.lua
+++ b/test/app/gh-4002-fiber-creation-backtrace.test.lua
@@ -2,8 +2,8 @@ yaml = require('yaml')
 fiber = require('fiber')
 test_run = require('test_run').new()
 
-local stack_len = 0
-local parent_stack_len = 0
+stack_len = 0
+parent_stack_len = 0
 
 test_run:cmd('setopt delimiter ";"')
 foo = function()
-- 
2.31.1

Re: [Tarantool-patches] [PATCH v2 1/4] fiber: add PoC for fiber creation backtrace

[Cyrill Gorcunov via Tarantool-patches]

On Fri, Jul 09, 2021 at 02:03:50PM +0300, Egor Elchinov via Tarantool-patches wrote:
> +void
> +backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
> +		     void *cb_ctx)
> +{
> +#ifndef TARGET_OS_DARWIN
> +	char proc_name[BACKTRACE_NAME_MAX];
> +	unw_word_t ip = 0, offset = 0;
> +	unw_proc_info_t pi;
> +	int frame_no, ret = 0;
> +	char *proc = NULL;
> +
> +	unw_accessors_t *acc = unw_get_accessors(unw_local_addr_space);
> +
> +	/*
> +	 * RIPs collecting comes from inside a helper routine
> +	 * so we skip the collector function address itself thus
> +	 * start fetching functions with frame number = 1.
> +	 */
> +	for (frame_no = 1; frame_no < limit && ip_buf[frame_no] != NULL;
> +	     frame_no++) {
> +		ip = (unw_word_t)ip_buf[frame_no];
> +
> +		if (acc->get_proc_name == NULL) {
> +			ret = unw_get_proc_info_by_ip(unw_local_addr_space,
> +						      ip, &pi, NULL);
> +			offset = ip - pi.start_ip;
> +		} else {
> +			ret = acc->get_proc_name(unw_local_addr_space, ip,
> +						 proc_name, sizeof(proc_name),
> +						 &offset, NULL);
> +			proc = proc_name;
> +		}
> +		if (ret != 0 || cb(frame_no - 1, (void *)ip, proc,
> +				   (size_t)offset, cb_ctx) != 0)
> +			break;
> +	}
> +
> +	if (ret != 0)
> +		say_debug("unwinding error: %s", unw_strerror(ret));
> +#else
> +	int frame_no, ret = 1;
> +	void *ip = NULL;
> +	size_t offset = 0;
> +	Dl_info dli;
> +
> +	for (frame_no = 1; frame_no < limit && ip_buf[frame_no] != NULL;
> +	     ++frame_no) {
> +		ip = ip_buf[frame_no];
> +		ret = dladdr(ip, &dli);
> +		offset = (char *)ip - (char *)dli.dli_saddr;
> +
> +		if (cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
> +			break;

Looks like you forgot to test for dladdr error if I'm not missing
something obvious. So it could be like

		ret = dladdr(ip, &dli);
		if (ret == 0)
			break;
		offset = (char *)ip - (char *)dli.dli_saddr;
		if (cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
			break;

no?

> +	}
> +
> +	if (ret == 0)
> +		say_debug("unwinding error: %i", ret);
> +#endif
> +}

Re: [Tarantool-patches] [PATCH v2 2/4] fiber: add option and PoC for Lua parent backtrace

[Cyrill Gorcunov via Tarantool-patches]

On Fri, Jul 09, 2021 at 02:03:51PM +0300, Egor Elchinov via Tarantool-patches wrote:
> 
> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index 924ff3c82..dd26e2f13 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -220,6 +220,10 @@ fiber_mprotect(void *addr, size_t len, int prot)
>  static __thread bool fiber_top_enabled = false;
>  #endif /* ENABLE_FIBER_TOP */
>  
> +#if ENABLE_BACKTRACE
> +static __thread bool fiber_parent_bt_enabled = false;
> +#endif /* ENABLE_BACKTRACE */
> +

So this variable is declared as TLS in a manner of fiber_top
functionality. You know I somehow find this fishy because
it is unclear why it is done this way. I think we should
revisit this moment for @fiber_top_enabled enabled as well
but not in this series, so lets leave it as is.

Please revisit snprintf calls in the patch and strncpy
as well because they _do_not_ append terminating \0 symbol
if destination size is not enough. I think we should add
\0 explicitly all the time.

I mean

+static int
+fiber_parent_backtrace_cb(int frameno, void *frameret, const char *func,
+			  size_t offset, void *cb_ctx)
+{
	...
+	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, "?");

we could add

	buf[sizeof(buf)-1] = '\0';

Actually 512 bytes for function name should be more than enough but
better be on a safe side.

Other than that the patch looks ok to me, great job, thanks!

Re: [Tarantool-patches] [PATCH v2 3/4] fiber: refactor lua backtrace routines

[Cyrill Gorcunov via Tarantool-patches]

On Fri, Jul 09, 2021 at 02:03:52PM +0300, Egor Elchinov via Tarantool-patches wrote:
> From: Egor Elchinov <elchinov.es@gmail.com>
> 
> Lua backtrace with callback moved to new routine
> `lua_backtrace_cb()` similar to the `backtrace_cb()`
> for C backtrace.
> 
> Needed for: #4002
>  #ifdef ENABLE_BACKTRACE
>  static void
> -dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
> +dump_lua_frame(struct lua_State *L, const char *name, const char *src, int line,
> +	       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);
> +		 name != NULL ? name : "(unnamed)", src, line);

Please add explicit \0 here to the buffer

	buf[sizeof(buf)-1] = 0;

and in other places of this patch as well.

>  
> +static int
> +dump_lua_frame_cb(lua_Debug *ar, int tb_frame, void *cb_ctx)
> +{
> +	(void) tb_frame;
> +	struct lua_fiber_tb_ctx *tb_ctx = (struct lua_fiber_tb_ctx *)cb_ctx;

Side note: for plain C you don't need to put explicit type conversion
from void, iow

	struct lua_fiber_tb_ctx *tb_ctx = cb_ctx;

ie enough, but up to you. The rest is ok to me, thanks!

Re: [Tarantool-patches] [PATCH v2 4/4] fiber: refactor C backtrace and add changelog

[Cyrill Gorcunov via Tarantool-patches]

On Fri, Jul 09, 2021 at 02:03:53PM +0300, Egor Elchinov via Tarantool-patches wrote:
> +
> +int
> +backtrace_proc_cache_put(unw_word_t ip, const char *name, unw_word_t offset)
> +{
>  	struct proc_cache_entry *entry;
>  	struct mh_i64ptr_node_t node;
>  	mh_int_t k;
> @@ -99,39 +108,51 @@ get_proc_name(unw_cursor_t *unw_cur, unw_word_t *offset, bool skip_cache)
>  	if (proc_cache == NULL) {
>  		region_create(&cache_region, &cord()->slabc);
>  		proc_cache = mh_i64ptr_new();
> -		if (proc_cache == NULL) {
> -			unw_get_proc_name(unw_cur, proc_name, sizeof(proc_name),
> -					  offset);
> -			goto error;
> -		}
> +		if (proc_cache == NULL)
> +			return -1;
> +	}
> +
> +	size_t size;
> +	entry = region_alloc_object(&cache_region, typeof(*entry),
> +				    &size);
> +	if (entry == NULL)
> +		return -1;
> +
> +	node.key = ip;
> +	node.val = entry;
> +	entry->offset = offset;
> +	snprintf(entry->name, BACKTRACE_NAME_MAX, "%s", name);

Please add

	entry->name[BACKTRACE_NAME_MAX - 1] = 0;

...

The rest looks ok to me, thanks!

Re: [Tarantool-patches] [PATCH v2 2/4] fiber: add option and PoC for Lua parent backtrace

[Egor Elchinov via Tarantool-patches]

On 12.07.2021 15:09, Cyrill Gorcunov via Tarantool-patches wrote:
> On Fri, Jul 09, 2021 at 02:03:51PM +0300, Egor Elchinov via Tarantool-patches wrote:
>>
>> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
>> index 924ff3c82..dd26e2f13 100644
>> --- a/src/lib/core/fiber.c
>> +++ b/src/lib/core/fiber.c
>> @@ -220,6 +220,10 @@ fiber_mprotect(void *addr, size_t len, int prot)
>>   static __thread bool fiber_top_enabled = false;
>>   #endif /* ENABLE_FIBER_TOP */
>>   
>> +#if ENABLE_BACKTRACE
>> +static __thread bool fiber_parent_bt_enabled = false;
>> +#endif /* ENABLE_BACKTRACE */
>> +
> 
> So this variable is declared as TLS in a manner of fiber_top
> functionality. You know I somehow find this fishy because
> it is unclear why it is done this way. I think we should
> revisit this moment for @fiber_top_enabled enabled as well
> but not in this series, so lets leave it as is.
> 
> Please revisit snprintf calls in the patch and strncpy
> as well because they _do_not_ append terminating \0 symbol
> if destination size is not enough. I think we should add
> \0 explicitly all the time.
Thanks, fixed hereinafter.
> 
> I mean
> 
> +static int
> +fiber_parent_backtrace_cb(int frameno, void *frameret, const char *func,
> +			  size_t offset, void *cb_ctx)
> +{
> 	...
> +	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, "?");
> 
> we could add
> 
> 	buf[sizeof(buf)-1] = '\0';
> 
> Actually 512 bytes for function name should be more than enough but
> better be on a safe side.
> 
> Other than that the patch looks ok to me, great job, thanks!
> 

Re: [Tarantool-patches] [PATCH v2 3/4] fiber: refactor lua backtrace routines

[Egor Elchinov via Tarantool-patches]

On 12.07.2021 15:13, Cyrill Gorcunov via Tarantool-patches wrote:
> On Fri, Jul 09, 2021 at 02:03:52PM +0300, Egor Elchinov via Tarantool-patches wrote:
>> From: Egor Elchinov <elchinov.es@gmail.com>
>>
>> Lua backtrace with callback moved to new routine
>> `lua_backtrace_cb()` similar to the `backtrace_cb()`
>> for C backtrace.
>>
>> Needed for: #4002
>>   #ifdef ENABLE_BACKTRACE
>>   static void
>> -dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
>> +dump_lua_frame(struct lua_State *L, const char *name, const char *src, int line,
>> +	       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);
>> +		 name != NULL ? name : "(unnamed)", src, line);
> 
> Please add explicit \0 here to the buffer
> 
> 	buf[sizeof(buf)-1] = 0;
> 
> and in other places of this patch as well.
> 
Fixed.
>>   
>> +static int
>> +dump_lua_frame_cb(lua_Debug *ar, int tb_frame, void *cb_ctx)
>> +{
>> +	(void) tb_frame;
>> +	struct lua_fiber_tb_ctx *tb_ctx = (struct lua_fiber_tb_ctx *)cb_ctx;
> 
> Side note: for plain C you don't need to put explicit type conversion
> from void, iow
> 
> 	struct lua_fiber_tb_ctx *tb_ctx = cb_ctx;
> 
> ie enough, but up to you. The rest is ok to me, thanks!
> 
Thanks for this observation. Although, it seems to me that in this case 
it's better to make an explicit cast for consistency with other similar 
cases and C++ rules.

Re: [Tarantool-patches] [PATCH v2 4/4] fiber: refactor C backtrace and add changelog

[Egor Elchinov via Tarantool-patches]

On 12.07.2021 15:35, Cyrill Gorcunov via Tarantool-patches wrote:
> On Fri, Jul 09, 2021 at 02:03:53PM +0300, Egor Elchinov via Tarantool-patches wrote:
>> +
>> +int
>> +backtrace_proc_cache_put(unw_word_t ip, const char *name, unw_word_t offset)
>> +{
>>   	struct proc_cache_entry *entry;
>>   	struct mh_i64ptr_node_t node;
>>   	mh_int_t k;
>> @@ -99,39 +108,51 @@ get_proc_name(unw_cursor_t *unw_cur, unw_word_t *offset, bool skip_cache)
>>   	if (proc_cache == NULL) {
>>   		region_create(&cache_region, &cord()->slabc);
>>   		proc_cache = mh_i64ptr_new();
>> -		if (proc_cache == NULL) {
>> -			unw_get_proc_name(unw_cur, proc_name, sizeof(proc_name),
>> -					  offset);
>> -			goto error;
>> -		}
>> +		if (proc_cache == NULL)
>> +			return -1;
>> +	}
>> +
>> +	size_t size;
>> +	entry = region_alloc_object(&cache_region, typeof(*entry),
>> +				    &size);
>> +	if (entry == NULL)
>> +		return -1;
>> +
>> +	node.key = ip;
>> +	node.val = entry;
>> +	entry->offset = offset;
>> +	snprintf(entry->name, BACKTRACE_NAME_MAX, "%s", name);
> 
> Please add
> 
> 	entry->name[BACKTRACE_NAME_MAX - 1] = 0;
> 
Thanks, fixed.
> ...
> 
> The rest looks ok to me, thanks!
>