Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace
@ 2021-07-01 20:24 Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches, Egor Elchinov

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.

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

Egor Elchinov (7):
  fiber: add PoC for fiber creation backtrace
  fiber: fix DARWIN build
  fiber: apply fix patch
  fiber: add PoC for Lua parent backtrace
  fiber: add dynamic option for parent backtrace
  fiber: refactor lua backtrace routine
  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                               | 235 ++++++++++++++---
 src/lua/fiber.h                               |  19 ++
 .../gh-4002-fiber-creation-backtrace.result   |  78 ++++++
 .../gh-4002-fiber-creation-backtrace.test.lua |  34 +++
 10 files changed, 626 insertions(+), 76 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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber creation backtrace
  2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:24 ` Egor Elchinov via Tarantool-patches
  2021-07-01 20:44   ` Egor Elchinov via Tarantool-patches
                     ` (2 more replies)
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 2/7] fiber: fix DARWIN build Egor Elchinov via Tarantool-patches
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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.

Needed for: #4002
---
 src/lib/core/backtrace.cc                     | 66 +++++++++++++++++++
 src/lib/core/backtrace.h                      |  6 ++
 src/lib/core/fiber.c                          |  8 +++
 src/lib/core/fiber.h                          | 16 +++++
 src/lua/fiber.c                               |  9 +++
 .../gh-4002-fiber-creation-backtrace.result   | 57 ++++++++++++++++
 .../gh-4002-fiber-creation-backtrace.test.lua | 27 ++++++++
 7 files changed, 189 insertions(+)
 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..276933ad6 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -432,6 +432,72 @@ 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
+fast_trace_collect(void **ip_buf, int limit)
+{
+	memset(ip_buf, 0, limit * sizeof(*ip_buf));
+	unw_backtrace(ip_buf, limit);
+}
+
+/**
+ * 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
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
+{
+	static __thread char proc_name[BACKTRACE_NAME_MAX];
+	int frame_no = 0;
+	unw_word_t ip = 0, offset = 0;
+	unw_proc_info_t pi;
+	int ret = 0;
+	char* proc = NULL;
+
+	unw_accessors_t* acc = unw_get_accessors(unw_local_addr_space);
+	assert(acc);
+
+	for (frame_no = 0; 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 || (frame_no > 0 &&
+		    cb(frame_no - 1, (void *)ip, proc,
+	 	       (size_t)offset, cb_ctx) != 0))
+			break;
+	}
+
+#ifndef TARGET_OS_DARWIN
+	if (ret != 0)
+		say_debug("unwinding error: %s", unw_strerror(ret));
+#else
+	if (ret != 0)
+		say_debug("unwinding error: %i", ret);
+#endif
+}
+
 void
 print_backtrace(void)
 {
diff --git a/src/lib/core/backtrace.h b/src/lib/core/backtrace.h
index e0ae56be4..1560f03e3 100644
--- a/src/lib/core/backtrace.h
+++ b/src/lib/core/backtrace.h
@@ -55,6 +55,12 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx);
 void
 backtrace_proc_cache_clear(void);
 
+void
+fast_trace_collect(void **ip_buf, int limit);
+
+void
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx);
+
 #endif /* ENABLE_BACKTRACE */
 
 #if defined(__cplusplus)
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 759c7da6a..73500e043 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
+	fast_trace_collect(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..6a155c0fe 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..dc7051edd 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -308,6 +308,15 @@ 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);
+		fast_trace_foreach(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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 2/7] fiber: fix DARWIN build
  2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:24 ` Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch Egor Elchinov via Tarantool-patches
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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

