backtrace: Use a static buffer in backtrace_ctx for symbols

Replace the dynamic allocation in os_backtrace_symbols() with a static
buffer embedded in struct backtrace_ctx. This avoids malloc recursion
when backtrace is called from within dlmalloc (e.g., for the upcoming
mcheck caller-tracking).

The API gets a complete rework as part of this:

- Combine addrs[] and syms[] arrays into struct backtrace_frame with
  addr and sym fields
- Store the strings in a unified buffer, with pointers from an array
- Change os_backtrace_symbols() to take ctx pointer and fill sym_buf
- Remove os_backtrace_symbols_free() as nothing needs freeing
- Rename BACKTRACE_MAX to BACKTRACE_MAX_FRAMES

Co-developed-by: Claude <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>

arch/sandbox/cpu/backtrace.c

@@ -16,6 +16,8 @@
 #include <string.h>
 
 #include <os.h>
+/* For BACKTRACE_MAX_FRAMES - include U-Boot's header after system headers */
+#include "../../../include/backtrace.h"
 
 /* libbacktrace state - created once and cached */
 static struct backtrace_state *bt_state;
@@ -77,46 +79,41 @@ static int bt_full_callback(void *data, uintptr_t pc, const char *fname,
 	return 0;  /* continue to get innermost frame for inlined functions */
 }
 
