From patchwork Wed Dec 10 00:07:03 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 869 Return-Path: X-Original-To: u-boot-concept@u-boot.org Delivered-To: u-boot-concept@u-boot.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765325340; bh=7g6Wws3ziqELN5NwO+gFWmD9PyEARvaDNLxP+fG/bZY=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=UZlRiYycgirPt1oaHewb2sBmxu+m2iu+vurALh8at1uN2emrsYpfk86WilcbdxvZH ZG2R+81Hpj70XB16ihu7Dn18deA+/dF8vc95TdEH6bvUCbloT0gZoODvOFKOo6NMRd cQqMQrhf0irMZb4QLTJFi+3aWpgftCqaFLXMdK+C1zMFTku3HMrx9FlTCVcB0E+mhA hXe8WubPpEDeKXDMkNmFJBtwlyD95fpoXlKKNabDLZbijuHK1qw10N6jmZD1OJCr1a W6OcSYzUNtccl0U/KTHxXd0M2Ya0Vwgh3zOFz51EVzz/nGw7+SAQYyfglIGPZwTdP8 YpPJOXYmSzjRw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 235BF689EC for ; Tue, 9 Dec 2025 17:09:00 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10024) with ESMTP id i1mAxulFPYDX for ; Tue, 9 Dec 2025 17:09:00 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765325340; bh=7g6Wws3ziqELN5NwO+gFWmD9PyEARvaDNLxP+fG/bZY=; h=From:To:Date:In-Reply-To:References:CC:Subject:List-Id: List-Archive:List-Help:List-Owner:List-Post:List-Subscribe: List-Unsubscribe:From; b=UZlRiYycgirPt1oaHewb2sBmxu+m2iu+vurALh8at1uN2emrsYpfk86WilcbdxvZH ZG2R+81Hpj70XB16ihu7Dn18deA+/dF8vc95TdEH6bvUCbloT0gZoODvOFKOo6NMRd cQqMQrhf0irMZb4QLTJFi+3aWpgftCqaFLXMdK+C1zMFTku3HMrx9FlTCVcB0E+mhA hXe8WubPpEDeKXDMkNmFJBtwlyD95fpoXlKKNabDLZbijuHK1qw10N6jmZD1OJCr1a W6OcSYzUNtccl0U/KTHxXd0M2Ya0Vwgh3zOFz51EVzz/nGw7+SAQYyfglIGPZwTdP8 YpPJOXYmSzjRw== Received: from mail.u-boot.org (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 11EE168972 for ; Tue, 9 Dec 2025 17:09:00 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765325338; bh=0Z4WjKl2yGaDBMylUab79zeTXpas7D1wROEoNDvSfVE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SEXNTAkBKXxmWx+hD3T7DtOrbHw+jT4spAImQwSnKJ33PAkVDMyfl6DmRhaySQSa1 5qf5ouuzKvi73q0L8u+1kUQG+kueLO1PRX6ttk86DGt8d7iaX9U6KVt9i19ZN76Y1G HlKOi8tTbgLMH6TK0yr4ldWC0K+73hHmonDqmCsDo47t9RBkdM8FDuvHevutqptmGm XAymvJMCcSv/rw9Up4nlv+tsqJCS20CI2v8eB5L1JlwptROXkFOCPVzj6sXPuoGrdm AWaeIki0nZ9gs6PQ5CL82mbVZa8F/x6JHQMNx14HABaE3S1CLd4d745Ps7E6UtGWcf jSf8PbF/M5Gqw== Received: from localhost (localhost [127.0.0.1]) by mail.u-boot.org (Postfix) with ESMTP id 7429C68957; Tue, 9 Dec 2025 17:08:58 -0700 (MST) X-Virus-Scanned: Debian amavis at Received: from mail.u-boot.org ([127.0.0.1]) by localhost (mail.u-boot.org [127.0.0.1]) (amavis, port 10026) with ESMTP id tWWMo4R2Dx4W; Tue, 9 Dec 2025 17:08:58 -0700 (MST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=u-boot.org; s=default; t=1765325331; bh=0mfHZI35VEowy+WkJ5yV1YmqvYvP9DirA1lGdewBEfk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=wTB1mzvm5C3AfFjEgu+pR/D2Vi0ciVg3JCVpH2xXyIIhTHeaXBjcW4ZBZCCa2j2At 4CwfN8SjaxLs/ppi0nAIZXTif0fe/Z7Kw0E6TACToD/p8bltDAOUkkqpMrTUt7ZV0U cJeFZDoyrHy0C5SbjXUwJ3eZl5n2p4qorhjp9TQFAvcnhZQmiuWB8yom0rcCKDANAU hR7c3Ca/+OtrctepPCjSMicueRxjF2BxpCLTL/A5GiWkqOH8ltGTNe/g5++KxrbJ1I fN5LCbyy4PTM3ej8Ca/prSK4Dg0VKUL2xRQQOXUJWw1u0Wou2qDEWPc5t0+/F0bpTY ILFGbfC++fsDg== Received: from u-boot.org (unknown [73.34.74.121]) by mail.u-boot.org (Postfix) with ESMTPSA id 2C3FB689EC; Tue, 9 Dec 2025 17:08:51 -0700 (MST) From: Simon Glass To: U-Boot Concept Date: Tue, 9 Dec 2025 17:07:03 -0700 Message-ID: <20251210000737.180797-13-sjg@u-boot.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20251210000737.180797-1-sjg@u-boot.org> References: <20251210000737.180797-1-sjg@u-boot.org> MIME-Version: 1.0 Message-ID-Hash: N7RL5WWAWZGCEGKHYP5EAEWMMGMJ4CG4 X-Message-ID-Hash: N7RL5WWAWZGCEGKHYP5EAEWMMGMJ4CG4 X-MailFrom: sjg@u-boot.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Heinrich Schuchardt , Simon Glass , Claude X-Mailman-Version: 3.3.10 Precedence: list Subject: [Concept] [PATCH 12/35] backtrace: Use a static buffer in backtrace_ctx for symbols List-Id: Discussion and patches related to U-Boot Concept Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Simon Glass 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 Signed-off-by: Simon Glass --- arch/sandbox/cpu/backtrace.c | 53 +++++++++++++++++------------------- arch/sandbox/lib/backtrace.c | 47 ++++++-------------------------- include/backtrace.h | 41 ++++++++++++++++++++++------ include/os.h | 22 ++++----------- lib/backtrace.c | 13 +++++---- test/lib/backtrace.c | 13 +++++---- 6 files changed, 86 insertions(+), 103 deletions(-) diff --git a/arch/sandbox/cpu/backtrace.c b/arch/sandbox/cpu/backtrace.c index 1f5a14ed541..2d0a0113cc0 100644 --- a/arch/sandbox/cpu/backtrace.c +++ b/arch/sandbox/cpu/backtrace.c @@ -16,6 +16,8 @@ #include #include +/* 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); - - for (i = 0; i < count; i++) { - struct bt_sym_ctx ctx; + remaining = end - p; + if (remaining <= 1) { + /* No more space, leave remaining syms as NULL */ + frame->sym = NULL; + continue; + } - 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); - return strings; -} - -void os_backtrace_symbols_free(char **strings) -{ - free(strings); + p += strlen(p) + 1; + } } diff --git a/arch/sandbox/lib/backtrace.c b/arch/sandbox/lib/backtrace.c index 073eb945622..62a1c9ee028 100644 --- a/arch/sandbox/lib/backtrace.c +++ b/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; } diff --git a/include/backtrace.h b/include/backtrace.h index eece61e4d9a..7bb2ba68bec 100644 --- a/include/backtrace.h +++ b/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]; }; /** diff --git a/include/os.h b/include/os.h index ab4710fc265..3ea88230af3 100644 --- a/include/os.h +++ b/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); -/** - * 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(). - * - * @buffer: array of addresses from os_backtrace() - * @count: number of addresses in buffer - * Return: array of symbol strings, or NULL on error - */ -char **os_backtrace_symbols(void *const *buffer, uint count); +struct backtrace_ctx; /** - * os_backtrace_symbols_free() - free symbol strings + * os_backtrace_symbols() - convert addresses to symbol strings * - * Free the array returned by os_backtrace_symbols(). + * Convert backtrace addresses to human-readable symbol strings. The symbol + * strings are written into ctx->sym_buf and ctx->syms pointers are set up. * - * @strings: array to free (may be NULL) + * @ctx: backtrace context with addrs and count already filled in */ -void os_backtrace_symbols_free(char **strings); +void os_backtrace_symbols(struct backtrace_ctx *ctx); /** * os_get_time_offset() - get time offset diff --git a/lib/backtrace.c b/lib/backtrace.c index 715d7d1d05e..b01a08af8ba 100644 --- a/lib/backtrace.c +++ b/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); diff --git a/test/lib/backtrace.c b/test/lib/backtrace.c index d9c36bbd495..11f0d43ca7e 100644 --- a/test/lib/backtrace.c +++ b/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; } }