Due to the limited libunwind API under osx
this fix 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 | 38 +++++++++++++++++++++++++++++++-------
 src/lib/core/backtrace.h  |  2 +-
 src/lib/core/crash.c      |  2 +-
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/lib/core/backtrace.cc b/src/lib/core/backtrace.cc
index 276933ad6..a1727374c 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;
@@ -442,7 +447,11 @@ void
 fast_trace_collect(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
 }
 
 /**
@@ -459,12 +468,12 @@ fast_trace_collect(void **ip_buf, int limit)
 void
 fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
 {
+#ifndef TARGET_OS_DARWIN
 	static __thread char proc_name[BACKTRACE_NAME_MAX];
-	int frame_no = 0;
+	char* proc = NULL;
+	int frame_no = 0, ret = 0;
 	unw_word_t ip = 0, offset = 0;
 	unw_proc_info_t pi;
-	int ret = 0;
-	char* proc = NULL;
 
 	unw_accessors_t* acc = unw_get_accessors(unw_local_addr_space);
 	assert(acc);
@@ -489,11 +498,26 @@ fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
 			break;
 	}
 
-#ifndef TARGET_OS_DARWIN
 	if (ret != 0)
 		say_debug("unwinding error: %s", unw_strerror(ret));
 #else
-	if (ret != 0)
+	int frame_no = 0, ret = 1;
+	void *ip = NULL;
+	size_t offset = 0;
+	Dl_info dli;
+
+	for (frame_no = 0; 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 (frame_no > 0 &&
+		    cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
+			break;
+	}
+
+	if (ret == 0)
 		say_debug("unwinding error: %i", ret);
 #endif
 }
@@ -502,7 +526,7 @@ 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 1560f03e3..8bb668366 100644
--- a/src/lib/core/backtrace.h
+++ b/src/lib/core/backtrace.h
@@ -41,7 +41,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);
 
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
-- 
2.31.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch
  2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 2/7] fiber: fix DARWIN build Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:24 ` Egor Elchinov via Tarantool-patches
  2021-07-05  9:42   ` Cyrill Gorcunov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 4/7] fiber: add PoC for Lua parent backtrace Egor Elchinov via Tarantool-patches
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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

Some mistakes fixed.
More consistent naming provided.
Based on review and patch from
Cyrill Gorcunov <gorcunov@gmail.com>

Needed for: #4002
---
 src/lib/core/backtrace.cc | 47 +++++++++++++++++++++------------------
 src/lib/core/backtrace.h  |  8 ++++---
 src/lib/core/fiber.c      |  2 +-
 src/lib/core/fiber.h      |  2 +-
 src/lua/fiber.c           |  5 +++--
 5 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/src/lib/core/backtrace.cc b/src/lib/core/backtrace.cc
index a1727374c..f97c4c0a9 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -443,8 +443,8 @@ out:
  * Must be by far faster than usual backtrace according to the
  * libunwind doc for unw_backtrace().
  */
-void
-fast_trace_collect(void **ip_buf, int limit)
+void NOINLINE
+backtrace_collect_ip(void **ip_buf, int limit)
 {
 	memset(ip_buf, 0, limit * sizeof(*ip_buf));
 #ifndef TARGET_OS_DARWIN
@@ -466,54 +466,57 @@ fast_trace_collect(void **ip_buf, int limit)
  * TODO: to add cache and demangling support
  */
 void
-fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
+backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
+		     void *cb_ctx)
 {
 #ifndef TARGET_OS_DARWIN
-	static __thread char proc_name[BACKTRACE_NAME_MAX];
-	char* proc = NULL;
-	int frame_no = 0, ret = 0;
+	char proc_name[BACKTRACE_NAME_MAX];
 	unw_word_t ip = 0, offset = 0;
 	unw_proc_info_t pi;
-
-	unw_accessors_t* acc = unw_get_accessors(unw_local_addr_space);
-	assert(acc);
-
-	for (frame_no = 0; frame_no < limit && ip_buf[frame_no] != NULL;
-	     ++frame_no) {
+	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_name, sizeof(proc_name),
+						 &offset, NULL);
 			proc = proc_name;
 		}
-
-		if (ret != 0 || (frame_no > 0 &&
-		    cb(frame_no - 1, (void *)ip, proc,
-	 	       (size_t)offset, cb_ctx) != 0))
+		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 = 0, ret = 1;
+	int frame_no, ret = 1;
 	void *ip = NULL;
 	size_t offset = 0;
 	Dl_info dli;
 
-	for (frame_no = 0; frame_no < limit && ip_buf[frame_no] != 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 (frame_no > 0 &&
-		    cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
+		if (cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
 			break;
 	}
 
diff --git a/src/lib/core/backtrace.h b/src/lib/core/backtrace.h
index 8bb668366..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)
@@ -55,11 +56,12 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx);
 void
 backtrace_proc_cache_clear(void);
 
-void
-fast_trace_collect(void **ip_buf, int limit);
+void NOINLINE
+backtrace_collect_ip(void **ip_buf, int limit);
 
 void
-fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx);
+backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
+		     void *cb_ctx);
 
 #endif /* ENABLE_BACKTRACE */
 
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 73500e043..924ff3c82 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -1265,7 +1265,7 @@ 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
-	fast_trace_collect(fiber->parent_bt_ip_buf, FIBER_PARENT_BT_MAX);
+	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 6a155c0fe..beed58866 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -654,7 +654,7 @@ struct fiber {
 	char inline_name[FIBER_NAME_INLINE];
 #if ENABLE_BACKTRACE
 	/** Fiber creation backtrace chunk. */
-	void* parent_bt_ip_buf[FIBER_PARENT_BT_MAX];
+	void *parent_bt_ip_buf[FIBER_PARENT_BT_MAX];
 #endif /* ENABLE_BACKTRACE */
 };
 
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index dc7051edd..7b21361d4 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -314,8 +314,9 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 		tb_ctx.R = NULL;
 		lua_pushstring(L, "backtrace_parent");
 		lua_newtable(L);