-char **os_backtrace_symbols(void *const *buffer, uint count)
+void os_backtrace_symbols(struct backtrace_ctx *ctx)
 {
+	char *end = ctx->sym_buf + BACKTRACE_SYM_BUFSZ;
 	struct backtrace_state *state;
-	char *str_storage;
-	char **strings;
-	uint i;
+	char *p = ctx->sym_buf;
+	int remaining, i;
 
 	state = get_bt_state();
 
-	/* Allocate array of string pointers plus space for strings */
-	strings = malloc(count * sizeof(char *) + count * 256);
-	if (!strings)
-		return NULL;
+	for (i = 0; i < ctx->count; i++) {
+		struct backtrace_frame *frame = &ctx->frame[i];
+		struct bt_sym_ctx sym_ctx;
 
-	/* String storage starts after the pointer array */
-	str_storage = (char *)(strings + count);
+		remaining = end - p;
+		if (remaining <= 1) {
+			/* No more space, leave remaining syms as NULL */
+			frame->sym = NULL;
+			continue;
+		}
 
-	for (i = 0; i < count; i++) {
-		struct bt_sym_ctx ctx;
-
-		strings[i] = str_storage + i * 256;
-		ctx.buf = strings[i];
-		ctx.size = 256;
-		ctx.found = 0;
+		frame->sym = p;
+		sym_ctx.buf = p;
+		sym_ctx.size = remaining;
+		sym_ctx.found = 0;
 
 		if (state) {
-			backtrace_pcinfo(state, (uintptr_t)buffer[i],
+			backtrace_pcinfo(state, (uintptr_t)frame->addr,
 					 bt_full_callback, bt_error_callback,
-					 &ctx);
+					 &sym_ctx);
 		}
 
 		/* Fall back to address if no symbol found */
-		if (!ctx.found)
-			snprintf(strings[i], 256, "%p", buffer[i]);
+		if (!sym_ctx.found)
+			snprintf(p, remaining, "%p", frame->addr);
+
+		p += strlen(p) + 1;
 	}
-
-	return strings;
-}
-
-void os_backtrace_symbols_free(char **strings)
-{
-	free(strings);
 }

arch/sandbox/lib/backtrace.c

@@ -13,54 +13,23 @@
 
 int backtrace_init(struct backtrace_ctx *ctx, uint skip)
 {
+	void *addrs[BACKTRACE_MAX_FRAMES];
 	uint i;
 
-	for (i = 0; i < BACKTRACE_MAX; i++)
-		ctx->syms[i] = NULL;
 	/* +1 to skip this function */
-	ctx->count = os_backtrace(ctx->addrs, BACKTRACE_MAX, skip + 1);
+	ctx->count = os_backtrace(addrs, BACKTRACE_MAX_FRAMES, skip + 1);
+
+	for (i = 0; i < ctx->count; i++) {
+		ctx->frame[i].addr = addrs[i];
+		ctx->frame[i].sym = NULL;
+	}
 
 	return ctx->count;
 }
 
 int backtrace_get_syms(struct backtrace_ctx *ctx, char *buf, int size)
 {
-	char **raw_syms;
-	size_t total_len;
-	char *p;
-	uint i;
-
-	raw_syms = os_backtrace_symbols(ctx->addrs, ctx->count);
-	if (!raw_syms)
-		return -ENOMEM;
-
-	/* Calculate total buffer size needed */
-	total_len = 0;
-	for (i = 0; i < ctx->count; i++) {
-		if (raw_syms[i])
-			total_len += strlen(raw_syms[i]) + 1;
-		else
-			total_len += 1;  /* empty string */
-	}
-
-	if ((size_t)size < total_len) {
-		os_backtrace_symbols_free(raw_syms);
-		return -ENOSPC;
-	}
-
-	/* Copy strings into buffer */
-	p = buf;
-	for (i = 0; i < ctx->count; i++) {
-		ctx->syms[i] = p;
-		if (raw_syms[i]) {
-			strcpy(p, raw_syms[i]);
-			p += strlen(raw_syms[i]) + 1;
-		} else {
-			*p++ = '\0';
-		}
-	}
-
-	os_backtrace_symbols_free(raw_syms);
+	os_backtrace_symbols(ctx);
 
 	return 0;
 }

include/backtrace.h

@@ -9,21 +9,46 @@
 #ifndef __BACKTRACE_H
 #define __BACKTRACE_H
 
-#define BACKTRACE_MAX		100
-#define BACKTRACE_SYM_SIZE	128
-#define BACKTRACE_BUFSZ		(BACKTRACE_MAX * BACKTRACE_SYM_SIZE)
+/* Maximum number of stack frames that can be collected */
+#define BACKTRACE_MAX_FRAMES	100
+
+/* Size of buffer for all symbol strings combined */
+#define BACKTRACE_SYM_BUFSZ	(4 * 1024)
+
+/**
+ * struct backtrace_frame - a single stack frame in a backtrace
+ *
+ * @addr: return address for this frame
+ * @sym: pointer to symbol string in backtrace_ctx->sym_buf, or NULL if not
+ *	yet resolved or if sym_buf ran out of space
+ */
+struct backtrace_frame {
+	void *addr;
+	char *sym;
+};
 
 /**
  * struct backtrace_ctx - context for backtrace operations
  *
- * @addrs: array of return addresses
- * @syms: array of symbol strings (NULL until backtrace_get_syms() called)
- * @count: number of entries in addrs/syms arrays
+ * This structure holds all state for collecting and symbolising a backtrace.
+ * It should be declared static to avoid consuming stack space (~5KB).
+ *
+ * Lifecycle:
+ *   1. Call backtrace_init() - fills @frame[].addr with return addresses and
+ *      sets @count. The @frame[].sym pointers are initialised to NULL.
+ *   2. Call backtrace_get_syms() - resolves addresses to symbol strings,
+ *      writing them into @sym_buf and setting @frame[].sym pointers.
+ *   3. Access @frame[0..count-1] to read addresses and symbol strings.
+ *   4. Call backtrace_uninit() to release resources (currently a no-op).
+ *
+ * @frame: array of stack frames
+ * @count: number of valid entries in @frame
+ * @sym_buf: buffer holding NUL-terminated symbol strings packed consecutively
  */
 struct backtrace_ctx {
-	void *addrs[BACKTRACE_MAX];
-	char *syms[BACKTRACE_MAX];
+	struct backtrace_frame frame[BACKTRACE_MAX_FRAMES];
 	unsigned int count;
+	char sym_buf[BACKTRACE_SYM_BUFSZ];
 };
 
 /**

include/os.h

@@ -588,27 +588,17 @@ void os_signal_action(int sig, unsigned long pc);
  */
 uint os_backtrace(void **buffer, uint size, uint skip);
 
+struct backtrace_ctx;
+
 /**
  * os_backtrace_symbols() - convert addresses to symbol strings
  *
- * Convert backtrace addresses to human-readable symbol strings. The returned
- * array and strings are allocated with malloc() and must be freed with
- * os_backtrace_symbols_free().
+ * Convert backtrace addresses to human-readable symbol strings. The symbol
+ * strings are written into ctx->sym_buf and ctx->syms pointers are set up.
  *
- * @buffer: array of addresses from os_backtrace()
- * @count: number of addresses in buffer
- * Return: array of symbol strings, or NULL on error
+ * @ctx: backtrace context with addrs and count already filled in
  */
-char **os_backtrace_symbols(void *const *buffer, uint count);
-
-/**
- * os_backtrace_symbols_free() - free symbol strings
- *
- * Free the array returned by os_backtrace_symbols().
- *
- * @strings: array to free (may be NULL)
- */
-void os_backtrace_symbols_free(char **strings);
+void os_backtrace_symbols(struct backtrace_ctx *ctx);
 
 /**
  * os_get_time_offset() - get time offset

lib/backtrace.c

@@ -26,8 +26,7 @@ static void print_sym(const char *sym)
 
 int backtrace_show(void)
 {
-	char buf[BACKTRACE_BUFSZ];
-	struct backtrace_ctx ctx;
+	static struct backtrace_ctx ctx;
 	uint i;
 	int ret;
 
@@ -35,7 +34,7 @@ int backtrace_show(void)
 	if (ret < 0)
 		return ret;
 
-	ret = backtrace_get_syms(&ctx, buf, sizeof(buf));
+	ret = backtrace_get_syms(&ctx, NULL, 0);
 	if (ret) {
 		backtrace_uninit(&ctx);
 		return ret;
@@ -43,10 +42,12 @@ int backtrace_show(void)
 
 	printf("backtrace: %d addresses\n", ctx.count);
 	for (i = 0; i < ctx.count; i++) {
-		if (ctx.syms[i])
-			print_sym(ctx.syms[i]);
+		const struct backtrace_frame *frame = &ctx.frame[i];
+
+		if (frame->sym)
+			print_sym(frame->sym);
 		else
-			printf("  %p\n", ctx.addrs[i]);
+			printf("  %p\n", frame->addr);
 	}
 
 	backtrace_uninit(&ctx);

test/lib/backtrace.c

@@ -15,24 +15,25 @@
 /* Test backtrace_init() and backtrace_get_syms() */
 static int lib_test_backtrace(struct unit_test_state *uts)
 {
-	char buf[BACKTRACE_BUFSZ];
-	struct backtrace_ctx ctx;
+	static struct backtrace_ctx ctx;
 	bool found_self = false;
 	bool found_ut_run_list = false;
 	uint i;
 
 	ut_assert(backtrace_init(&ctx, 0) > 2);
-	ut_assertok(backtrace_get_syms(&ctx, buf, sizeof(buf)));
+	ut_assertok(backtrace_get_syms(&ctx, NULL, 0));
 
 	/*
 	 * Check for known functions in the call stack. With libbacktrace
 	 * we can find static functions too, so check for this test function.
 	 */
 	for (i = 0; i < ctx.count; i++) {
-		if (ctx.syms[i]) {
-			if (strstr(ctx.syms[i], "lib_test_backtrace"))
+		const struct backtrace_frame *frame = &ctx.frame[i];
+
+		if (frame->sym) {
+			if (strstr(frame->sym, "lib_test_backtrace"))
 				found_self = true;
-			if (strstr(ctx.syms[i], "ut_run_list"))
+			if (strstr(frame->sym, "ut_run_list"))
 				found_ut_run_list = true;
 		}
 	}