From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Mon, 30 Sep 2019 20:00:37 +0000 (+0300)
Subject: lib: backtrace_get/append() - skip stack frames only based name
X-Git-Tag: 2.3.9~107
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cd8782bb7b073d80bc871927e4eb43ddf2ad988e;p=thirdparty%2Fdovecot%2Fcore.git

lib: backtrace_get/append() - skip stack frames only based name

It's not really safe to do anything else because functions may potentially
be inlined.
---

diff --git a/src/lib/backtrace-string.c b/src/lib/backtrace-string.c
index d9845e7a17..eb87fc3816 100644
--- a/src/lib/backtrace-string.c
+++ b/src/lib/backtrace-string.c
@@ -5,7 +5,7 @@
 #include "backtrace-string.h"
 
 #define MAX_STACK_SIZE 30
-#define STACK_SKIP_COUNT 2
+#define BACKTRACE_SKIP_PREFIX "backtrace_"
 
 #if defined(HAVE_LIBUNWIND)
 
@@ -13,6 +13,7 @@
 
 int backtrace_append(string_t *str)
 {
+	size_t str_orig_size = str_len(str);
 	char proc_name[256];
 	int ret;
 	unsigned int fp = 0;
@@ -31,11 +32,7 @@ int backtrace_append(string_t *str)
 	}
 
 	do {
-		if (fp < STACK_SKIP_COUNT) {
-			fp++;
-			continue;
-		}
-		str_printfa(str, "#%d ", fp - STACK_SKIP_COUNT);
+		str_printfa(str, "#%d ", fp);
 		if ((ret = unw_get_proc_info(&c, &pip)) != 0) {
 			str_printfa(str, "[unw_get_proc_info_failed(): %d]", ret);
 		} else if (pip.start_ip == 0 || pip.end_ip == 0) {
@@ -43,6 +40,9 @@ int backtrace_append(string_t *str)
 		} else if ((ret = unw_get_proc_name(&c, proc_name, sizeof(proc_name), 0)) != 0 &&
 			   ret != UNW_ENOMEM) {
 			str_printfa(str, "[unw_get_proc_name() failed: %d]", ret);
+		} else if (!success && str_begins(proc_name, BACKTRACE_SKIP_PREFIX)) {
+			str_truncate(str, str_orig_size);
+			continue;
 		} else {
 			str_append_max(str, proc_name, sizeof(proc_name));
 			str_printfa(str, "[0x%08lx]", pip.start_ip);
@@ -64,25 +64,26 @@ int backtrace_append(string_t *str)
 
 int backtrace_append(string_t *str)
 {
+	size_t str_orig_size = str_len(str);
 	void *stack[MAX_STACK_SIZE];
 	char **strings;
 	int ret, i;
 
 	ret = backtrace(stack, N_ELEMENTS(stack));
-	if (ret <= STACK_SKIP_COUNT)
+	if (ret <= 0)
 		return -1;
 
 	strings = backtrace_symbols(stack, ret);
-	for (i = STACK_SKIP_COUNT; i < ret; i++) {
-		if (i > STACK_SKIP_COUNT)
+	for (i = 0; i < ret; i++) {
+		if (str_len(str) > str_orig_size)
 			str_append(str, " -> ");
 
-		if (strings != NULL)
-			str_append(str, strings[i]);
-		else {
+		if (strings == NULL) {
 			/* out of memory case */
 			str_printfa(str, "0x%p", stack[i]);
-		}
+		} else if (str_len(str) != str_orig_size ||
+			   !str_begins(strings[i], BACKTRACE_SKIP_PREFIX))
+			str_append(str, strings[i]);
 	}
 	free(strings);
 	return 0;
@@ -101,11 +102,9 @@ static int walk_callback(uintptr_t ptr, int signo ATTR_UNUSED,
 {
 	struct walk_context *ctx = context;
 
-	if (ctx->pos >= STACK_SKIP_COUNT) {
-		if (ctx->pos > STACK_SKIP_COUNT)
-			str_append(ctx->str, " -> ");
-		str_printfa(ctx->str, "0x%p", (void *)ptr);
-	}
+	if (ctx->pos > 0)
+		str_append(ctx->str, " -> ");
+	str_printfa(ctx->str, "0x%p", (void *)ptr);
 	ctx->pos++;
 	return 0;
 }
diff --git a/src/lib/test-backtrace.c b/src/lib/test-backtrace.c
index c6e35495d1..fdebe0fd38 100644
--- a/src/lib/test-backtrace.c
+++ b/src/lib/test-backtrace.c
@@ -1,13 +1,40 @@
 #include "test-lib.h"
+#include "str.h"
 #include "backtrace-string.h"
 
+static void test_backtrace_append(void)
+{
+	test_begin("backtrace_append");
+	string_t *bt = t_str_new(128);
+#if (defined(HAVE_LIBUNWIND))
+        test_assert(backtrace_append(bt) == 0);
+	/* Check that there's a usable function in the backtrace.
+	   Note that this function may be inlined, so don't check for
+	   test_backtrace_get() */
+	test_assert(strstr(str_c(bt), "test_backtrace") != NULL);
+	/* make sure the backtrace_append is not */
+	test_assert(strstr(str_c(bt), " backtrace_append") == NULL);
+#elif (defined(HAVE_BACKTRACE_SYMBOLS) && defined(HAVE_EXECINFO_H)) || \
+      (defined(HAVE_WALKCONTEXT) && defined(HAVE_UCONTEXT_H))
+	test_assert(backtrace_append(bt) == 0);
+	/* it should have some kind of main in it */
+	test_assert(strstr(str_c(bt), "main") != NULL);
+#else
+	/* should not work in this context */
+	test_assert(backtrace_append(bt) == -1);
+#endif
+	test_end();
+}
+
 static void test_backtrace_get(void)
 {
 	test_begin("backtrace_get");
 	const char *bt = NULL;
 #if (defined(HAVE_LIBUNWIND))
         test_assert(backtrace_get(&bt) == 0);
-	/* check that this function is there */
+	/* Check that there's a usable function in the backtrace.
+	   Note that this function may be inlined, so don't check for
+	   test_backtrace_get() */
         test_assert(strstr(bt, "test_backtrace") != NULL);
 	/* make sure the backtrace_get is not */
 	test_assert(strstr(bt, " backtrace_get") == NULL);
@@ -25,5 +52,6 @@ static void test_backtrace_get(void)
 
 void test_backtrace(void)
 {
+	test_backtrace_append();
 	test_backtrace_get();
 }