-		fast_trace_foreach(fiber_backtrace_cb, f->parent_bt_ip_buf,
-		     		   FIBER_PARENT_BT_MAX, &tb_ctx);
+		backtrace_foreach_ip(fiber_backtrace_cb,
+				     f->parent_bt_ip_buf,
+				     FIBER_PARENT_BT_MAX, &tb_ctx);
 		lua_settable(L, -3);
 #endif /* ENABLE_BACKTRACE */
 	}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 4/7] fiber: add PoC for Lua parent backtrace
  2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
                   ` (2 preceding siblings ...)
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:24 ` Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for " Egor Elchinov via Tarantool-patches
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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.

Needed for: #4002
---
 src/lib/core/fiber.h |   7 ++
 src/lua/fiber.c      | 168 ++++++++++++++++++++++++++++++++++++++++---
 src/lua/fiber.h      |  19 +++++
 3 files changed, 185 insertions(+), 9 deletions(-)

diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index beed58866..ce1c83d11 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.
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 7b21361d4..026e30bc6 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,14 +446,14 @@ 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;
+		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_backtrace_cb,
+		backtrace_foreach_ip(fiber_parent_backtrace_cb,
 				     f->parent_bt_ip_buf,
-				     FIBER_PARENT_BT_MAX, &tb_ctx);
+				     FIBER_PARENT_BT_MAX, &parent_tb_ctx);
 		lua_settable(L, -3);
 #endif /* ENABLE_BACKTRACE */
 	}
@@ -481,10 +618,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 +647,11 @@ fiber_create(struct lua_State *L)
 		luaT_error(L);
 	}
 
+#ifdef ENABLE_BACKTRACE
+	// TODO: error handling
+	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 +1032,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;
 }
 
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
 */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for parent backtrace
  2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
                   ` (3 preceding siblings ...)
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 4/7] fiber: add PoC for Lua parent backtrace Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:24 ` Egor Elchinov via Tarantool-patches
  2021-07-05  9:49   ` Cyrill Gorcunov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 6/7] fiber: refactor lua backtrace routine Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 7/7] fiber: refactor C backtrace and add changelog Egor Elchinov via Tarantool-patches
  6 siblings, 1 reply; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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

This patch 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                          | 11 ++++
 src/lua/fiber.c                               | 62 +++++++++++++------
 .../gh-4002-fiber-creation-backtrace.result   | 21 +++++++
 .../gh-4002-fiber-creation-backtrace.test.lua |  7 +++
 5 files changed, 110 insertions(+), 20 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 ce1c83d11..477b68e4c 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -878,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 026e30bc6..fe01ae23b 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -205,7 +205,7 @@ struct lua_parent_tb_ctx {
 	int tb_frame;
 };
 
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 static void
 dump_lua_frame(struct lua_State *L, lua_Debug *ar, int tb_frame)
 {
@@ -433,7 +433,7 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 	lua_settable(L, -3);
 
 	if (backtrace) {
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 		struct lua_fiber_tb_ctx tb_ctx;
 		struct lua_parent_tb_ctx parent_tb_ctx;
 		tb_ctx.L = L;
@@ -446,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);
 
-		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);
+		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;
@@ -471,7 +474,7 @@ lbox_fiber_statof(struct fiber *f, void *cb_ctx, bool backtrace)
 	return 0;
 }
 
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 static int
 lbox_fiber_statof_bt(struct fiber *f, void *cb_ctx)
 {
@@ -560,7 +563,7 @@ lbox_fiber_top_disable(struct lua_State *L)
 }
 #endif /* ENABLE_FIBER_TOP */
 
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 bool
 lbox_do_backtrace(struct lua_State *L)
 {
@@ -578,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 */
 
 /**
@@ -586,7 +605,7 @@ lbox_do_backtrace(struct lua_State *L)
 static int
 lbox_fiber_info(struct lua_State *L)
 {
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 	bool do_backtrace = lbox_do_backtrace(L);
 	if (do_backtrace) {
 		lua_newtable(L);
@@ -621,7 +640,7 @@ lua_fiber_run_f(MAYBE_UNUSED va_list ap)
 	if (f->flags & FIBER_IS_JOINABLE) {
 		lua_pushinteger(L, coro_ref);
 	} else {
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 		fiber_parent_bt_free(f);
 #endif
 		luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
@@ -647,9 +666,10 @@ fiber_create(struct lua_State *L)
 		luaT_error(L);
 	}
 
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 	// TODO: error handling
-	fiber_parent_bt_init(f, L);
+	if (fiber_parent_bt_is_enabled())
+		fiber_parent_bt_init(f, L);
 #endif
 
 	/* Move the arguments to the new coro */
@@ -748,7 +768,7 @@ lbox_fiber_object_info(struct lua_State *L)
 	struct fiber *f = lbox_get_fiber(L);
 	if (f == NULL)
 		luaL_error(L, "the fiber is dead");
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 	bool do_backtrace = lbox_do_backtrace(L);
 	if (do_backtrace) {
 		lua_newtable(L);
@@ -1033,7 +1053,7 @@ lbox_fiber_join(struct lua_State *L)
 		}
 	}
 	if (child_L != NULL) {
-#ifdef ENABLE_BACKTRACE
+#if ENABLE_BACKTRACE
 		fiber_parent_bt_free(fiber);
 #endif
 		luaL_unref(L, LUA_REGISTRYINDEX, coro_ref);
@@ -1091,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/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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 6/7] fiber: refactor lua backtrace routine
  2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
                   ` (4 preceding siblings ...)
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for " Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:24 ` Egor Elchinov via Tarantool-patches
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 7/7] fiber: refactor C backtrace and add changelog Egor Elchinov via Tarantool-patches
  6 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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 | 165 +++++++++++++++++++++---------------------------
 1 file changed, 73 insertions(+), 92 deletions(-)

diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index fe01ae23b..9e0c33a8f 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);
+
 #if 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;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 7/7] fiber: refactor C backtrace and add changelog
  2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
                   ` (5 preceding siblings ...)
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 6/7] fiber: refactor lua backtrace routine Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:24 ` Egor Elchinov via Tarantool-patches
  6 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:24 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber creation backtrace
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
@ 2021-07-01 20:44   ` Egor Elchinov via Tarantool-patches
  2021-07-02  7:00   ` Egor Elchinov via Tarantool-patches
  2021-07-02  7:11   ` [Tarantool-patches] [PATCH v2 " Egor Elchinov via Tarantool-patches
  2 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 20:44 UTC (permalink / raw)
  To: gorcunov, alyapunov, skaplun; +Cc: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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.

Needed for: #4002
---
 src/lib/core/backtrace.cc                     | 66 +++++++++++++++++++
 src/lib/core/backtrace.h                      |  6 ++
 src/lib/core/fiber.c                          |  8 +++
 src/lib/core/fiber.h                          | 16 +++++
 src/lua/fiber.c                               |  9 +++
 .../gh-4002-fiber-creation-backtrace.result   | 57 ++++++++++++++++
 .../gh-4002-fiber-creation-backtrace.test.lua | 27 ++++++++
 7 files changed, 189 insertions(+)
 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..276933ad6 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -432,6 +432,72 @@ 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
+fast_trace_collect(void **ip_buf, int limit)
+{
+	memset(ip_buf, 0, limit * sizeof(*ip_buf));
+	unw_backtrace(ip_buf, limit);
+}
+
+/**
+ * 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
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
+{
+	static __thread char proc_name[BACKTRACE_NAME_MAX];
+	int frame_no = 0;
+	unw_word_t ip = 0, offset = 0;
+	unw_proc_info_t pi;
+	int ret = 0;
+	char* proc = NULL;
+
+	unw_accessors_t* acc = unw_get_accessors(unw_local_addr_space);
+	assert(acc);
+
+	for (frame_no = 0; 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 || (frame_no > 0 &&
+		    cb(frame_no - 1, (void *)ip, proc,
+	 	       (size_t)offset, cb_ctx) != 0))
+			break;
+	}
+
+#ifndef TARGET_OS_DARWIN
+	if (ret != 0)
+		say_debug("unwinding error: %s", unw_strerror(ret));
+#else
+	if (ret != 0)
+		say_debug("unwinding error: %i", ret);
+#endif
+}
+
 void
 print_backtrace(void)
 {
diff --git a/src/lib/core/backtrace.h b/src/lib/core/backtrace.h
index e0ae56be4..1560f03e3 100644
--- a/src/lib/core/backtrace.h
+++ b/src/lib/core/backtrace.h
@@ -55,6 +55,12 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx);
 void
 backtrace_proc_cache_clear(void);
 
+void
+fast_trace_collect(void **ip_buf, int limit);
+
+void
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx);
+
 #endif /* ENABLE_BACKTRACE */
 
 #if defined(__cplusplus)
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 759c7da6a..73500e043 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
+	fast_trace_collect(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..6a155c0fe 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..dc7051edd 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -308,6 +308,15 @@ 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);
+		fast_trace_foreach(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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber creation backtrace
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
  2021-07-01 20:44   ` Egor Elchinov via Tarantool-patches
@ 2021-07-02  7:00   ` Egor Elchinov via Tarantool-patches
  2021-07-02  7:11   ` [Tarantool-patches] [PATCH v2 " Egor Elchinov via Tarantool-patches
  2 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-02  7:00 UTC (permalink / raw)
  To: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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.

Needed for: #4002
---
 src/lib/core/backtrace.cc                     | 66 +++++++++++++++++++
 src/lib/core/backtrace.h                      |  6 ++
 src/lib/core/fiber.c                          |  8 +++
 src/lib/core/fiber.h                          | 16 +++++
 src/lua/fiber.c                               |  9 +++
 .../gh-4002-fiber-creation-backtrace.result   | 57 ++++++++++++++++
 .../gh-4002-fiber-creation-backtrace.test.lua | 27 ++++++++
 7 files changed, 189 insertions(+)
 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..276933ad6 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -432,6 +432,72 @@ 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
+fast_trace_collect(void **ip_buf, int limit)
+{
+	memset(ip_buf, 0, limit * sizeof(*ip_buf));
+	unw_backtrace(ip_buf, limit);
+}
+
+/**
+ * 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
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
+{
+	static __thread char proc_name[BACKTRACE_NAME_MAX];
+	int frame_no = 0;
+	unw_word_t ip = 0, offset = 0;
+	unw_proc_info_t pi;
+	int ret = 0;
+	char* proc = NULL;
+
+	unw_accessors_t* acc = unw_get_accessors(unw_local_addr_space);
+	assert(acc);
+
+	for (frame_no = 0; 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 || (frame_no > 0 &&
+		    cb(frame_no - 1, (void *)ip, proc,
+	 	       (size_t)offset, cb_ctx) != 0))
+			break;
+	}
+
+#ifndef TARGET_OS_DARWIN
+	if (ret != 0)
+		say_debug("unwinding error: %s", unw_strerror(ret));
+#else
+	if (ret != 0)
+		say_debug("unwinding error: %i", ret);
+#endif
+}
+
 void
 print_backtrace(void)
 {
diff --git a/src/lib/core/backtrace.h b/src/lib/core/backtrace.h
index e0ae56be4..1560f03e3 100644
--- a/src/lib/core/backtrace.h
+++ b/src/lib/core/backtrace.h
@@ -55,6 +55,12 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx);
 void
 backtrace_proc_cache_clear(void);
 
+void
+fast_trace_collect(void **ip_buf, int limit);
+
+void
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx);
+
 #endif /* ENABLE_BACKTRACE */
 
 #if defined(__cplusplus)
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 759c7da6a..73500e043 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
+	fast_trace_collect(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..6a155c0fe 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..dc7051edd 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -308,6 +308,15 @@ 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);
+		fast_trace_foreach(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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH v2 1/7] fiber: add PoC for fiber creation backtrace
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
  2021-07-01 20:44   ` Egor Elchinov via Tarantool-patches
  2021-07-02  7:00   ` Egor Elchinov via Tarantool-patches
@ 2021-07-02  7:11   ` Egor Elchinov via Tarantool-patches
  2 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-02  7:11 UTC (permalink / raw)
  To: tarantool-patches

From: Egor Elchinov <eelchinov@tarantool.org>

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.

Needed for: #4002
---
 src/lib/core/backtrace.cc                     | 66 +++++++++++++++++++
 src/lib/core/backtrace.h                      |  6 ++
 src/lib/core/fiber.c                          |  8 +++
 src/lib/core/fiber.h                          | 16 +++++
 src/lua/fiber.c                               |  9 +++
 .../gh-4002-fiber-creation-backtrace.result   | 57 ++++++++++++++++
 .../gh-4002-fiber-creation-backtrace.test.lua | 27 ++++++++
 7 files changed, 189 insertions(+)
 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..276933ad6 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -432,6 +432,72 @@ 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
+fast_trace_collect(void **ip_buf, int limit)
+{
+	memset(ip_buf, 0, limit * sizeof(*ip_buf));
+	unw_backtrace(ip_buf, limit);
+}
+
+/**
+ * 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
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
+{
+	static __thread char proc_name[BACKTRACE_NAME_MAX];
+	int frame_no = 0;
+	unw_word_t ip = 0, offset = 0;
+	unw_proc_info_t pi;
+	int ret = 0;
+	char* proc = NULL;
+
+	unw_accessors_t* acc = unw_get_accessors(unw_local_addr_space);
+	assert(acc);
+
+	for (frame_no = 0; 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 || (frame_no > 0 &&
+		    cb(frame_no - 1, (void *)ip, proc,
+	 	       (size_t)offset, cb_ctx) != 0))
+			break;
+	}
+
+#ifndef TARGET_OS_DARWIN
+	if (ret != 0)
+		say_debug("unwinding error: %s", unw_strerror(ret));
+#else
+	if (ret != 0)
+		say_debug("unwinding error: %i", ret);
+#endif
+}
+
 void
 print_backtrace(void)
 {
diff --git a/src/lib/core/backtrace.h b/src/lib/core/backtrace.h
index e0ae56be4..1560f03e3 100644
--- a/src/lib/core/backtrace.h
+++ b/src/lib/core/backtrace.h
@@ -55,6 +55,12 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx);
 void
 backtrace_proc_cache_clear(void);
 
+void
+fast_trace_collect(void **ip_buf, int limit);
+
+void
+fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx);
+
 #endif /* ENABLE_BACKTRACE */
 
 #if defined(__cplusplus)
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 759c7da6a..73500e043 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
+	fast_trace_collect(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..6a155c0fe 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..dc7051edd 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -308,6 +308,15 @@ 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);
+		fast_trace_foreach(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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch Egor Elchinov via Tarantool-patches
@ 2021-07-05  9:42   ` Cyrill Gorcunov via Tarantool-patches
  2021-07-09  9:32     ` Egor Elchinov via Tarantool-patches
  0 siblings, 1 reply; 17+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-07-05  9:42 UTC (permalink / raw)
  To: Egor Elchinov via Tarantool-patches

On Thu, Jul 01, 2021 at 11:24:41PM +0300, Egor Elchinov via Tarantool-patches wrote:
> From: Egor Elchinov <eelchinov@tarantool.org>
> 
> From: Egor Elchinov <elchinov.es@gmail.com>
> 
> Some mistakes fixed.
> More consistent naming provided.
> Based on review and patch from
> Cyrill Gorcunov <gorcunov@gmail.com>
> 
> Needed for: #4002

I think you may simply merge first 3 paches into one instead.
And please use versioning in email subjects, for example here
we could have "[PATCH v2 3/7]". Otherwise looks ok.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for parent backtrace
  2021-07-01 20:24 ` [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for " Egor Elchinov via Tarantool-patches
@ 2021-07-05  9:49   ` Cyrill Gorcunov via Tarantool-patches
  2021-07-09  9:50     ` Egor Elchinov via Tarantool-patches
  0 siblings, 1 reply; 17+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-07-05  9:49 UTC (permalink / raw)
  To: Egor Elchinov via Tarantool-patches

On Thu, Jul 01, 2021 at 11:24:43PM +0300, Egor Elchinov via Tarantool-patches wrote:
> diff --git a/src/lua/fiber.c b/src/lua/fiber.c
> index 026e30bc6..fe01ae23b 100644
> --- a/src/lua/fiber.c
> +++ b/src/lua/fiber.c
> @@ -205,7 +205,7 @@ struct lua_parent_tb_ctx {
>  	int tb_frame;
>  };
>  
> -#ifdef ENABLE_BACKTRACE
> +#if ENABLE_BACKTRACE

Please don't squash unrelated changes into the patch. If you prefer
to shange #ifdef to #if better make it as a separate commit (though
I don't understand yet what is wrong with existing code).

> @@ -446,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);
>  
> -		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);
> +		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);
> +		}

Maybe it would worth to merge fiber_parent_bt_is_enabled option immediately
into previous patch?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch
  2021-07-05  9:42   ` Cyrill Gorcunov via Tarantool-patches
@ 2021-07-09  9:32     ` Egor Elchinov via Tarantool-patches
  0 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-09  9:32 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tarantool-patches

On 05.07.2021 12:42, Cyrill Gorcunov via Tarantool-patches wrote:
> On Thu, Jul 01, 2021 at 11:24:41PM +0300, Egor Elchinov via Tarantool-patches wrote:
> 
> I think you may simply merge first 3 paches into one instead.
> And please use versioning in email subjects, for example here
> we could have "[PATCH v2 3/7]". Otherwise looks ok.
> 
Thanks, squashed.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for parent backtrace
  2021-07-05  9:49   ` Cyrill Gorcunov via Tarantool-patches
@ 2021-07-09  9:50     ` Egor Elchinov via Tarantool-patches
  2021-07-09 10:10       ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 1 reply; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-09  9:50 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tarantool-patches

Thanks for the review!

On 05.07.2021 12:49, Cyrill Gorcunov via Tarantool-patches wrote:
> On Thu, Jul 01, 2021 at 11:24:43PM +0300, Egor Elchinov via Tarantool-patches wrote:
>> diff --git a/src/lua/fiber.c b/src/lua/fiber.c
>> index 026e30bc6..fe01ae23b 100644
>> --- a/src/lua/fiber.c
>> +++ b/src/lua/fiber.c
>> @@ -205,7 +205,7 @@ struct lua_parent_tb_ctx {
>>   	int tb_frame;
>>   };
>>   
>> -#ifdef ENABLE_BACKTRACE
>> +#if ENABLE_BACKTRACE
> 
> Please don't squash unrelated changes into the patch. If you prefer
> to shange #ifdef to #if better make it as a separate commit (though
> I don't understand yet what is wrong with existing code)
Changes rebased into refactoring commit.

I believe this is more conventional way to check the ENABLE_BACKTRACE 
option because it is defined as
#cmakedefine ENABLE_BACKTRACE 1
but not
#cmakedefine ENABLE_BACKTRACE

> 
>> @@ -446,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);
>>   
>> -		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);
>> +		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);
>> +		}
> 
> Maybe it would worth to merge fiber_parent_bt_is_enabled option immediately
> into previous patch?
> 
Merged.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for parent backtrace
  2021-07-09  9:50     ` Egor Elchinov via Tarantool-patches
@ 2021-07-09 10:10       ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 0 replies; 17+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-07-09 10:10 UTC (permalink / raw)
  To: Egor Elchinov; +Cc: tarantool-patches

On Fri, Jul 09, 2021 at 12:50:40PM +0300, Egor Elchinov wrote:
> > > -#ifdef ENABLE_BACKTRACE
> > > +#if ENABLE_BACKTRACE
> > 
> > Please don't squash unrelated changes into the patch. If you prefer
> > to shange #ifdef to #if better make it as a separate commit (though
> > I don't understand yet what is wrong with existing code)
> Changes rebased into refactoring commit.
> 
> I believe this is more conventional way to check the ENABLE_BACKTRACE option
> because it is defined as
> #cmakedefine ENABLE_BACKTRACE 1
> but not
> #cmakedefine ENABLE_BACKTRACE

For this particular case we don't check the _value_ of defined variable,
we rather interested if it is defined at all. In general for conditional compilation
we use #ifdef you know, and all our current code uses #ifdef ENABLE_BACKTRACE
so I personally don't see much point in renaming statement. Still,
up to you, but if you prefer #if form could you please make it as a separate
patch.

	Cyrill

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch
  2021-07-01 15:54 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
@ 2021-07-01 15:54 ` Egor Elchinov via Tarantool-patches
  0 siblings, 0 replies; 17+ messages in thread
From: Egor Elchinov via Tarantool-patches @ 2021-07-01 15:54 UTC (permalink / raw)
  To: tarantool-patches; +Cc: gorcunov

From: Egor Elchinov <eelchinov@tarantool.org>

Some mistakes fixed.
More consistent naming provided.
Based on review and patch from
Cyrill Gorcunov <gorcunov@gmail.com>

Needed for: #4002
---
 src/lib/core/backtrace.cc | 47 +++++++++++++++++++++------------------
 src/lib/core/backtrace.h  |  8 ++++---
 src/lib/core/fiber.c      |  2 +-
 src/lib/core/fiber.h      |  2 +-
 src/lua/fiber.c           |  5 +++--
 5 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/src/lib/core/backtrace.cc b/src/lib/core/backtrace.cc
index a1727374c..f97c4c0a9 100644
--- a/src/lib/core/backtrace.cc
+++ b/src/lib/core/backtrace.cc
@@ -443,8 +443,8 @@ out:
  * Must be by far faster than usual backtrace according to the
  * libunwind doc for unw_backtrace().
  */
-void
-fast_trace_collect(void **ip_buf, int limit)
+void NOINLINE
+backtrace_collect_ip(void **ip_buf, int limit)
 {
 	memset(ip_buf, 0, limit * sizeof(*ip_buf));
 #ifndef TARGET_OS_DARWIN
@@ -466,54 +466,57 @@ fast_trace_collect(void **ip_buf, int limit)
  * TODO: to add cache and demangling support
  */
 void
-fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx)
+backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
+		     void *cb_ctx)
 {
 #ifndef TARGET_OS_DARWIN
-	static __thread char proc_name[BACKTRACE_NAME_MAX];
-	char* proc = NULL;
-	int frame_no = 0, ret = 0;
+	char proc_name[BACKTRACE_NAME_MAX];
 	unw_word_t ip = 0, offset = 0;
 	unw_proc_info_t pi;
-
-	unw_accessors_t* acc = unw_get_accessors(unw_local_addr_space);
-	assert(acc);
-
-	for (frame_no = 0; frame_no < limit && ip_buf[frame_no] != NULL;
-	     ++frame_no) {
+	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_name, sizeof(proc_name),
+						 &offset, NULL);
 			proc = proc_name;
 		}
-
-		if (ret != 0 || (frame_no > 0 &&
-		    cb(frame_no - 1, (void *)ip, proc,
-	 	       (size_t)offset, cb_ctx) != 0))
+		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 = 0, ret = 1;
+	int frame_no, ret = 1;
 	void *ip = NULL;
 	size_t offset = 0;
 	Dl_info dli;
 
-	for (frame_no = 0; frame_no < limit && ip_buf[frame_no] != 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 (frame_no > 0 &&
-		    cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
+		if (cb(frame_no - 1, ip, dli.dli_sname, offset, cb_ctx) != 0)
 			break;
 	}
 
diff --git a/src/lib/core/backtrace.h b/src/lib/core/backtrace.h
index 8bb668366..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)
@@ -55,11 +56,12 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx);
 void
 backtrace_proc_cache_clear(void);
 
-void
-fast_trace_collect(void **ip_buf, int limit);
+void NOINLINE
+backtrace_collect_ip(void **ip_buf, int limit);
 
 void
-fast_trace_foreach(backtrace_cb cb, void **ip_buf, int limit, void *cb_ctx);
+backtrace_foreach_ip(backtrace_cb cb, void **ip_buf, int limit,
+		     void *cb_ctx);
 
 #endif /* ENABLE_BACKTRACE */
 
diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 73500e043..924ff3c82 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -1265,7 +1265,7 @@ 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
-	fast_trace_collect(fiber->parent_bt_ip_buf, FIBER_PARENT_BT_MAX);
+	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 6a155c0fe..beed58866 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -654,7 +654,7 @@ struct fiber {
 	char inline_name[FIBER_NAME_INLINE];
 #if ENABLE_BACKTRACE
 	/** Fiber creation backtrace chunk. */
-	void* parent_bt_ip_buf[FIBER_PARENT_BT_MAX];
+	void *parent_bt_ip_buf[FIBER_PARENT_BT_MAX];
 #endif /* ENABLE_BACKTRACE */
 };
 
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index dc7051edd..7b21361d4 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -314,8 +314,9 @@ lbox_fiber_statof_map(struct fiber *f, void *cb_ctx, bool backtrace)
 		tb_ctx.R = NULL;
 		lua_pushstring(L, "backtrace_parent");
 		lua_newtable(L);
-		fast_trace_foreach(fiber_backtrace_cb, f->parent_bt_ip_buf,
-		     		   FIBER_PARENT_BT_MAX, &tb_ctx);
+		backtrace_foreach_ip(fiber_backtrace_cb,
+				     f->parent_bt_ip_buf,
+				     FIBER_PARENT_BT_MAX, &tb_ctx);
 		lua_settable(L, -3);
 #endif /* ENABLE_BACKTRACE */
 	}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-07-09 10:10 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-01 20:24 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
2021-07-01 20:24 ` [Tarantool-patches] [PATCH 1/7] fiber: add PoC for fiber " Egor Elchinov via Tarantool-patches
2021-07-01 20:44   ` Egor Elchinov via Tarantool-patches
2021-07-02  7:00   ` Egor Elchinov via Tarantool-patches
2021-07-02  7:11   ` [Tarantool-patches] [PATCH v2 " Egor Elchinov via Tarantool-patches
2021-07-01 20:24 ` [Tarantool-patches] [PATCH 2/7] fiber: fix DARWIN build Egor Elchinov via Tarantool-patches
2021-07-01 20:24 ` [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch Egor Elchinov via Tarantool-patches
2021-07-05  9:42   ` Cyrill Gorcunov via Tarantool-patches
2021-07-09  9:32     ` Egor Elchinov via Tarantool-patches
2021-07-01 20:24 ` [Tarantool-patches] [PATCH 4/7] fiber: add PoC for Lua parent backtrace Egor Elchinov via Tarantool-patches
2021-07-01 20:24 ` [Tarantool-patches] [PATCH 5/7] fiber: add dynamic option for " Egor Elchinov via Tarantool-patches
2021-07-05  9:49   ` Cyrill Gorcunov via Tarantool-patches
2021-07-09  9:50     ` Egor Elchinov via Tarantool-patches
2021-07-09 10:10       ` Cyrill Gorcunov via Tarantool-patches
2021-07-01 20:24 ` [Tarantool-patches] [PATCH 6/7] fiber: refactor lua backtrace routine Egor Elchinov via Tarantool-patches
2021-07-01 20:24 ` [Tarantool-patches] [PATCH 7/7] fiber: refactor C backtrace and add changelog Egor Elchinov via Tarantool-patches
  -- strict thread matches above, loose matches on Subject: below --
2021-07-01 15:54 [Tarantool-patches] [PATCH 0/7] fiber: introduce creation backtrace Egor Elchinov via Tarantool-patches
2021-07-01 15:54 ` [Tarantool-patches] [PATCH 3/7] fiber: apply fix patch Egor Elchinov via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox