[PATCH 00/35] malloc: Add heap debugging commands and mcheck caller tracking
From: Simon Glass <simon.glass@canonical.com> This series adds improved heap-debugging capabilities. As part of this, the recently added backtrace feature is reworked to avoid itself using malloc(), which makes it difficult for malloc() to use. A new 'malloc' command with 'info' and 'dump' subcommands allows inspecting the heap state at runtime. The mcheck heap-protection feature is integrated into Kconfig and can be used to to track caller information for each allocation, showing the function name and line number. The caller info can be seen with 'malloc dump', including for freed chunks where possible. The 'malloc info' command shows a few more heap statistics: => malloc info total bytes = 96 MiB in use bytes = 700.9 KiB malloc count = 1234 free count = 567 realloc count = 89 The 'malloc dump' command walks the heap showing each chunk: => malloc dump Heap dump: 19a0e000 - 1fa10000 Address Size Status ---------------------------------- 19a0e000 10 (chunk header) 19a0e010 a0 log_init:453 <-board_init_r:774 <-sandbox_flow: 19a0e0b0 20070 membuf_new:420 <-console_record_init:880 <-boar 19a2e120 170 membuf_new:420 <-console_record_init:886 <-boar 19a2e290 150 unflatten_device_tree:299 <-of_live_build:328 < 19a2e3e0 a0 uclass_get:157 <-device_bind_common:59 <-device ... 19a4b080 70 free done_word:2489 <-parse_stream_outer:3190 <-pars ... This is useful for debugging memory leaks, understanding allocation patterns, and tracking down heap corruption issues. Some additional patches are included to make all this work: - format_size() for human-readable size formatting - ut_asserteq_regex() for flexible test assertions - backtrace_str() for condensed backtrace output Finally, this series includes a few patches for some of the more obvious memory leaks (scmi and some test drivers), plus a reduction in truetype memory allocations and a fix for a watchdog crash with 'ut dm'. Simon Glass (35): watchdog: Unregister cyclic on device removal firmware: scmi: Free allocated strings on device removal test: dm: Fix memory leaks in test drivers test: dm: Fix delta usage in dm_test_devres_free() mmc: sandbox: Fix memory leak on probe failure video: truetype: Use pre-allocated buffer for glyph rendering lib: Add format_size() to format sizes as strings test: Add ut_asserteq_regex() for regex pattern matching test: Show the required size when console-record overflows malloc: Use mcheck.h header instead of local declaration malloc: Make mcheck respect REALLOC_ZERO_BYTES_FREES backtrace: Use a static buffer in backtrace_ctx for symbols backtrace: Add backtrace_str() for condensed backtrace output malloc: Fix unused internal_memalign warning with mcheck malloc: Add malloc_get_info() to retrieve memory statistics malloc: Add a Kconfig option for debugging malloc() malloc: Enable stats if UNIT_TEST or MALLOC_DEBUG malloc: Add 'malloc' command with 'info' subcommand malloc: Add call counters for malloc, free, realloc malloc: Add a Kconfig option for heap protection malloc: Move mcheck block after includes malloc: Fix internal calls and memalign for mcheck malloc: Update the valloc functions to use mcheck wrappers malloc: Increase the mcheck registry size sandbox: Enable mcheck heap protection malloc: Remove warning messages during relocation malloc: Support storing caller information malloc: Add a caller-info parameter to dlmalloc_impl() malloc: Add malloc dump command to walk the heap malloc: Fix malloc_dump to find mcheck headers in memalign chunks malloc: Record caller backtrace for each allocation malloc: Skip backtrace when stack is corrupted malloc: Show caller info for freed chunks in malloc_dump malloc: Print mcheck registry-overflow message only once doc: malloc: Document debugging features Kconfig | 18 ++ arch/sandbox/cpu/backtrace.c | 53 ++-- arch/sandbox/lib/backtrace.c | 47 +-- cmd/Kconfig | 9 + cmd/Makefile | 1 + cmd/font.c | 30 +- cmd/malloc.c | 47 +++ cmd/stackprot_test.c | 7 + common/Kconfig | 1 + common/board_f.c | 4 +- common/console.c | 16 +- common/dlmalloc.c | 366 +++++++++++++++++++--- common/mcheck_core.inc.h | 59 ++-- common/stackprot.c | 6 + configs/sandbox_defconfig | 5 +- doc/develop/malloc.rst | 103 +++++- doc/develop/tests_writing.rst | 5 + doc/usage/cmd/font.rst | 20 +- doc/usage/cmd/malloc.rst | 83 +++++ doc/usage/index.rst | 1 + drivers/firmware/scmi/scmi_agent-uclass.c | 21 ++ drivers/mmc/sandbox_mmc.c | 11 +- drivers/video/Kconfig | 24 ++ drivers/video/console_truetype.c | 59 +++- drivers/watchdog/wdt-uclass.c | 11 + include/asm-generic/global_data.h | 36 +++ include/backtrace.h | 77 ++++- include/display_options.h | 14 + include/malloc.h | 58 ++++ include/mcheck.h | 3 + include/os.h | 22 +- include/test/ut.h | 29 ++ lib/Kconfig | 16 + lib/backtrace.c | 135 +++++++- lib/display_options.c | 34 +- test/cmd/Makefile | 1 + test/cmd/font.c | 20 ++ test/cmd/malloc.c | 54 ++++ test/common/malloc.c | 2 +- test/dm/core.c | 1 + test/dm/devres.c | 10 +- test/dm/test-driver.c | 3 + test/lib/backtrace.c | 50 ++- test/lib/test_print.c | 14 + test/ut.c | 45 ++- 45 files changed, 1411 insertions(+), 220 deletions(-) create mode 100644 cmd/malloc.c create mode 100644 doc/usage/cmd/malloc.rst create mode 100644 test/cmd/malloc.c -- 2.43.0 base-commit: 6f2f9e2a30add30b4de20a5a7bb3ee39e4f6f485 branch: malb
From: Simon Glass <simon.glass@canonical.com> When a watchdog device is destroyed, the cyclic_info embedded in the device's private data is freed but remains in the global cyclic list. The subsequent cyclic_unregister_all() call then accesses freed memory, causing a crash. Add a pre_remove hook to the watchdog uclass to unregister the cyclic function before the device is destroyed. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- drivers/watchdog/wdt-uclass.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index 10be334e9ed..c2d19530b3d 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -256,10 +256,21 @@ static int wdt_pre_probe(struct udevice *dev) return 0; } +static int wdt_pre_remove(struct udevice *dev) +{ + struct wdt_priv *priv = dev_get_uclass_priv(dev); + + if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running) + cyclic_unregister(&priv->cyclic); + + return 0; +} + UCLASS_DRIVER(wdt) = { .id = UCLASS_WDT, .name = "watchdog", .flags = DM_UC_FLAG_SEQ_ALIAS, .pre_probe = wdt_pre_probe, + .pre_remove = wdt_pre_remove, .per_device_auto = sizeof(struct wdt_priv), }; -- 2.43.0
Am 10. Dezember 2025 01:06:52 MEZ schrieb Simon Glass <sjg@u-boot.org>:
From: Simon Glass <simon.glass@canonical.com>
When a watchdog device is destroyed, the cyclic_info embedded in the device's private data is freed but remains in the global cyclic list. The subsequent cyclic_unregister_all() call then accesses freed memory, causing a crash.
Add a pre_remove hook to the watchdog uclass to unregister the cyclic function before the device is destroyed.
Is this a sandbox only problem? Or can this happen on real hardware when booting?
Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> ---
drivers/watchdog/wdt-uclass.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index 10be334e9ed..c2d19530b3d 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -256,10 +256,21 @@ static int wdt_pre_probe(struct udevice *dev) return 0; }
+static int wdt_pre_remove(struct udevice *dev) +{ + struct wdt_priv *priv = dev_get_uclass_priv(dev); + + if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running) + cyclic_unregister(&priv->cyclic); + + return 0; +} + UCLASS_DRIVER(wdt) = { .id = UCLASS_WDT, .name = "watchdog", .flags = DM_UC_FLAG_SEQ_ALIAS, .pre_probe = wdt_pre_probe, + .pre_remove = wdt_pre_remove, .per_device_auto = sizeof(struct wdt_priv), };
Hi Heinrich, On Tue, 9 Dec 2025 at 23:53, Heinrich Schuchardt via Concept <concept@u-boot.org> wrote:
Am 10. Dezember 2025 01:06:52 MEZ schrieb Simon Glass <sjg@u-boot.org>:
From: Simon Glass <simon.glass@canonical.com>
When a watchdog device is destroyed, the cyclic_info embedded in the device's private data is freed but remains in the global cyclic list. The subsequent cyclic_unregister_all() call then accesses freed memory, causing a crash.
Add a pre_remove hook to the watchdog uclass to unregister the cyclic function before the device is destroyed.
Is this a sandbox only problem? Or can this happen on real hardware when booting?
Normally when booting we don't have idle time, so cyclic is not called. It looks like only octeontx_wdt.c is marked with the DM_FLAG_OS_PREPARE flag. So perhaps on that platform, if the device is removed and then schedule() is called, it would cause the problem. Regards, Simon
Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> ---
drivers/watchdog/wdt-uclass.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index 10be334e9ed..c2d19530b3d 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -256,10 +256,21 @@ static int wdt_pre_probe(struct udevice *dev) return 0; }
+static int wdt_pre_remove(struct udevice *dev) +{ + struct wdt_priv *priv = dev_get_uclass_priv(dev); + + if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running) + cyclic_unregister(&priv->cyclic); + + return 0; +} + UCLASS_DRIVER(wdt) = { .id = UCLASS_WDT, .name = "watchdog", .flags = DM_UC_FLAG_SEQ_ALIAS, .pre_probe = wdt_pre_probe, + .pre_remove = wdt_pre_remove, .per_device_auto = sizeof(struct wdt_priv), };
_______________________________________________ Concept mailing list -- concept@u-boot.org To unsubscribe send an email to concept-leave@u-boot.org
From: Simon Glass <simon.glass@canonical.com> The SCMI agent uclass allocates strings for vendor, sub_vendor, agent_name, and protocols in the probe function but never frees them when the device is removed. This causes memory leaks when the device model is rebuilt, such as during test runs. Add a pre_remove function to free these allocated strings. Take care to only free sub_vendor and agent_name if they were actually allocated (they may point to the string literal "NA" when the corresponding SCMI calls are not supported). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-developed-by: Claude <noreply@anthropic.com> --- drivers/firmware/scmi/scmi_agent-uclass.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c index 8c907c3b032..c77dd068c60 100644 --- a/drivers/firmware/scmi/scmi_agent-uclass.c +++ b/drivers/firmware/scmi/scmi_agent-uclass.c @@ -462,10 +462,31 @@ static int scmi_bind_protocols(struct udevice *dev) return ret; } +static int scmi_remove(struct udevice *dev) +{ + struct scmi_agent_priv *priv = dev_get_uclass_plat(dev); + + free(priv->vendor); + free(priv->protocols); + + /* + * Note: sub_vendor and agent_name may point to string literals "NA" + * when the corresponding SCMI calls are not supported; only free if + * they were allocated (i.e., not pointing to "NA") + */ + if (strcmp(priv->sub_vendor, "NA")) + free(priv->sub_vendor); + if (strcmp(priv->agent_name, "NA")) + free(priv->agent_name); + + return 0; +} + UCLASS_DRIVER(scmi_agent) = { .id = UCLASS_SCMI_AGENT, .name = "scmi_agent", .post_bind = scmi_bind_protocols, + .pre_remove = scmi_remove, .per_device_plat_auto = sizeof(struct scmi_agent_priv), .per_child_auto = sizeof(struct scmi_agent_proto_priv), }; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The test_manual_drv driver allocates private data in its probe function with calloc(), but never frees it in remove(). Add the missing free() call to test_manual_remove(). Similarly, create_children() allocates platform data with calloc() and sets it with dev_set_plat(), but doesn't set the DM_FLAG_ALLOC_PDATA flag. This flag tells the device removal code to free the platform data. Set this flag so driver model will free the allocated memory on unbind. These fixes eliminate memory leaks that caused the malloc_dump output to grow excessively after running DM tests. Co-developed-by: Claude Opus 4 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/dm/core.c | 1 + test/dm/test-driver.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/test/dm/core.c b/test/dm/core.c index 959b834576f..75c44e8328c 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -606,6 +606,7 @@ static int create_children(struct unit_test_state *uts, struct udevice *parent, pdata = calloc(1, sizeof(*pdata)); pdata->ping_add = key + i; dev_set_plat(dev, pdata); + dev_or_flags(dev, DM_FLAG_ALLOC_PDATA); if (child) child[i] = dev; } diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c index 759de3a5f77..fc4e3d82a1f 100644 --- a/test/dm/test-driver.c +++ b/test/dm/test-driver.c @@ -135,12 +135,15 @@ static int test_manual_probe(struct udevice *dev) static int test_manual_remove(struct udevice *dev) { dm_testdrv_op_count[DM_TEST_OP_REMOVE]++; + free(dev_get_priv(dev)); + return 0; } static int test_manual_unbind(struct udevice *dev) { dm_testdrv_op_count[DM_TEST_OP_UNBIND]++; + return 0; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> This test passes delta values to ut_check_delta() where it expects absolute values. ut_check_delta(last) computes (current - last), so 'last' must be an absolute reference point from a previous ut_check_delta(0) call, not a delta. The bug causes incorrect memory accounting that happens to work with smaller allocation overhead but fails when mcheck header size increases. Use ut_check_delta(0) to capture absolute reference points before each operation. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/dm/devres.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/dm/devres.c b/test/dm/devres.c index efc5c72ae2a..353353767b4 100644 --- a/test/dm/devres.c +++ b/test/dm/devres.c @@ -44,23 +44,21 @@ DM_TEST(dm_test_devres_alloc, UTF_SCAN_PDATA); /* Test devm_kfree() can be used to free memory too */ static int dm_test_devres_free(struct unit_test_state *uts) { - ulong mem_start, mem_dev, mem_kmalloc; + ulong mem_start, mem_alloc; struct udevice *dev; void *ptr; mem_start = ut_check_delta(0); ut_assertok(uclass_first_device_err(UCLASS_TEST, &dev)); - mem_dev = ut_check_delta(mem_start); - ut_assert(mem_dev > 0); + ut_assert(ut_check_delta(mem_start) > 0); ptr = devm_kmalloc(dev, TEST_DEVRES_SIZE, 0); ut_assert(ptr != NULL); - mem_kmalloc = ut_check_delta(mem_dev); - ut_assert(mem_kmalloc > 0); /* Free the ptr and check that memory usage goes down */ + mem_alloc = ut_check_delta(0); devm_kfree(dev, ptr); - ut_assert(ut_check_delta(mem_kmalloc) < 0); + ut_assert(ut_check_delta(mem_alloc) < 0); device_remove(dev, DM_REMOVE_NORMAL); ut_asserteq(0, ut_check_delta(mem_start)); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> If mmc_init() fails, the buffer allocated by calloc() or mapped by os_map_file() is not freed. Free or unmap the buffer before returning the error. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- drivers/mmc/sandbox_mmc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c index a24520f2e78..f49cb4b146a 100644 --- a/drivers/mmc/sandbox_mmc.c +++ b/drivers/mmc/sandbox_mmc.c @@ -190,7 +190,16 @@ static int sandbox_mmc_probe(struct udevice *dev) } } - return mmc_init(&plat->mmc); + ret = mmc_init(&plat->mmc); + if (ret) { + if (plat->fname) + os_unmap(priv->buf, priv->size); + else + free(priv->buf); + return ret; + } + + return 0; } static int sandbox_mmc_remove(struct udevice *dev) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The TrueType console driver calls malloc/free for every character rendered, which causes significant memory fragmentation and allocation traffic. Add a pre-allocated buffer (CONFIG_CONSOLE_TRUETYPE_GLYPH_BUF_SIZE, default 4KB) to the driver's private data. When rendering a glyph, use this buffer if the glyph bitmap fits, avoiding malloc/free for normal characters. Fall back to malloc only for oversized glyphs. The buffer is allocated lazily after relocation to avoid consuming early malloc space before the full heap is available. Add CONFIG_VIDEO_GLYPH_STATS (default y on sandbox) to track cumulative glyph rendering statistics in global_data. This persists across video device rebinds and can be viewed via 'font info'. Co-developed-by: Claude Opus 4 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- cmd/font.c | 30 ++++++++++++++-- doc/usage/cmd/font.rst | 20 ++++++++++- drivers/video/Kconfig | 24 +++++++++++++ drivers/video/console_truetype.c | 59 +++++++++++++++++++++++++++---- include/asm-generic/global_data.h | 22 ++++++++++++ test/cmd/font.c | 20 +++++++++++ 6 files changed, 166 insertions(+), 9 deletions(-) diff --git a/cmd/font.c b/cmd/font.c index 384751e787a..99c8ae76f2e 100644 --- a/cmd/font.c +++ b/cmd/font.c @@ -11,6 +11,22 @@ #include <video.h> #include <video_console.h> +#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) +static int do_font_info(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + int buf_count, malloc_count; + + /* read the results first so that they don't change while printing */ + buf_count = gd->glyph_buf_count; + malloc_count = gd->glyph_malloc_count; + printf("glyph buffer renders: %u\n", buf_count); + printf("glyph malloc renders: %u\n", malloc_count); + + return 0; +} +#endif + static int do_font_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -75,12 +91,22 @@ static int do_font_size(struct cmd_tbl *cmdtp, int flag, int argc, return 0; } +#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) +#define FONT_INFO_HELP "\nfont info - show glyph rendering statistics" +#define FONT_INFO_SUB , U_BOOT_SUBCMD_MKENT(info, 1, 1, do_font_info) +#else +#define FONT_INFO_HELP +#define FONT_INFO_SUB +#endif + U_BOOT_LONGHELP(font, "list - list available fonts\n" "font select <name> [<size>] - select font to use\n" - "font size <size> - select font size to"); + "font size <size> - select font size to" + FONT_INFO_HELP); U_BOOT_CMD_WITH_SUBCMDS(font, "Fonts", font_help_text, U_BOOT_SUBCMD_MKENT(list, 1, 1, do_font_list), U_BOOT_SUBCMD_MKENT(select, 3, 1, do_font_select), - U_BOOT_SUBCMD_MKENT(size, 2, 1, do_font_size)); + U_BOOT_SUBCMD_MKENT(size, 2, 1, do_font_size) + FONT_INFO_SUB); diff --git a/doc/usage/cmd/font.rst b/doc/usage/cmd/font.rst index 6e313e70c7a..5e1f6e446e4 100644 --- a/doc/usage/cmd/font.rst +++ b/doc/usage/cmd/font.rst @@ -14,6 +14,7 @@ Synopsis font list font select [<name> [<size>]] font size [<size>] + font info Description ----------- @@ -38,6 +39,15 @@ font size This changes the font size only. With no argument it shows the current size. +font info +~~~~~~~~~ + +This shows glyph rendering statistics. The TrueType driver uses a pre-allocated +buffer to avoid malloc/free for each character rendered. This command shows how +many glyphs were rendered using the buffer vs. falling back to malloc. + +This subcommand requires CONFIG_VIDEO_GLYPH_STATS=y. + Examples -------- @@ -52,7 +62,7 @@ Examples => font select cantoraone_regular 20 => -This shows an example of selecting a bitmap font Truetype is active:: +This shows an example of selecting a bitmap font when Truetype is active:: => font list 8x16 @@ -61,12 +71,20 @@ This shows an example of selecting a bitmap font Truetype is active:: cantoraone_regular => font sel 8x16 +This shows glyph rendering statistics:: + + => font info + glyph buffer renders: 32705 + glyph malloc renders: 0 + Configuration ------------- The command is only available if CONFIG_CONSOLE_TRUETYPE=y. +CONFIG_VIDEO_GLYPH_STATS enables tracking of glyph-rendering statistics. + Return value ------------ diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 446ce51fe27..c09341a08dc 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -247,6 +247,30 @@ config CONSOLE_TRUETYPE_MAX_METRICS font metrics which are expensive to regenerate each time the font size changes. +config CONSOLE_TRUETYPE_GLYPH_BUF_SIZE + int "TrueType per-character glyph buffer size" + depends on CONSOLE_TRUETYPE + default 4096 + help + This sets the size of a pre-allocated buffer for rendering glyph + bitmaps. If a glyph fits within this buffer, no malloc/free is + needed during character rendering, which reduces memory fragmentation + and improves performance. Glyphs larger than this size fall back to + malloc. + + The default of 4096 bytes supports characters up to 64x64 pixels, + which covers most font sizes. Increase this if using very large fonts. + +config VIDEO_GLYPH_STATS + bool "Track glyph rendering statistics" + depends on CONSOLE_TRUETYPE + default y if SANDBOX + help + Track cumulative glyph rendering statistics in global_data, so they + persist across video device rebinds. This allows seeing the total + count of glyphs rendered using the pre-allocated buffer vs. malloc + fallback. Use 'font info' to view the statistics. + config SYS_WHITE_ON_BLACK bool "Display console as white on a black background" default y if ARCH_AT91 || ARCH_EXYNOS || ARCH_ROCKCHIP || ARCH_TEGRA || X86 || ARCH_SUNXI diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c index 7f5a2262b17..f335c9f1148 100644 --- a/drivers/video/console_truetype.c +++ b/drivers/video/console_truetype.c @@ -180,6 +180,9 @@ struct console_tt_metrics { * @pos_start: Value of pos_ptr when the cursor is at the start of the text * being entered by the user * @pos_count: Maximum value reached by pos_ptr (initially zero) + * @glyph_buf: Pre-allocated buffer for rendering glyphs. If a glyph fits, + * this avoids malloc/free per character. Allocated lazily after + * relocation to avoid using early malloc space. */ struct console_tt_priv { struct console_tt_metrics *cur_met; @@ -190,6 +193,7 @@ struct console_tt_priv { struct video_fontdata *cur_fontdata; int pos_start; int pos_count; + u8 *glyph_buf; }; /** @@ -365,6 +369,7 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, int advance; void *start, *end, *line; int row, kern; + bool use_buf; /* Use fixed font if selected */ if (priv->cur_fontdata) @@ -440,13 +445,53 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, * information into the render, which will return a 8-bit-per-pixel * image of the character. For empty characters, like ' ', data will * return NULL; + * + * Use the pre-allocated glyph buffer if large enough, falling back to + * malloc for oversized glyphs. This avoids alloc/free traffic for + * normal characters. */ - data = stbtt_GetCodepointBitmapSubpixel(font, met->scale, met->scale, - x_shift, 0, cp, &width, &height, - &xoff, &yoff); - if (!data) + { + int ix0, iy0, ix1, iy1; + + stbtt_GetCodepointBitmapBoxSubpixel(font, cp, met->scale, + met->scale, x_shift, 0, + &ix0, &iy0, &ix1, &iy1); + width = ix1 - ix0; + height = iy1 - iy0; + xoff = ix0; + yoff = iy0; + } + if (!width || !height) return width_frac; + /* + * Use the pre-allocated buffer if available and large enough. Allocate + * it lazily, but only after relocation to avoid using early malloc. + */ + use_buf = false; + if (xpl_phase() >= PHASE_BOARD_R && + width * height <= CONFIG_CONSOLE_TRUETYPE_GLYPH_BUF_SIZE) { + if (!priv->glyph_buf) { + priv->glyph_buf = + malloc(CONFIG_CONSOLE_TRUETYPE_GLYPH_BUF_SIZE); + } + if (priv->glyph_buf) { + data = priv->glyph_buf; + use_buf = true; + gd_inc_glyph_buf_count(); + } + } + if (!use_buf) { + data = malloc(width * height); + if (!data) + return width_frac; + gd_inc_glyph_malloc_count(); + } + + stbtt_MakeCodepointBitmapSubpixel(font, data, width, height, width, + met->scale, met->scale, x_shift, 0, + cp); + /* Figure out where to write the character in the frame buffer */ bits = data; start = vid_priv->fb + y * vid_priv->line_length + @@ -534,7 +579,8 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, break; } default: - free(data); + if (!use_buf) + free(data); return -ENOSYS; } @@ -547,7 +593,8 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, width, height); - free(data); + if (!use_buf) + free(data); return width_frac; } diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index cff9066de53..d91103dfe00 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -365,6 +365,16 @@ struct global_data { */ ulong video_bottom; #endif +#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) + /** + * @glyph_buf_count: glyphs rendered using pre-allocated buffer + */ + uint glyph_buf_count; + /** + * @glyph_malloc_count: glyphs rendered using malloc fallback + */ + uint glyph_malloc_count; +#endif #ifdef CONFIG_BOOTSTAGE /** * @bootstage: boot stage information @@ -637,6 +647,18 @@ static_assert(sizeof(struct global_data) == GD_SIZE); #define gd_pager_page_len() 0 #endif +#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) +#define gd_glyph_buf_count() gd->glyph_buf_count +#define gd_inc_glyph_buf_count() gd->glyph_buf_count++ +#define gd_glyph_malloc_count() gd->glyph_malloc_count +#define gd_inc_glyph_malloc_count() gd->glyph_malloc_count++ +#else +#define gd_glyph_buf_count() 0 +#define gd_inc_glyph_buf_count() +#define gd_glyph_malloc_count() 0 +#define gd_inc_glyph_malloc_count() +#endif + /** * enum gd_flags - global data flags * diff --git a/test/cmd/font.c b/test/cmd/font.c index adfeebe920d..1bdc14e0250 100644 --- a/test/cmd/font.c +++ b/test/cmd/font.c @@ -98,3 +98,23 @@ static int font_test_base(struct unit_test_state *uts) } FONT_TEST(font_test_base, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE | UTF_DM); + +/* Test 'font info' command */ +static int font_test_info(struct unit_test_state *uts) +{ + int buf_count, malloc_count; + + if (!CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS)) + return -EAGAIN; + + /* read the results first so that they don't change while printing */ + buf_count= gd_glyph_buf_count(); + malloc_count = gd_glyph_malloc_count(); + ut_assertok(run_command("font info", 0)); + ut_assert_nextline("glyph buffer renders: %u", buf_count); + ut_assert_nextline("glyph malloc renders: %u", malloc_count); + ut_assert_console_end(); + + return 0; +} +FONT_TEST(font_test_info, UTF_CONSOLE); -- 2.43.0
Am 10. Dezember 2025 01:06:57 MEZ schrieb Simon Glass <sjg@u-boot.org>:
From: Simon Glass <simon.glass@canonical.com>
The TrueType console driver calls malloc/free for every character rendered, which causes significant memory fragmentation and allocation traffic.
Add a pre-allocated buffer (CONFIG_CONSOLE_TRUETYPE_GLYPH_BUF_SIZE, default 4KB) to the driver's private data. When rendering a glyph, use this buffer if the glyph bitmap fits, avoiding malloc/free for normal characters. Fall back to malloc only for oversized glyphs.
Please, avoid the config variable. Instead use realloc() to resize the buffer whenever it is too small. This should retain the performance gain but will allow for arbitrary font sizes set by the user. Best regards Heinrich
The buffer is allocated lazily after relocation to avoid consuming early malloc space before the full heap is available.
Add CONFIG_VIDEO_GLYPH_STATS (default y on sandbox) to track cumulative glyph rendering statistics in global_data. This persists across video device rebinds and can be viewed via 'font info'.
Co-developed-by: Claude Opus 4 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> ---
cmd/font.c | 30 ++++++++++++++-- doc/usage/cmd/font.rst | 20 ++++++++++- drivers/video/Kconfig | 24 +++++++++++++ drivers/video/console_truetype.c | 59 +++++++++++++++++++++++++++---- include/asm-generic/global_data.h | 22 ++++++++++++ test/cmd/font.c | 20 +++++++++++ 6 files changed, 166 insertions(+), 9 deletions(-)
diff --git a/cmd/font.c b/cmd/font.c index 384751e787a..99c8ae76f2e 100644 --- a/cmd/font.c +++ b/cmd/font.c @@ -11,6 +11,22 @@ #include <video.h> #include <video_console.h>
+#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) +static int do_font_info(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + int buf_count, malloc_count; + + /* read the results first so that they don't change while printing */ + buf_count = gd->glyph_buf_count; + malloc_count = gd->glyph_malloc_count; + printf("glyph buffer renders: %u\n", buf_count); + printf("glyph malloc renders: %u\n", malloc_count); + + return 0; +} +#endif + static int do_font_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -75,12 +91,22 @@ static int do_font_size(struct cmd_tbl *cmdtp, int flag, int argc, return 0; }
+#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) +#define FONT_INFO_HELP "\nfont info - show glyph rendering statistics" +#define FONT_INFO_SUB , U_BOOT_SUBCMD_MKENT(info, 1, 1, do_font_info) +#else +#define FONT_INFO_HELP +#define FONT_INFO_SUB +#endif + U_BOOT_LONGHELP(font, "list - list available fonts\n" "font select <name> [<size>] - select font to use\n" - "font size <size> - select font size to"); + "font size <size> - select font size to" + FONT_INFO_HELP);
U_BOOT_CMD_WITH_SUBCMDS(font, "Fonts", font_help_text, U_BOOT_SUBCMD_MKENT(list, 1, 1, do_font_list), U_BOOT_SUBCMD_MKENT(select, 3, 1, do_font_select), - U_BOOT_SUBCMD_MKENT(size, 2, 1, do_font_size)); + U_BOOT_SUBCMD_MKENT(size, 2, 1, do_font_size) + FONT_INFO_SUB); diff --git a/doc/usage/cmd/font.rst b/doc/usage/cmd/font.rst index 6e313e70c7a..5e1f6e446e4 100644 --- a/doc/usage/cmd/font.rst +++ b/doc/usage/cmd/font.rst @@ -14,6 +14,7 @@ Synopsis font list font select [<name> [<size>]] font size [<size>] + font info
Description ----------- @@ -38,6 +39,15 @@ font size
This changes the font size only. With no argument it shows the current size.
+font info +~~~~~~~~~ + +This shows glyph rendering statistics. The TrueType driver uses a pre-allocated +buffer to avoid malloc/free for each character rendered. This command shows how +many glyphs were rendered using the buffer vs. falling back to malloc. + +This subcommand requires CONFIG_VIDEO_GLYPH_STATS=y. + Examples --------
@@ -52,7 +62,7 @@ Examples => font select cantoraone_regular 20 =>
-This shows an example of selecting a bitmap font Truetype is active:: +This shows an example of selecting a bitmap font when Truetype is active::
=> font list 8x16 @@ -61,12 +71,20 @@ This shows an example of selecting a bitmap font Truetype is active:: cantoraone_regular => font sel 8x16
+This shows glyph rendering statistics:: + + => font info + glyph buffer renders: 32705 + glyph malloc renders: 0 +
Configuration -------------
The command is only available if CONFIG_CONSOLE_TRUETYPE=y.
+CONFIG_VIDEO_GLYPH_STATS enables tracking of glyph-rendering statistics. + Return value ------------
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 446ce51fe27..c09341a08dc 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -247,6 +247,30 @@ config CONSOLE_TRUETYPE_MAX_METRICS font metrics which are expensive to regenerate each time the font size changes.
+config CONSOLE_TRUETYPE_GLYPH_BUF_SIZE + int "TrueType per-character glyph buffer size" + depends on CONSOLE_TRUETYPE + default 4096 + help + This sets the size of a pre-allocated buffer for rendering glyph + bitmaps. If a glyph fits within this buffer, no malloc/free is + needed during character rendering, which reduces memory fragmentation + and improves performance. Glyphs larger than this size fall back to + malloc. + + The default of 4096 bytes supports characters up to 64x64 pixels, + which covers most font sizes. Increase this if using very large fonts. + +config VIDEO_GLYPH_STATS + bool "Track glyph rendering statistics" + depends on CONSOLE_TRUETYPE + default y if SANDBOX + help + Track cumulative glyph rendering statistics in global_data, so they + persist across video device rebinds. This allows seeing the total + count of glyphs rendered using the pre-allocated buffer vs. malloc + fallback. Use 'font info' to view the statistics. + config SYS_WHITE_ON_BLACK bool "Display console as white on a black background" default y if ARCH_AT91 || ARCH_EXYNOS || ARCH_ROCKCHIP || ARCH_TEGRA || X86 || ARCH_SUNXI diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c index 7f5a2262b17..f335c9f1148 100644 --- a/drivers/video/console_truetype.c +++ b/drivers/video/console_truetype.c @@ -180,6 +180,9 @@ struct console_tt_metrics { * @pos_start: Value of pos_ptr when the cursor is at the start of the text * being entered by the user * @pos_count: Maximum value reached by pos_ptr (initially zero) + * @glyph_buf: Pre-allocated buffer for rendering glyphs. If a glyph fits, + * this avoids malloc/free per character. Allocated lazily after + * relocation to avoid using early malloc space. */ struct console_tt_priv { struct console_tt_metrics *cur_met; @@ -190,6 +193,7 @@ struct console_tt_priv { struct video_fontdata *cur_fontdata; int pos_start; int pos_count; + u8 *glyph_buf; };
/** @@ -365,6 +369,7 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, int advance; void *start, *end, *line; int row, kern; + bool use_buf;
/* Use fixed font if selected */ if (priv->cur_fontdata) @@ -440,13 +445,53 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, * information into the render, which will return a 8-bit-per-pixel * image of the character. For empty characters, like ' ', data will * return NULL; + * + * Use the pre-allocated glyph buffer if large enough, falling back to + * malloc for oversized glyphs. This avoids alloc/free traffic for + * normal characters. */ - data = stbtt_GetCodepointBitmapSubpixel(font, met->scale, met->scale, - x_shift, 0, cp, &width, &height, - &xoff, &yoff); - if (!data) + { + int ix0, iy0, ix1, iy1; + + stbtt_GetCodepointBitmapBoxSubpixel(font, cp, met->scale, + met->scale, x_shift, 0, + &ix0, &iy0, &ix1, &iy1); + width = ix1 - ix0; + height = iy1 - iy0; + xoff = ix0; + yoff = iy0; + } + if (!width || !height) return width_frac;
+ /* + * Use the pre-allocated buffer if available and large enough. Allocate + * it lazily, but only after relocation to avoid using early malloc. + */ + use_buf = false; + if (xpl_phase() >= PHASE_BOARD_R && + width * height <= CONFIG_CONSOLE_TRUETYPE_GLYPH_BUF_SIZE) { + if (!priv->glyph_buf) { + priv->glyph_buf = + malloc(CONFIG_CONSOLE_TRUETYPE_GLYPH_BUF_SIZE); + } + if (priv->glyph_buf) { + data = priv->glyph_buf; + use_buf = true; + gd_inc_glyph_buf_count(); + } + } + if (!use_buf) { + data = malloc(width * height); + if (!data) + return width_frac; + gd_inc_glyph_malloc_count(); + } + + stbtt_MakeCodepointBitmapSubpixel(font, data, width, height, width, + met->scale, met->scale, x_shift, 0, + cp); + /* Figure out where to write the character in the frame buffer */ bits = data; start = vid_priv->fb + y * vid_priv->line_length + @@ -534,7 +579,8 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, break; } default: - free(data); + if (!use_buf) + free(data); return -ENOSYS; }
@@ -547,7 +593,8 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, width, height);
- free(data); + if (!use_buf) + free(data);
return width_frac; } diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index cff9066de53..d91103dfe00 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -365,6 +365,16 @@ struct global_data { */ ulong video_bottom; #endif +#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) + /** + * @glyph_buf_count: glyphs rendered using pre-allocated buffer + */ + uint glyph_buf_count; + /** + * @glyph_malloc_count: glyphs rendered using malloc fallback + */ + uint glyph_malloc_count; +#endif #ifdef CONFIG_BOOTSTAGE /** * @bootstage: boot stage information @@ -637,6 +647,18 @@ static_assert(sizeof(struct global_data) == GD_SIZE); #define gd_pager_page_len() 0 #endif
+#if CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS) +#define gd_glyph_buf_count() gd->glyph_buf_count +#define gd_inc_glyph_buf_count() gd->glyph_buf_count++ +#define gd_glyph_malloc_count() gd->glyph_malloc_count +#define gd_inc_glyph_malloc_count() gd->glyph_malloc_count++ +#else +#define gd_glyph_buf_count() 0 +#define gd_inc_glyph_buf_count() +#define gd_glyph_malloc_count() 0 +#define gd_inc_glyph_malloc_count() +#endif + /** * enum gd_flags - global data flags * diff --git a/test/cmd/font.c b/test/cmd/font.c index adfeebe920d..1bdc14e0250 100644 --- a/test/cmd/font.c +++ b/test/cmd/font.c @@ -98,3 +98,23 @@ static int font_test_base(struct unit_test_state *uts) } FONT_TEST(font_test_base, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE | UTF_DM); + +/* Test 'font info' command */ +static int font_test_info(struct unit_test_state *uts) +{ + int buf_count, malloc_count; + + if (!CONFIG_IS_ENABLED(VIDEO_GLYPH_STATS)) + return -EAGAIN; + + /* read the results first so that they don't change while printing */ + buf_count= gd_glyph_buf_count(); + malloc_count = gd_glyph_malloc_count(); + ut_assertok(run_command("font info", 0)); + ut_assert_nextline("glyph buffer renders: %u", buf_count); + ut_assert_nextline("glyph malloc renders: %u", malloc_count); + ut_assert_console_end(); + + return 0; +} +FONT_TEST(font_test_info, UTF_CONSOLE);
From: Simon Glass <simon.glass@canonical.com> Refactor print_size() to use a new format_size() helper that formats a size into a buffer. This allows callers to get the formatted string without printing it directly. The format_size() function is only exported in U-Boot proper (controlled by CONFIG_LIB_FORMAT_SIZE) to avoid code-size impact in SPL/TPL where it remains static. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- include/display_options.h | 14 ++++++++++++++ lib/Kconfig | 7 +++++++ lib/display_options.c | 34 ++++++++++++++++++++++++---------- test/lib/test_print.c | 14 ++++++++++++++ 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/include/display_options.h b/include/display_options.h index f3bdff172eb..5941d6bd37a 100644 --- a/include/display_options.h +++ b/include/display_options.h @@ -11,6 +11,20 @@ #include <linux/types.h> +#if CONFIG_IS_ENABLED(LIB_FORMAT_SIZE) +/** + * format_size() - Format a size with a suffix + * + * Format sizes as "xxx KiB", "xxx.y KiB", "xxx MiB", "xxx.y MiB", + * xxx GiB, xxx.y GiB, etc as needed + * + * @buf: Buffer to write to (must be at least 12 bytes) + * @size: Size to format + * Return: @buf + */ +char *format_size(char *buf, uint64_t size); +#endif + /** * print_size() - Print a size with a suffix * diff --git a/lib/Kconfig b/lib/Kconfig index d7f791f77f3..79a75f98446 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -36,6 +36,13 @@ config BACKTRACE stack. This is currently only available on sandbox. The backtrace command can be used to print the backtrace. +config LIB_FORMAT_SIZE + bool + default y + help + Enables the format_size() helper which formats a size as a + human-readable string. + config BCH bool "Enable Software based BCH ECC" help diff --git a/lib/display_options.c b/lib/display_options.c index 7f9dfcc43ee..3aee08c7b00 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -98,7 +98,11 @@ void print_freq(uint64_t freq, const char *s) printf(" %cHz%s", c, s); } -void print_size(uint64_t size, const char *s) +#if CONFIG_IS_ENABLED(LIB_FORMAT_SIZE) +char *format_size(char *buf, uint64_t size) +#else +static char *format_size(char *buf, uint64_t size) +#endif { unsigned long m = 0, n; uint64_t f; @@ -107,6 +111,7 @@ void print_size(uint64_t size, const char *s) char c = 0; unsigned int i; + /* Find the best unit to display */ for (i = 0; i < ARRAY_SIZE(names); i++, d -= 10) { if (size >> d) { c = names[i]; @@ -116,12 +121,12 @@ void print_size(uint64_t size, const char *s) if (!c) { /* - * SPL tiny-printf is not capable for printing uint64_t. - * We have just checked that the size is small enought to fit + * SPL tiny-printf is not capable of printing uint64_t. + * We have just checked that the size is small enough to fit * unsigned int safely. */ - printf("%u Bytes%s", (unsigned int)size, s); - return; + sprintf(buf, "%u Bytes", (unsigned int)size); + return buf; } n = size >> d; @@ -143,11 +148,20 @@ void print_size(uint64_t size, const char *s) } } - printf ("%lu", n); - if (m) { - printf (".%ld", m); - } - printf (" %ciB%s", c, s); + if (m) + sprintf(buf, "%lu.%ld %ciB", n, m, c); + else + sprintf(buf, "%lu %ciB", n, c); + + return buf; +} + +void print_size(uint64_t size, const char *s) +{ + char buf[12]; + + format_size(buf, size); + printf("%s%s", buf, s); } #define MAX_LINE_LENGTH_BYTES 64 diff --git a/test/lib/test_print.c b/test/lib/test_print.c index cd7f3f85769..f9b04a3e3cb 100644 --- a/test/lib/test_print.c +++ b/test/lib/test_print.c @@ -68,3 +68,17 @@ static int lib_test_print_size(struct unit_test_state *uts) return 0; } LIB_TEST(lib_test_print_size, UTF_CONSOLE); + +static int lib_test_format_size(struct unit_test_state *uts) +{ + char buf[12]; + + ut_asserteq_str("321 Bytes", format_size(buf, 321)); + ut_asserteq_str("4.2 KiB", format_size(buf, 4321)); + ut_asserteq_str("53 KiB", format_size(buf, 54321)); + ut_asserteq_str("1 GiB", format_size(buf, 1073741824)); + ut_asserteq_str("49.4 TiB", format_size(buf, 54321987654321)); + + return 0; +} +LIB_TEST(lib_test_format_size, 0); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a new assertion macro ut_asserteq_regex() that checks if a string matches a regular expression pattern using the SLRE library. This is useful for tests where exact string-matching is difficult, such as when output contains line numbers or other variable content. Use a helper function ut_check_regex() to avoid including slre.h in the header. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- doc/develop/tests_writing.rst | 5 +++++ include/test/ut.h | 29 +++++++++++++++++++++++++++++ test/ut.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/doc/develop/tests_writing.rst b/doc/develop/tests_writing.rst index 31454aa4819..43756989d43 100644 --- a/doc/develop/tests_writing.rst +++ b/doc/develop/tests_writing.rst @@ -422,6 +422,11 @@ ut_asserteq_addr(expr1, expr2) Assert that two addresses (converted from pointers via map_to_sysmem()) are equal +ut_asserteq_regex(pattern, str) + Assert that a string matches a regular expression pattern. Uses the SLRE + library for regex matching. Useful when exact matching is fragile, e.g. + when output contains line numbers or variable content. + Pointer assertions ~~~~~~~~~~~~~~~~~~ diff --git a/include/test/ut.h b/include/test/ut.h index 70eaaea5e0e..a2b42cdf414 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -13,6 +13,9 @@ #include <linux/err.h> #include <test/test.h> +/* Size of error buffer for ut_check_regex() */ +#define UT_REGEX_ERR_SIZE 256 + struct unit_test_state; /** @@ -41,6 +44,16 @@ void ut_failf(struct unit_test_state *uts, const char *fname, int line, const char *func, const char *cond, const char *fmt, ...) __attribute__ ((format (__printf__, 6, 7))); +/** + * ut_check_regex() - Check if a string matches a regex pattern + * + * @pattern: Regular expression pattern + * @str: String to match against + * @err: Buffer to hold error message on failure (UT_REGEX_ERR_SIZE bytes) + * Return: 0 if match, -EINVAL if pattern is invalid, -ENOENT if no match + */ +int ut_check_regex(const char *pattern, const char *str, char *err); + /** * ut_check_console_line() - Check the next console line against expectations * @@ -254,6 +267,22 @@ int ut_check_console_dump(struct unit_test_state *uts, int total_bytes); __ret; \ }) +/* Assert that a string matches a regex pattern */ +#define ut_asserteq_regex(pattern, str) ({ \ + const char *_pattern = (pattern), *_str = (str); \ + char _err[UT_REGEX_ERR_SIZE]; \ + int __ret = 0; \ + \ + __ret = ut_check_regex(_pattern, _str, _err); \ + if (__ret) { \ + ut_failf(uts, __FILE__, __LINE__, __func__, \ + #pattern " matches " #str, "%s", _err); \ + if (!uts->soft_fail) \ + return CMD_RET_FAILURE; \ + } \ + __ret; \ +}) + /* Assert that two memory areas are equal */ #define ut_asserteq_mem(expr1, expr2, len) ({ \ const u8 *_val1 = (u8 *)(expr1), *_val2 = (u8 *)(expr2); \ diff --git a/test/ut.c b/test/ut.c index a16fdfb3a93..b4f2a8bf40f 100644 --- a/test/ut.c +++ b/test/ut.c @@ -6,7 +6,10 @@ */ #include <console.h> +#include <errno.h> #include <malloc.h> +#include <slre.h> +#include <vsprintf.h> #ifdef CONFIG_SANDBOX #include <asm/state.h> #endif @@ -38,6 +41,33 @@ void ut_failf(struct unit_test_state *uts, const char *fname, int line, uts->cur.fail_count++; } +int ut_check_regex(const char *pattern, const char *str, char *err) +{ + struct slre slre; + + if (!pattern || !str) { + snprintf(err, UT_REGEX_ERR_SIZE, + "NULL value: pattern=%s, str=%s", + pattern ? pattern : "(null)", + str ? str : "(null)"); + return -EINVAL; + } + + if (!slre_compile(&slre, pattern)) { + snprintf(err, UT_REGEX_ERR_SIZE, + "Invalid regex '%s': %s", pattern, slre.err_str); + return -EINVAL; + } + + if (!slre_match(&slre, str, strlen(str), NULL)) { + snprintf(err, UT_REGEX_ERR_SIZE, + "No match: pattern '%s', str '%s'", pattern, str); + return -ENOENT; + } + + return 0; +} + ulong ut_check_free(void) { struct mallinfo info = mallinfo(); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When the console-record buffer overflows, show both the current buffer size and the size needed. This helps the user know what value to set for CONFIG_CONSOLE_RECORD_OUT_SIZE. Add a console_out_ovf field to global_data to track the number of bytes that could not be written due to overflow. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/console.c | 16 +++++++++++----- include/asm-generic/global_data.h | 14 ++++++++++++++ test/ut.c | 15 +++++++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/common/console.c b/common/console.c index c033d72486a..da1125cfa98 100644 --- a/common/console.c +++ b/common/console.c @@ -36,21 +36,26 @@ static void console_record_putc(const char c) { if (!(gd->flags & GD_FLG_RECORD)) return; - if (gd->console_out.start && - !membuf_putbyte((struct membuf *)&gd->console_out, c)) + if (gd->console_out.start && + !membuf_putbyte((struct membuf *)&gd->console_out, c)) { gd->flags |= GD_FLG_RECORD_OVF; + gd->console_out_ovf++; + } } static void console_record_puts(const char *s) { if (!(gd->flags & GD_FLG_RECORD)) return; - if (gd->console_out.start) { + if (gd->console_out.start) { int len = strlen(s); + int written; - if (membuf_put((struct membuf *)&gd->console_out, s, len) != - len) + written = membuf_put((struct membuf *)&gd->console_out, s, len); + if (written != len) { gd->flags |= GD_FLG_RECORD_OVF; + gd->console_out_ovf += len - written; + } } } @@ -893,6 +898,7 @@ void console_record_reset(void) membuf_purge((struct membuf *)&gd->console_out); membuf_purge((struct membuf *)&gd->console_in); gd->flags &= ~GD_FLG_RECORD_OVF; + gd->console_out_ovf = 0; } int console_record_reset_enable(void) diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index d91103dfe00..4a197a2c230 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -347,6 +347,12 @@ struct global_data { * This buffer is used to collect output during console recording. */ struct membuf console_out; + /** + * @console_out_ovf: overflow byte count for console recording + * + * Number of bytes that could not be written due to buffer overflow. + */ + int console_out_ovf; /** * @console_in: input buffer for console recording * @@ -589,6 +595,14 @@ static_assert(sizeof(struct global_data) == GD_SIZE); #define gd_malloc_ptr() 0L #endif +#ifdef CONFIG_CONSOLE_RECORD +#define gd_console_out() (&gd->console_out) +#define gd_console_out_ovf() gd->console_out_ovf +#else +#define gd_console_out() NULL +#define gd_console_out_ovf() 0 +#endif + #if CONFIG_IS_ENABLED(UPL) #define gd_upl() gd->upl #define gd_set_upl(_val) gd->upl = (_val) diff --git a/test/ut.c b/test/ut.c index b4f2a8bf40f..94b09364687 100644 --- a/test/ut.c +++ b/test/ut.c @@ -8,6 +8,7 @@ #include <console.h> #include <errno.h> #include <malloc.h> +#include <membuf.h> #include <slre.h> #include <vsprintf.h> #ifdef CONFIG_SANDBOX @@ -86,8 +87,18 @@ static int readline_check(struct unit_test_state *uts) ret = console_record_readline(uts->actual_str, sizeof(uts->actual_str)); if (ret == -ENOSPC) { - ut_fail(uts, __FILE__, __LINE__, __func__, - "Console record buffer too small - increase CONFIG_CONSOLE_RECORD_OUT_SIZE"); + if (IS_ENABLED(CONFIG_CONSOLE_RECORD)) { + int cur_size = membuf_size(gd_console_out()); + + ut_failf(uts, __FILE__, __LINE__, __func__, + "Console record buffer too small", + "CONFIG_CONSOLE_RECORD_OUT_SIZE=%#x, need %#x", + cur_size, cur_size + gd_console_out_ovf()); + } else { + ut_fail(uts, __FILE__, __LINE__, __func__, + "Console record buffer too small - increase " + "CONFIG_CONSOLE_RECORD_OUT_SIZE"); + } return ret; } else if (ret == -ENOENT) { strcpy(uts->actual_str, "<no-more-output>"); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Include <mcheck.h> and remove the local declaration of mcheck_on_ramrelocation() to use the proper header file. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/board_f.c | 2 +- include/mcheck.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/common/board_f.c b/common/board_f.c index 879862865e1..a3e4c69d449 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -33,6 +33,7 @@ #include <log.h> #include <malloc.h> #include <mapmem.h> +#include <mcheck.h> #include <os.h> #include <post.h> #include <relocate.h> @@ -723,7 +724,6 @@ static int reloc_bloblist(void) return 0; } -void mcheck_on_ramrelocation(size_t offset); static int setup_reloc(void) { if (!(gd->flags & GD_FLG_SKIP_RELOC)) { diff --git a/include/mcheck.h b/include/mcheck.h index bd506ae6291..b170acf6281 100644 --- a/include/mcheck.h +++ b/include/mcheck.h @@ -48,4 +48,7 @@ void mcheck_check_all(void); */ enum mcheck_status mprobe(void *__ptr); +/* Called during RAM relocation to reset the heap registry */ +void mcheck_on_ramrelocation(size_t offset); + #endif -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The mcheck wrapper for realloc() unconditionally frees memory and returns NULL when size is 0. This differs from dlmalloc's default behaviour which returns a minimum-sized allocation unless REALLOC_ZERO_BYTES_FREES is defined. Make the mcheck wrapper respect the same REALLOC_ZERO_BYTES_FREES setting for consistent behavior with or without mcheck enabled. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 2 ++ test/common/malloc.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index c1c9d8a8938..b46be1899f0 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -5923,11 +5923,13 @@ void dlfree(void *mem) { dlfree_impl(mcheck_free_prehook(mem)); } void *dlrealloc(void *oldmem, size_t bytes) { mcheck_pedantic_prehook(); +#ifdef REALLOC_ZERO_BYTES_FREES if (bytes == 0) { if (oldmem) dlfree(oldmem); return NULL; } +#endif if (oldmem == NULL) return dlmalloc(bytes); diff --git a/test/common/malloc.c b/test/common/malloc.c index b114267dd83..9fdc1789645 100644 --- a/test/common/malloc.c +++ b/test/common/malloc.c @@ -178,7 +178,7 @@ COMMON_TEST(common_test_realloc_null, 0); /* * Test realloc() with zero size * - * Standard dlmalloc behavior (without REALLOC_ZERO_BYTES_FREES or mcheck): + * Standard dlmalloc behavior (without REALLOC_ZERO_BYTES_FREES): * realloc(ptr, 0) returns a minimum-sized allocation. */ static int common_test_realloc_zero(struct unit_test_state *uts) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> 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 | 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 <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); - - 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; } } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a new function backtrace_str() that returns a condensed backtrace string containing function names and line numbers separated by " <-". For example: "func_a:123 <-func_b:456 <-func_c:789" This is useful for logging and debugging where a compact representation of the call stack is needed. The depth is controlled by the new CONFIG_BACKTRACE_DEPTH option (default 3). Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- include/backtrace.h | 36 +++++++++++++ lib/Kconfig | 9 ++++ lib/backtrace.c | 122 +++++++++++++++++++++++++++++++++++++++++++ test/lib/backtrace.c | 36 +++++++++++++ 4 files changed, 203 insertions(+) diff --git a/include/backtrace.h b/include/backtrace.h index 7bb2ba68bec..7d9d8c1bb88 100644 --- a/include/backtrace.h +++ b/include/backtrace.h @@ -94,4 +94,40 @@ void backtrace_uninit(struct backtrace_ctx *ctx); */ int backtrace_show(void); +/** + * backtrace_strf() - get a condensed backtrace string into a buffer + * + * Return a string containing the last CONFIG_BACKTRACE_SUMMARY_FRAMES function names + * and line number, separated by ``<-``. + * + * For example: ``func_a:12 <-func_b:34 <-func_c:56`` + * + * @skip: number of stack frames to skip (0 to include backtrace_strf itself) + * @buf: buffer to write the string to + * @size: size of buffer + * Return: pointer to buf, or NULL on error + */ +char *backtrace_strf(unsigned int skip, char *buf, int size); + +/** + * backtrace_str() - get a condensed backtrace string + * + * Return a string containing the last CONFIG_BACKTRACE_SUMMARY_FRAMES function names + * and line number, separated by ``<-``. The string is statically allocated and + * will be overwritten on the next call. + * + * For example: ``func_a:12 <-func_b:34 <-func_c:56`` + * + * @skip: number of stack frames to skip (0 to include backtrace_str itself) + * Return: pointer to static string, or NULL on error + */ +#if CONFIG_IS_ENABLED(BACKTRACE) +const char *backtrace_str(unsigned int skip); +#else +static inline const char *backtrace_str(unsigned int skip) +{ + return NULL; +} +#endif + #endif /* __BACKTRACE_H */ diff --git a/lib/Kconfig b/lib/Kconfig index 79a75f98446..9c7eb27c392 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -36,6 +36,15 @@ config BACKTRACE stack. This is currently only available on sandbox. The backtrace command can be used to print the backtrace. +config BACKTRACE_SUMMARY_FRAMES + int "Number of frames in condensed backtrace" + depends on BACKTRACE + default 3 + help + Number of stack frames to include in the condensed backtrace + string returned by backtrace_str(). This affects BSS usage + since space must be allocated for the string. + config LIB_FORMAT_SIZE bool default y diff --git a/lib/backtrace.c b/lib/backtrace.c index b01a08af8ba..b657e8336ab 100644 --- a/lib/backtrace.c +++ b/lib/backtrace.c @@ -7,6 +7,7 @@ */ #include <backtrace.h> +#include <stdbool.h> #include <stdio.h> #include <string.h> @@ -54,3 +55,124 @@ int backtrace_show(void) return 0; } + +/** + * extract_func_info() - extract function name and line number from a symbol + * + * Parse a backtrace symbol string and extract function name with line number. + * The format is typically "func_name() at /path/to/file.c:line" or similar. + * + * @sym: symbol string from backtrace + * @buf: buffer to write "func_name:line" to + * @size: size of buffer + * Return: pointer to buf, or NULL if extraction failed + */ +static char *extract_func_info(const char *sym, char *buf, int size) +{ + const char *start, *end, *colon; + int len; + + if (!sym) + return NULL; + + /* + * Skip leading whitespace and any address prefix (e.g. "0x12345678 ") + * Look for the function name which ends at '+' or '(' or ' ' + */ + start = sym; + while (*start == ' ') + start++; + + /* Skip hex address if present */ + if (start[0] == '0' && start[1] == 'x') { + while (*start && *start != ' ') + start++; + while (*start == ' ') + start++; + } + + /* Find end of function name */ + end = start; + while (*end && *end != '+' && *end != '(' && *end != ' ') + end++; + + len = end - start; + if (len <= 0 || len >= size) + return NULL; + + memcpy(buf, start, len); + + /* Look for line number after last colon (file:line format) */ + colon = strrchr(sym, ':'); + if (colon && colon[1] >= '0' && colon[1] <= '9') { + buf[len++] = ':'; + colon++; + /* Copy digits */ + while (*colon >= '0' && *colon <= '9' && len < size - 1) + buf[len++] = *colon++; + } + buf[len] = '\0'; + + return buf; +} + +char *backtrace_strf(unsigned int skip, char *buf, int size) +{ + static struct backtrace_ctx ctx; + int remaining = size; + bool first = true; + char func[64]; + char *p = buf; + uint i, count; + int ret, len; + + /* skip + 1 to skip backtrace_strf() */ + ret = backtrace_init(&ctx, skip + 1); + if (ret < 0) + return NULL; + + ret = backtrace_get_syms(&ctx, NULL, 0); + if (ret) { + backtrace_uninit(&ctx); + return NULL; + } + + count = ctx.count; + if (count > CONFIG_BACKTRACE_SUMMARY_FRAMES) + count = CONFIG_BACKTRACE_SUMMARY_FRAMES; + + for (i = 0; i < count; i++) { + if (!extract_func_info(ctx.frame[i].sym, func, sizeof(func))) + continue; + + if (!first) { + if (remaining < 4) + break; + *p++ = ' '; + *p++ = '<'; + *p++ = '-'; + remaining -= 3; + } + first = false; + + len = strlen(func); + if (len >= remaining) + break; + memcpy(p, func, len); + p += len; + remaining -= len; + } + *p = '\0'; + + backtrace_uninit(&ctx); + + return buf; +} + +const char *backtrace_str(unsigned int skip) +{ + static char result[CONFIG_BACKTRACE_SUMMARY_FRAMES * 64]; + + /* skip + 1 to account for this wrapper function */ + return backtrace_strf(skip + 1, result, sizeof(result)); +} diff --git a/test/lib/backtrace.c b/test/lib/backtrace.c index 11f0d43ca7e..3f20b7854bf 100644 --- a/test/lib/backtrace.c +++ b/test/lib/backtrace.c @@ -46,3 +46,39 @@ static int lib_test_backtrace(struct unit_test_state *uts) return 0; } LIB_TEST(lib_test_backtrace, 0); + +/* Test backtrace_strf() and backtrace_str() */ +static int lib_test_backtrace_str(struct unit_test_state *uts) +{ + char pattern[128]; + char buf[256]; + const char *cstr; + char *str; + int line; + + /* Test backtrace_strf() with skip=1 to skip backtrace_strf() itself */ + line = __LINE__ + 1; + str = backtrace_strf(1, buf, sizeof(buf)); + ut_assertnonnull(str); + ut_asserteq_ptr(buf, str); + + printf("backtrace_strf: %s\n", str); + snprintf(pattern, sizeof(pattern), + "lib_test_backtrace_str:%d <-ut_run_test:\\d+ <-ut_run_test_live_flat:\\d+", + line); + ut_asserteq_regex(pattern, str); + + /* Test backtrace_str() */ + line = __LINE__ + 1; + cstr = backtrace_str(0); + ut_assertnonnull(cstr); + + printf("backtrace_str: %s\n", cstr); + snprintf(pattern, sizeof(pattern), + "lib_test_backtrace_str:%d <-ut_run_test:\\d+ <-ut_run_test_live_flat:\\d+", + line); + ut_asserteq_regex(pattern, cstr); + + return 0; +} +LIB_TEST(lib_test_backtrace_str, 0); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When CONFIG_MCHECK_HEAP_PROTECTION is enabled, dlmemalign_impl() calls dlmalloc_impl() directly since the mcheck wrapper handles alignment. This leaves internal_memalign() unused, causing a compiler warning. Guard internal_memalign() with a preprocessor check so it's only compiled when needed (mcheck disabled or MSPACES enabled). Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index b46be1899f0..efeff18653f 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -5309,6 +5309,7 @@ static mchunkptr try_realloc_chunk(mstate m, mchunkptr p, size_t nb, } #endif /* !defined(__UBOOT__) || !NO_REALLOC_IN_PLACE */ +#if !CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) || MSPACES static void* internal_memalign(mstate m, size_t alignment, size_t bytes) { void* mem = 0; if (alignment < MIN_CHUNK_SIZE) /* must be at least a minimum chunk size */ @@ -5422,6 +5423,7 @@ static void* internal_memalign(mstate m, size_t alignment, size_t bytes) { } return mem; } +#endif /* !CONFIG_MCHECK_HEAP_PROTECTION || MSPACES */ /* Common support for independent_X routines, handling -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add struct malloc_info and malloc_get_info() function to programmatically access the memory-allocation stats that malloc_stats() prints. The struct contains the size of the malloc() poll and the number of bytes in use. Add a static inline stub to return an error when DEBUG is not defined. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 10 ++++++++++ include/malloc.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index efeff18653f..004dafe4b3b 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -3689,6 +3689,16 @@ static struct mallinfo internal_mallinfo(mstate m) { } return nm; } + +int malloc_get_info(struct malloc_info *info) +{ + struct mallinfo mi = internal_mallinfo(gm); + + info->total_bytes = mem_malloc_end - mem_malloc_start; + info->in_use_bytes = mi.uordblks; + + return 0; +} #endif /* !NO_MALLINFO */ #if !NO_MALLOC_STATS diff --git a/include/malloc.h b/include/malloc.h index 73b2da0c383..fd8a8ddcfaf 100644 --- a/include/malloc.h +++ b/include/malloc.h @@ -665,8 +665,22 @@ void mspace_inspect_all(mspace msp, /* --------------------- U-Boot additions --------------------- */ #ifdef __UBOOT__ +#include <linux/errno.h> #include <linux/types.h> +/** + * struct malloc_info - Memory allocation statistics + * + * This is filled in by malloc_get_info(). + * + * @total_bytes: Total bytes available in the heap + * @in_use_bytes: Current bytes allocated (in use by application) + */ +struct malloc_info { + ulong total_bytes; + ulong in_use_bytes; +}; + /* Memory pool boundaries */ extern ulong mem_malloc_start; extern ulong mem_malloc_end; @@ -735,6 +749,21 @@ void *memalign_simple(size_t alignment, size_t bytes); */ int initf_malloc(void); +/** + * malloc_get_info() - Get memory allocation statistics + * + * @info: Place to put the statistics + * Return: 0 on success, -ENOSYS if not available (DEBUG not defined) + */ +#ifdef DEBUG +int malloc_get_info(struct malloc_info *info); +#else +static inline int malloc_get_info(struct malloc_info *info) +{ + return -ENOSYS; +} +#endif + #endif /* __UBOOT__ */ #ifdef __cplusplus -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a new CONFIG_MALLOC_DEBUG option to control malloc() debugging features. This replaces the direct UNIT_TEST check and allows enabling malloc debugging independently of whether UNIT_TEST is enabled. The option defaults to y when UNIT_TEST is enabled, preserving existing behaviour. Disable for RISC-V since it seems to have a size limitation. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- Kconfig | 8 ++++++++ common/dlmalloc.c | 2 +- include/malloc.h | 4 ++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Kconfig b/Kconfig index 378ecfb1867..86276c89f38 100644 --- a/Kconfig +++ b/Kconfig @@ -337,6 +337,14 @@ config SYS_MALLOC_LEN This defines memory to be allocated for Dynamic allocation TODO: Use for other architectures +config MALLOC_DEBUG + bool "Enable malloc debugging" + default y if UNIT_TEST && !CPU_RISCV + help + Enable debugging features in the malloc implementation. This + enables additional assertions and the malloc_get_info() function + to retrieve memory-allocation statistics. + config SPL_SYS_MALLOC_F bool "Enable malloc() pool in SPL" depends on SPL_FRAMEWORK && SYS_MALLOC_F && SPL diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 004dafe4b3b..b5dc2b13dc6 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -568,7 +568,7 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP #ifdef __UBOOT__ -#if CONFIG_IS_ENABLED(UNIT_TEST) +#if CONFIG_IS_ENABLED(MALLOC_DEBUG) #define DEBUG 1 #endif diff --git a/include/malloc.h b/include/malloc.h index fd8a8ddcfaf..7e276d444ab 100644 --- a/include/malloc.h +++ b/include/malloc.h @@ -753,9 +753,9 @@ int initf_malloc(void); * malloc_get_info() - Get memory allocation statistics * * @info: Place to put the statistics - * Return: 0 on success, -ENOSYS if not available (DEBUG not defined) + * Return: 0 on success, -ENOSYS if not available (MALLOC_DEBUG not enabled) */ -#ifdef DEBUG +#if CONFIG_IS_ENABLED(MALLOC_DEBUG) int malloc_get_info(struct malloc_info *info); #else static inline int malloc_get_info(struct malloc_info *info) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When debugging or running unit tests it is helpful to have information available from the malloc subsystem. Enable these features in those cases. Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index b5dc2b13dc6..b0845e6ea7c 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -599,12 +599,12 @@ static inline void MALLOC_COPY(void *dest, const void *src, size_t sz) { memcpy( #define MORECORE_CONTIGUOUS 1 #define MORECORE_CANNOT_TRIM 1 #define MORECORE_CLEARS 1 -#define NO_MALLOC_STATS 1 #define USE_LOCKS 0 #define USE_SPIN_LOCKS 0 -#if !CONFIG_IS_ENABLED(UNIT_TEST) +#if !CONFIG_IS_ENABLED(UNIT_TEST) && !IS_ENABLED(CONFIG_MALLOC_DEBUG) #define NO_MALLINFO 1 +#define NO_MALLOC_STATS 1 #endif #if !CONFIG_IS_ENABLED(SANDBOX) #define INSECURE 1 @@ -3690,6 +3690,9 @@ static struct mallinfo internal_mallinfo(mstate m) { return nm; } +#endif /* !NO_MALLINFO */ + +#if CONFIG_IS_ENABLED(MALLOC_DEBUG) int malloc_get_info(struct malloc_info *info) { struct mallinfo mi = internal_mallinfo(gm); @@ -3699,7 +3702,7 @@ int malloc_get_info(struct malloc_info *info) return 0; } -#endif /* !NO_MALLINFO */ +#endif #if !NO_MALLOC_STATS static void internal_malloc_stats(mstate m) { -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a command to display malloc heap statistics, showing total heap size and memory currently in use. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- cmd/Kconfig | 9 ++++++++ cmd/Makefile | 1 + cmd/malloc.c | 34 ++++++++++++++++++++++++++++++ doc/usage/cmd/malloc.rst | 45 ++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + test/cmd/Makefile | 1 + test/cmd/malloc.c | 30 +++++++++++++++++++++++++++ 7 files changed, 121 insertions(+) create mode 100644 cmd/malloc.c create mode 100644 doc/usage/cmd/malloc.rst create mode 100644 test/cmd/malloc.c diff --git a/cmd/Kconfig b/cmd/Kconfig index ff5f6f85144..43e12e413f8 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -3018,6 +3018,15 @@ config CMD_LOG maximum log level for emitting of records). It also provides access to a command used for testing the log system. +config CMD_MALLOC + bool "malloc - Show malloc statistics" + depends on MALLOC_DEBUG + default y + help + This provides access to malloc information. It shows statistics + about memory allocation, such as total memory allocated and + currently in use. + config CMD_MOUSE bool "mouse - Show mouse input" default y if MOUSE diff --git a/cmd/Makefile b/cmd/Makefile index ebf66ea0d3c..c010da8dc3b 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -113,6 +113,7 @@ obj-$(CONFIG_CMD_LED) += led.o obj-$(CONFIG_CMD_LICENSE) += license.o obj-y += load.o obj-$(CONFIG_CMD_LOG) += log.o +obj-$(CONFIG_CMD_MALLOC) += malloc.o obj-$(CONFIG_CMD_LSBLK) += lsblk.o obj-$(CONFIG_CMD_MD5SUM) += md5sum.o obj-$(CONFIG_CMD_MEMORY) += mem.o diff --git a/cmd/malloc.c b/cmd/malloc.c new file mode 100644 index 00000000000..cb2fa34155b --- /dev/null +++ b/cmd/malloc.c @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * malloc command - show malloc information + * + * Copyright 2025 Canonical Ltd + * Written by Simon Glass <simon.glass@canonical.com> + */ + +#include <command.h> +#include <display_options.h> +#include <malloc.h> + +static int do_malloc_info(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct malloc_info info; + char buf[12]; + int ret; + + ret = malloc_get_info(&info); + if (ret) + return CMD_RET_FAILURE; + + printf("total bytes = %s\n", format_size(buf, info.total_bytes)); + printf("in use bytes = %s\n", format_size(buf, info.in_use_bytes)); + + return 0; +} + +U_BOOT_LONGHELP(malloc, + "info - display malloc statistics\n"); + +U_BOOT_CMD_WITH_SUBCMDS(malloc, "malloc information", malloc_help_text, + U_BOOT_SUBCMD_MKENT(info, 1, 1, do_malloc_info)); diff --git a/doc/usage/cmd/malloc.rst b/doc/usage/cmd/malloc.rst new file mode 100644 index 00000000000..d21aa6c1421 --- /dev/null +++ b/doc/usage/cmd/malloc.rst @@ -0,0 +1,45 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +.. index:: + single: malloc (command) + +malloc command +============== + +Synopsis +-------- + +:: + + malloc info + +Description +----------- + +The malloc command shows information about the malloc heap. + +info + Shows memory-allocation statistics, including the total heap size and the + amount currently in use. + +The total heap size is set by ``CONFIG_SYS_MALLOC_LEN``. + +Example +------- + +:: + + => malloc info + total bytes = 96 MiB + in use bytes = 700.9 KiB + +Configuration +------------- + +The malloc command is enabled by CONFIG_CMD_MALLOC which depends on +CONFIG_MALLOC_DEBUG. + +Return value +------------ + +The return value $? is 0 (true) on success, 1 (false) on failure. diff --git a/doc/usage/index.rst b/doc/usage/index.rst index bee7884a066..7ed79f89af6 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -97,6 +97,7 @@ Shell commands cmd/loadx cmd/loady cmd/luks + cmd/malloc cmd/meminfo cmd/mbr cmd/md diff --git a/test/cmd/Makefile b/test/cmd/Makefile index c43aefb4eb3..14cbdc3a6e6 100644 --- a/test/cmd/Makefile +++ b/test/cmd/Makefile @@ -26,6 +26,7 @@ endif obj-$(CONFIG_CMD_HASH) += hash.o obj-$(CONFIG_CMD_HISTORY) += history.o obj-$(CONFIG_CMD_LOADM) += loadm.o +obj-$(CONFIG_CMD_MALLOC) += malloc.o ifdef CONFIG_SANDBOX obj-$(CONFIG_CMD_MEMINFO) += meminfo.o endif diff --git a/test/cmd/malloc.c b/test/cmd/malloc.c new file mode 100644 index 00000000000..6cd52b68900 --- /dev/null +++ b/test/cmd/malloc.c @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test for 'malloc' command + * + * Copyright 2025 Canonical Ltd + * Written by Simon Glass <simon.glass@canonical.com> + */ + +#include <malloc.h> +#include <dm/test.h> +#include <test/cmd.h> +#include <test/ut.h> + +/* Test 'malloc info' command */ +static int cmd_test_malloc_info(struct unit_test_state *uts) +{ + struct malloc_info info; + + ut_assertok(malloc_get_info(&info)); + ut_assert(info.total_bytes >= CONFIG_SYS_MALLOC_LEN); + ut_assert(info.in_use_bytes < info.total_bytes); + + ut_assertok(run_command("malloc info", 0)); + ut_assert_nextlinen("total bytes = "); + ut_assert_nextlinen("in use bytes = "); + ut_assert_console_end(); + + return 0; +} +CMD_TEST(cmd_test_malloc_info, UTF_CONSOLE); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add counters to track the number of calls to malloc(), free(), and realloc(). These are displayed by the 'malloc info' command and accessible via malloc_get_info(). Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- cmd/malloc.c | 7 +++++-- common/dlmalloc.c | 18 ++++++++++++++++++ doc/usage/cmd/malloc.rst | 12 ++++++++---- include/malloc.h | 6 ++++++ test/cmd/malloc.c | 4 ++++ 5 files changed, 41 insertions(+), 6 deletions(-) diff --git a/cmd/malloc.c b/cmd/malloc.c index cb2fa34155b..3750a16158b 100644 --- a/cmd/malloc.c +++ b/cmd/malloc.c @@ -21,8 +21,11 @@ static int do_malloc_info(struct cmd_tbl *cmdtp, int flag, int argc, if (ret) return CMD_RET_FAILURE; - printf("total bytes = %s\n", format_size(buf, info.total_bytes)); - printf("in use bytes = %s\n", format_size(buf, info.in_use_bytes)); + printf("total bytes = %s\n", format_size(buf, info.total_bytes)); + printf("in use bytes = %s\n", format_size(buf, info.in_use_bytes)); + printf("malloc count = %lu\n", info.malloc_count); + printf("free count = %lu\n", info.free_count); + printf("realloc count = %lu\n", info.realloc_count); return 0; } diff --git a/common/dlmalloc.c b/common/dlmalloc.c index b0845e6ea7c..b8de42cc47e 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -646,6 +646,12 @@ ulong mem_malloc_start; ulong mem_malloc_end; ulong mem_malloc_brk; +#ifndef NO_MALLOC_STATS +static ulong malloc_count; +static ulong free_count; +static ulong realloc_count; +#endif /* !NO_MALLOC_STATS */ + #endif /* __UBOOT__ */ #ifndef SMALLCHUNKS_AS_FUNCS @@ -3699,6 +3705,9 @@ int malloc_get_info(struct malloc_info *info) info->total_bytes = mem_malloc_end - mem_malloc_start; info->in_use_bytes = mi.uordblks; + info->malloc_count = malloc_count; + info->free_count = free_count; + info->realloc_count = realloc_count; return 0; } @@ -4926,6 +4935,9 @@ void* dlmalloc_impl(size_t bytes) { if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) return malloc_simple(bytes); #endif +#if !NO_MALLOC_STATS + malloc_count++; +#endif /* Return NULL if not initialized yet */ if (!mem_malloc_start && !mem_malloc_end) @@ -5092,6 +5104,9 @@ void* dlmalloc_impl(size_t bytes) { STATIC_IF_MCHECK void dlfree_impl(void* mem) { #ifdef __UBOOT__ +#if !NO_MALLOC_STATS + free_count++; +#endif #if CONFIG_IS_ENABLED(SYS_MALLOC_F) /* free() is a no-op - all the memory will be freed on relocation */ if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { @@ -5662,6 +5677,9 @@ static void internal_inspect_all(mstate m, STATIC_IF_MCHECK void* dlrealloc_impl(void* oldmem, size_t bytes) { #ifdef __UBOOT__ +#if !NO_MALLOC_STATS + realloc_count++; +#endif #if CONFIG_IS_ENABLED(SYS_MALLOC_F) if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { /* This is harder to support and should not be needed */ diff --git a/doc/usage/cmd/malloc.rst b/doc/usage/cmd/malloc.rst index d21aa6c1421..7b08b358258 100644 --- a/doc/usage/cmd/malloc.rst +++ b/doc/usage/cmd/malloc.rst @@ -19,8 +19,9 @@ Description The malloc command shows information about the malloc heap. info - Shows memory-allocation statistics, including the total heap size and the - amount currently in use. + Shows memory-allocation statistics, including the total heap size, the + amount currently in use, and call counts for malloc(), free(), and + realloc(). The total heap size is set by ``CONFIG_SYS_MALLOC_LEN``. @@ -30,8 +31,11 @@ Example :: => malloc info - total bytes = 96 MiB - in use bytes = 700.9 KiB + total bytes = 96 MiB + in use bytes = 700.9 KiB + malloc count = 1234 + free count = 567 + realloc count = 89 Configuration ------------- diff --git a/include/malloc.h b/include/malloc.h index 7e276d444ab..0d8a2a43f2a 100644 --- a/include/malloc.h +++ b/include/malloc.h @@ -675,10 +675,16 @@ void mspace_inspect_all(mspace msp, * * @total_bytes: Total bytes available in the heap * @in_use_bytes: Current bytes allocated (in use by application) + * @malloc_count: Number of calls to malloc() + * @free_count: Number of calls to free() + * @realloc_count: Number of calls to realloc() */ struct malloc_info { ulong total_bytes; ulong in_use_bytes; + ulong malloc_count; + ulong free_count; + ulong realloc_count; }; /* Memory pool boundaries */ diff --git a/test/cmd/malloc.c b/test/cmd/malloc.c index 6cd52b68900..f9d41a36cad 100644 --- a/test/cmd/malloc.c +++ b/test/cmd/malloc.c @@ -19,10 +19,14 @@ static int cmd_test_malloc_info(struct unit_test_state *uts) ut_assertok(malloc_get_info(&info)); ut_assert(info.total_bytes >= CONFIG_SYS_MALLOC_LEN); ut_assert(info.in_use_bytes < info.total_bytes); + ut_assert(info.malloc_count > 0); ut_assertok(run_command("malloc info", 0)); ut_assert_nextlinen("total bytes = "); ut_assert_nextlinen("in use bytes = "); + ut_assert_nextlinen("malloc count = "); + ut_assert_nextlinen("free count = "); + ut_assert_nextlinen("realloc count = "); ut_assert_console_end(); return 0; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add CONFIG_MCHECK_HEAP_PROTECTION option to enable mcheck heap protection. Convert all uses of MCHECK_HEAP_PROTECTION to use the CONFIG_ -prefixed version to work with Kconfig. Disable this option when tracing is enabled, since the mcheck hooks (mcheck_pedantic_prehook(), etc.) interfere with function tracing. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- Kconfig | 10 ++++++++++ common/board_f.c | 2 +- common/dlmalloc.c | 6 +++--- common/mcheck_core.inc.h | 6 +++--- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/Kconfig b/Kconfig index 86276c89f38..fb320fdb418 100644 --- a/Kconfig +++ b/Kconfig @@ -345,6 +345,16 @@ config MALLOC_DEBUG enables additional assertions and the malloc_get_info() function to retrieve memory-allocation statistics. +config MCHECK_HEAP_PROTECTION + bool "Enable mcheck heap protection" + depends on !TRACE + help + Enable heap protection using the mcheck library. This adds canary + values before and after each allocation to detect buffer overflows + and underflows, double-frees, and memory corruption. This + significantly increases memory overhead and should only be used for + debugging. + config SPL_SYS_MALLOC_F bool "Enable malloc() pool in SPL" depends on SPL_FRAMEWORK && SYS_MALLOC_F && SPL diff --git a/common/board_f.c b/common/board_f.c index a3e4c69d449..9dce08002c5 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -749,7 +749,7 @@ static int setup_reloc(void) if (gd->flags & GD_FLG_SKIP_RELOC) { debug("Skipping relocation due to flag\n"); } else { -#ifdef MCHECK_HEAP_PROTECTION +#ifdef CONFIG_MCHECK_HEAP_PROTECTION mcheck_on_ramrelocation(gd->reloc_off); #endif debug("Relocation Offset is: %08lx\n", gd->reloc_off); diff --git a/common/dlmalloc.c b/common/dlmalloc.c index b8de42cc47e..7258a7dda84 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -572,7 +572,7 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP #define DEBUG 1 #endif -#ifdef MCHECK_HEAP_PROTECTION +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) #define STATIC_IF_MCHECK static #undef MALLOC_COPY #undef MALLOC_ZERO @@ -5451,7 +5451,7 @@ static void* internal_memalign(mstate m, size_t alignment, size_t bytes) { } return mem; } -#endif /* !CONFIG_MCHECK_HEAP_PROTECTION || MSPACES */ +#endif /* !MCHECK_HEAP_PROTECTION || MSPACES */ /* Common support for independent_X routines, handling @@ -5937,7 +5937,7 @@ size_t dlmalloc_usable_size(const void* mem) { return 0; } -#ifdef MCHECK_HEAP_PROTECTION +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) #include "mcheck_core.inc.h" void *dlmalloc(size_t bytes) diff --git a/common/mcheck_core.inc.h b/common/mcheck_core.inc.h index 69021409922..7caa9ac8dff 100644 --- a/common/mcheck_core.inc.h +++ b/common/mcheck_core.inc.h @@ -45,8 +45,8 @@ * an array, for index(+1/-1) errors. * * U-Boot is a BL, not an OS with a lib. Activity of the library is set not in runtime, - * rather in compile-time, by MCHECK_HEAP_PROTECTION macro. That guarantees that - * we haven't missed first malloc. + * rather in compile-time, by CONFIG_MCHECK_HEAP_PROTECTION macro. That + * guarantees that we haven't missed first malloc. */ /* @@ -59,7 +59,7 @@ #define _MCHECKCORE_INC_H 1 #include "mcheck.h" -#if defined(MCHECK_HEAP_PROTECTION) +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) #define mcheck_flood memset // these are from /dev/random: -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Move the CONFIG_MCHECK_HEAP_PROTECTION block after the standard includes so that size_t and string functions are available for the inline MALLOC_ZERO and MALLOC_COPY functions. Add the <string.h> and <linux/types.h> includes needed for mcheck. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 7258a7dda84..f59b1151606 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -572,21 +572,6 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP #define DEBUG 1 #endif -#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) -#define STATIC_IF_MCHECK static -#undef MALLOC_COPY -#undef MALLOC_ZERO -static inline void MALLOC_ZERO(void *p, size_t sz) { memset(p, 0, sz); } -static inline void MALLOC_COPY(void *dest, const void *src, size_t sz) { memcpy(dest, src, sz); } -#else -#define STATIC_IF_MCHECK -#define dlmalloc_impl dlmalloc -#define dlfree_impl dlfree -#define dlrealloc_impl dlrealloc -#define dlmemalign_impl dlmemalign -#define dlcalloc_impl dlcalloc -#endif - #define LACKS_FCNTL_H #define LACKS_UNISTD_H #define LACKS_SYS_PARAM_H @@ -633,12 +618,29 @@ static inline void MALLOC_COPY(void *dest, const void *src, size_t sz) { memcpy( #include <log.h> #include <malloc.h> #include <mapmem.h> +#include <string.h> #include <vsprintf.h> #include <asm/global_data.h> +#include <linux/types.h> #include <valgrind/memcheck.h> DECLARE_GLOBAL_DATA_PTR; +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) +#define STATIC_IF_MCHECK static +#undef MALLOC_COPY +#undef MALLOC_ZERO +static inline void MALLOC_ZERO(void *p, size_t sz) { memset(p, 0, sz); } +static inline void MALLOC_COPY(void *dest, const void *src, size_t sz) { memcpy(dest, src, sz); } +#else +#define STATIC_IF_MCHECK +#define dlmalloc_impl dlmalloc +#define dlfree_impl dlfree +#define dlrealloc_impl dlrealloc +#define dlmemalign_impl dlmemalign +#define dlcalloc_impl dlcalloc +#endif + static bool malloc_testing; /* enable test mode */ static int malloc_max_allocs; /* return NULL after this many calls to malloc() */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> With mcheck enabled, internal_malloc and internal_free go through mcheck wrappers which expect user pointers (after mcheck header), not raw allocator pointers. Also, dlmemalign_impl() needs a properly aligned base pointer for mcheck to place its header correctly. Use _impl functions directly for internal_malloc/internal_free. Call internal_memalign() for alignments greater than MALLOC_ALIGNMENT. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index f59b1151606..bcdb4d29424 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -4092,8 +4092,17 @@ static void unlink_large_chunk(mstate M, tchunkptr X) { #define internal_free(m, mem)\ if (m == gm) dlfree(mem); else mspace_free(m,mem); #else /* MSPACES */ +/* + * When mcheck is enabled, internal calls must use the _impl functions + * to avoid going through the mcheck wrappers which expect user pointers. + */ +#ifdef CONFIG_MCHECK_HEAP_PROTECTION +#define internal_malloc(m, b) dlmalloc_impl(b) +#define internal_free(m, mem) dlfree_impl(mem) +#else #define internal_malloc(m, b) dlmalloc(b) #define internal_free(m, mem) dlfree(mem) +#endif #endif /* MSPACES */ #endif /* ONLY_MSPACES */ @@ -5339,7 +5348,6 @@ static mchunkptr try_realloc_chunk(mstate m, mchunkptr p, size_t nb, } #endif /* !defined(__UBOOT__) || !NO_REALLOC_IN_PLACE */ -#if !CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) || MSPACES static void* internal_memalign(mstate m, size_t alignment, size_t bytes) { void* mem = 0; if (alignment < MIN_CHUNK_SIZE) /* must be at least a minimum chunk size */ @@ -5453,7 +5461,6 @@ static void* internal_memalign(mstate m, size_t alignment, size_t bytes) { } return mem; } -#endif /* !MCHECK_HEAP_PROTECTION || MSPACES */ /* Common support for independent_X routines, handling @@ -5808,9 +5815,13 @@ void* dlmemalign_impl(size_t alignment, size_t bytes) { return memalign_simple(alignment, bytes); #endif #endif - if (alignment <= MALLOC_ALIGNMENT) { + /* + * With mcheck, the wrapper handles alignment via mcheck_memalign_prehook() + * which adds space for the header aligned to the requested alignment. + * The base pointer must still be properly aligned for this to work. + */ + if (alignment <= MALLOC_ALIGNMENT) return dlmalloc_impl(bytes); - } return internal_memalign(gm, alignment, bytes); } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Fix dlposix_memalign(), dlvalloc(), and dlpvalloc() to call dlmemalign() instead of dlmemalign_impl() or internal_memalign() directly. This ensures these functions go through the mcheck wrappers when CONFIG_MCHECK_HEAP_PROTECTION is enabled. Without this fix, internal_memalign is undefined when mcheck is enabled, causing a build error. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index bcdb4d29424..e288d94d433 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -4096,7 +4096,7 @@ static void unlink_large_chunk(mstate M, tchunkptr X) { * When mcheck is enabled, internal calls must use the _impl functions * to avoid going through the mcheck wrappers which expect user pointers. */ -#ifdef CONFIG_MCHECK_HEAP_PROTECTION +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) #define internal_malloc(m, b) dlmalloc_impl(b) #define internal_free(m, mem) dlfree_impl(mem) #else @@ -5827,39 +5827,32 @@ void* dlmemalign_impl(size_t alignment, size_t bytes) { int dlposix_memalign(void** pp, size_t alignment, size_t bytes) { void* mem = 0; - if (alignment == MALLOC_ALIGNMENT) - mem = dlmalloc_impl(bytes); - else { - size_t d = alignment / sizeof(void*); - size_t r = alignment % sizeof(void*); - if (r != 0 || d == 0 || (d & (d-SIZE_T_ONE)) != 0) - return EINVAL; - else if (bytes <= MAX_REQUEST - alignment) { - if (alignment < MIN_CHUNK_SIZE) - alignment = MIN_CHUNK_SIZE; - mem = internal_memalign(gm, alignment, bytes); - } - } + size_t d = alignment / sizeof(void*); + size_t r = alignment % sizeof(void*); + + if (r != 0 || d == 0 || (d & (d-SIZE_T_ONE)) != 0) + return EINVAL; + if (bytes > MAX_REQUEST - alignment) + return ENOMEM; + mem = dlmemalign(alignment, bytes); if (mem == 0) return ENOMEM; - else { - *pp = mem; - return 0; - } + *pp = mem; + return 0; } void* dlvalloc(size_t bytes) { size_t pagesz; ensure_initialization(); pagesz = mparams.page_size; - return dlmemalign_impl(pagesz, bytes); + return dlmemalign(pagesz, bytes); } void* dlpvalloc(size_t bytes) { size_t pagesz; ensure_initialization(); pagesz = mparams.page_size; - return dlmemalign_impl(pagesz, (bytes + pagesz - SIZE_T_ONE) & ~(pagesz - SIZE_T_ONE)); + return dlmemalign(pagesz, (bytes + pagesz - SIZE_T_ONE) & ~(pagesz - SIZE_T_ONE)); } void** dlindependent_calloc(size_t n_elements, size_t elem_size, -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The mcheck registry tracks concurrent allocations for pedantic checking. The current size of 6608 entries is insufficient when running the full pytest suite, causing registry overflow warnings. Increase REGISTRY_SZ to 12000 to handle the full test suite. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/mcheck_core.inc.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/mcheck_core.inc.h b/common/mcheck_core.inc.h index 7caa9ac8dff..2839027564e 100644 --- a/common/mcheck_core.inc.h +++ b/common/mcheck_core.inc.h @@ -70,8 +70,8 @@ #define FREEFLOOD ((char)0xf5) #define PADDINGFLOOD ((char)0x58) -// my normal run demands 4427-6449 chunks: -#define REGISTRY_SZ 6608 +// Full test suite can exceed 10000 concurrent allocations +#define REGISTRY_SZ 12000 #define CANARY_DEPTH 2 // avoid problems with BSS at early stage: -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Enable CONFIG_MCHECK_HEAP_PROTECTION for the sandbox board to detect heap corruption during development and testing. Increase the heap size from 96 MiB to 128 MiB to account for the additional overhead of mcheck metadata (~48 bytes per allocation). Increase the console-out buffer to 128K to allow space for the new malloc dump, when checked in tests. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- configs/sandbox_defconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 44185ccb7eb..b1b2e725b4c 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -1,5 +1,5 @@ CONFIG_TEXT_BASE=0 -CONFIG_SYS_MALLOC_LEN=0x6000000 +CONFIG_SYS_MALLOC_LEN=0x8000000 CONFIG_BLOBLIST_SIZE_RELOC=0x20000 CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x2000 @@ -12,6 +12,7 @@ CONFIG_DEBUG_UART=y CONFIG_SYS_MEMTEST_START=0x00100000 CONFIG_SYS_MEMTEST_END=0x00101000 CONFIG_ULIB=y +CONFIG_MCHECK_HEAP_PROTECTION=y CONFIG_EXAMPLES=y CONFIG_EFI_SECURE_BOOT=y CONFIG_EFI_RT_VOLATILE_STORE=y @@ -49,7 +50,7 @@ CONFIG_IMAGE_PRE_LOAD=y CONFIG_IMAGE_PRE_LOAD_SIG=y CONFIG_CEDIT=y CONFIG_CONSOLE_RECORD=y -CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 +CONFIG_CONSOLE_RECORD_OUT_SIZE=0x20000 CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_LOG=y CONFIG_LOG_MAX_LEVEL=9 -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Pre-relocation heap allocations are expected to be discarded during relocation - this is normal behavior, not a warning condition. Remove the verbose messages that were printed for each chunk. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/mcheck_core.inc.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/mcheck_core.inc.h b/common/mcheck_core.inc.h index 2839027564e..c2089add0d6 100644 --- a/common/mcheck_core.inc.h +++ b/common/mcheck_core.inc.h @@ -289,14 +289,14 @@ static void mcheck_initialize(mcheck_abortfunc_t new_func, char pedantic_flag) void mcheck_on_ramrelocation(size_t offset) { - char *p; int i; - // Simple, but inaccurate strategy: drop the pre-reloc heap + + /* + * Pre-relocation heap allocations are no longer valid after + * relocation. Clear the registry since we can't track them. + */ for (i = 0; i < REGISTRY_SZ; ++i) - if ((p = mcheck_registry[i]) != NULL ) { - printf("mcheck, WRN: forgetting %p chunk\n", p); - mcheck_registry[i] = 0; - } + mcheck_registry[i] = 0; mcheck_chunk_count = 0; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a field to store the caller for each allocation, using the backtrace information. This can be useful when debugging heap corruption. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 8 ++++---- common/mcheck_core.inc.h | 28 +++++++++++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index e288d94d433..706c12d9b9a 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -5954,7 +5954,7 @@ void *dlmalloc(size_t bytes) if (!p) return p; - return mcheck_alloc_posthook(p, bytes); + return mcheck_alloc_posthook(p, bytes, NULL); } void dlfree(void *mem) { dlfree_impl(mcheck_free_prehook(mem)); } @@ -5979,7 +5979,7 @@ void *dlrealloc(void *oldmem, size_t bytes) p = dlrealloc_impl(p, newsz); if (!p) return p; - return mcheck_alloc_noclean_posthook(p, bytes); + return mcheck_alloc_noclean_posthook(p, bytes, NULL); } void *dlmemalign(size_t alignment, size_t bytes) @@ -5990,7 +5990,7 @@ void *dlmemalign(size_t alignment, size_t bytes) if (!p) return p; - return mcheck_memalign_posthook(alignment, p, bytes); + return mcheck_memalign_posthook(alignment, p, bytes, NULL); } /* dlpvalloc, dlvalloc redirect to dlmemalign, so they need no wrapping */ @@ -6004,7 +6004,7 @@ void *dlcalloc(size_t n, size_t elem_size) if (!p) return p; - return mcheck_alloc_noclean_posthook(p, n * elem_size); + return mcheck_alloc_noclean_posthook(p, n * elem_size, NULL); } /* mcheck API */ diff --git a/common/mcheck_core.inc.h b/common/mcheck_core.inc.h index c2089add0d6..63dbeaa5f56 100644 --- a/common/mcheck_core.inc.h +++ b/common/mcheck_core.inc.h @@ -73,6 +73,7 @@ // Full test suite can exceed 10000 concurrent allocations #define REGISTRY_SZ 12000 #define CANARY_DEPTH 2 +#define MCHECK_CALLER_LEN 48 // avoid problems with BSS at early stage: static char mcheck_pedantic_flag __section(".data") = 0; @@ -88,6 +89,7 @@ struct mcheck_hdr { size_t size; /* Exact size requested by user. */ size_t aln_skip; /* Ignored bytes, before the mcheck_hdr, to fulfill alignment */ mcheck_canary canary; /* Magic number to check header integrity. */ + char caller[MCHECK_CALLER_LEN]; /* caller info for debugging */ }; static void mcheck_default_abort(enum mcheck_status status, const void *p) @@ -196,7 +198,8 @@ static size_t mcheck_alloc_prehook(size_t sz) } static void *mcheck_allocated_helper(void *altoghether_ptr, size_t customer_sz, - size_t alignment, int clean_content) + size_t alignment, int clean_content, + const char *caller) { const size_t slop = alignment ? mcheck_evaluate_memalign_prefix_size(alignment) - sizeof(struct mcheck_hdr) : 0; @@ -207,6 +210,10 @@ static void *mcheck_allocated_helper(void *altoghether_ptr, size_t customer_sz, hdr->aln_skip = slop; for (i = 0; i < CANARY_DEPTH; ++i) hdr->canary.elems[i] = MAGICWORD; + if (caller) + strlcpy(hdr->caller, caller, MCHECK_CALLER_LEN); + else + hdr->caller[0] = '\0'; char *payload = (char *)&hdr[1]; @@ -239,14 +246,19 @@ static void *mcheck_allocated_helper(void *altoghether_ptr, size_t customer_sz, return payload; } -static void *mcheck_alloc_posthook(void *altoghether_ptr, size_t customer_sz) +static void *mcheck_alloc_posthook(void *altoghether_ptr, size_t customer_sz, + const char *caller) { - return mcheck_allocated_helper(altoghether_ptr, customer_sz, ANY_ALIGNMENT, CLEAN_CONTENT); + return mcheck_allocated_helper(altoghether_ptr, customer_sz, + ANY_ALIGNMENT, CLEAN_CONTENT, caller); } -static void *mcheck_alloc_noclean_posthook(void *altoghether_ptr, size_t customer_sz) +static void *mcheck_alloc_noclean_posthook(void *altoghether_ptr, + size_t customer_sz, + const char *caller) { - return mcheck_allocated_helper(altoghether_ptr, customer_sz, ANY_ALIGNMENT, KEEP_CONTENT); + return mcheck_allocated_helper(altoghether_ptr, customer_sz, + ANY_ALIGNMENT, KEEP_CONTENT, caller); } static size_t mcheck_memalign_prehook(size_t alig, size_t sz) @@ -254,9 +266,11 @@ static size_t mcheck_memalign_prehook(size_t alig, size_t sz) return mcheck_evaluate_memalign_prefix_size(alig) + sz + sizeof(mcheck_canary); } -static void *mcheck_memalign_posthook(size_t alignment, void *altoghether_ptr, size_t customer_sz) +static void *mcheck_memalign_posthook(size_t alignment, void *altoghether_ptr, + size_t customer_sz, const char *caller) { - return mcheck_allocated_helper(altoghether_ptr, customer_sz, alignment, CLEAN_CONTENT); + return mcheck_allocated_helper(altoghether_ptr, customer_sz, alignment, + CLEAN_CONTENT, caller); } static enum mcheck_status mcheck_mprobe(void *ptr) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When CONFIG_MALLOC_DEBUG is enabled, add an optional caller string parameter to dlmalloc_impl(). This allows tracking where allocations originate from when debugging memory issues. The CALLER_PARAM, CALLER_ARG, and CALLER_NULL macros hide the parameter when MALLOC_DEBUG is not enabled, keeping the non-debug code path clean. When MALLOC_DEBUG is enabled (with or without MCHECK), the _impl functions must be separate from the public API functions since they have different signatures. Add simple pass-through wrappers for this case. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 54 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 706c12d9b9a..420bed1c480 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -570,6 +570,13 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP #if CONFIG_IS_ENABLED(MALLOC_DEBUG) #define DEBUG 1 +#define CALLER_PARAM , const char *caller +#define CALLER_ARG , caller +#define CALLER_NULL , NULL +#else +#define CALLER_PARAM +#define CALLER_ARG +#define CALLER_NULL #endif #define LACKS_FCNTL_H @@ -626,12 +633,14 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP DECLARE_GLOBAL_DATA_PTR; -#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) || CONFIG_IS_ENABLED(MALLOC_DEBUG) #define STATIC_IF_MCHECK static +#ifdef CONFIG_MCHECK_HEAP_PROTECTION #undef MALLOC_COPY #undef MALLOC_ZERO static inline void MALLOC_ZERO(void *p, size_t sz) { memset(p, 0, sz); } static inline void MALLOC_COPY(void *dest, const void *src, size_t sz) { memcpy(dest, src, sz); } +#endif #else #define STATIC_IF_MCHECK #define dlmalloc_impl dlmalloc @@ -4097,7 +4106,7 @@ static void unlink_large_chunk(mstate M, tchunkptr X) { * to avoid going through the mcheck wrappers which expect user pointers. */ #if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) -#define internal_malloc(m, b) dlmalloc_impl(b) +#define internal_malloc(m, b) dlmalloc_impl(b CALLER_NULL) #define internal_free(m, mem) dlfree_impl(mem) #else #define internal_malloc(m, b) dlmalloc(b) @@ -4940,7 +4949,7 @@ static void* tmalloc_small(mstate m, size_t nb) { #if !ONLY_MSPACES STATIC_IF_MCHECK -void* dlmalloc_impl(size_t bytes) { +void *dlmalloc_impl(size_t bytes CALLER_PARAM) { #ifdef __UBOOT__ #if CONFIG_IS_ENABLED(SYS_MALLOC_F) if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) @@ -5247,7 +5256,7 @@ void* dlcalloc_impl(size_t n_elements, size_t elem_size) { (req / n_elements != elem_size)) req = MAX_SIZE_T; /* force downstream failure on overflow */ } - mem = dlmalloc_impl(req); + mem = dlmalloc_impl(req CALLER_NULL); #ifdef __UBOOT__ #if CONFIG_IS_ENABLED(SYS_MALLOC_F) /* For pre-reloc simple malloc, just zero the memory directly */ @@ -5702,7 +5711,7 @@ void* dlrealloc_impl(void* oldmem, size_t bytes) { #endif void* mem = 0; if (oldmem == 0) { - mem = dlmalloc_impl(bytes); + mem = dlmalloc_impl(bytes CALLER_NULL); } else if (bytes >= MAX_REQUEST) { MALLOC_FAILURE_ACTION; @@ -5755,7 +5764,7 @@ void* dlrealloc_impl(void* oldmem, size_t bytes) { } } #else /* defined(__UBOOT__) && NO_REALLOC_IN_PLACE */ - mem = dlmalloc_impl(bytes); + mem = dlmalloc_impl(bytes CALLER_NULL); if (mem != 0) { size_t oc = chunksize(oldp) - overhead_for(oldp); memcpy(mem, oldmem, (oc < bytes)? oc : bytes); @@ -5821,7 +5830,7 @@ void* dlmemalign_impl(size_t alignment, size_t bytes) { * The base pointer must still be properly aligned for this to work. */ if (alignment <= MALLOC_ALIGNMENT) - return dlmalloc_impl(bytes); + return dlmalloc_impl(bytes CALLER_NULL); return internal_memalign(gm, alignment, bytes); } @@ -5950,7 +5959,7 @@ void *dlmalloc(size_t bytes) { mcheck_pedantic_prehook(); size_t fullsz = mcheck_alloc_prehook(bytes); - void *p = dlmalloc_impl(fullsz); + void *p = dlmalloc_impl(fullsz CALLER_NULL); if (!p) return p; @@ -6023,6 +6032,35 @@ int mcheck(mcheck_abortfunc_t f) void mcheck_check_all(void) { mcheck_pedantic_check(); } enum mcheck_status mprobe(void *__ptr) { return mcheck_mprobe(__ptr); } +#elif CONFIG_IS_ENABLED(MALLOC_DEBUG) +/* + * Simple wrappers when MALLOC_DEBUG is enabled but not MCHECK. + * These just forward to the _impl functions. + */ +void *dlmalloc(size_t bytes) +{ + return dlmalloc_impl(bytes CALLER_NULL); +} + +void dlfree(void *mem) +{ + dlfree_impl(mem); +} + +void *dlrealloc(void *oldmem, size_t bytes) +{ + return dlrealloc_impl(oldmem, bytes); +} + +void *dlmemalign(size_t alignment, size_t bytes) +{ + return dlmemalign_impl(alignment, bytes); +} + +void *dlcalloc(size_t n, size_t elem_size) +{ + return dlcalloc_impl(n, elem_size); +} #endif /* MCHECK_HEAP_PROTECTION */ #endif /* !ONLY_MSPACES */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a new 'malloc dump' command that walks the dlmalloc heap from start to end, printing each chunk's address, size (in hex), and status (used/free/top). This is useful for debugging memory allocation issues. When CONFIG_MCHECK_HEAP_PROTECTION is enabled, the caller string is also shown if available. Example output: Heap dump: 18a1d000 - 1ea1f000 Address Size Status ---------------------------------- 18a1d000 10 (chunk header) 18a1d010 90 used 18adfc30 60 <free> 18adff90 5f3f030 top 1ea1f000 end ---------------------------------- Used: c2ef0 bytes in 931 chunks Free: 5f3f0c0 bytes in 2 chunks + top Expand the console-record size to handle this command. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- cmd/malloc.c | 14 ++++++-- common/Kconfig | 1 + common/dlmalloc.c | 71 +++++++++++++++++++++++++++++++++++++++- doc/usage/cmd/malloc.rst | 22 +++++++++++++ include/malloc.h | 8 +++++ test/cmd/malloc.c | 20 +++++++++++ 6 files changed, 133 insertions(+), 3 deletions(-) diff --git a/cmd/malloc.c b/cmd/malloc.c index 3750a16158b..9c7dfbfc0c3 100644 --- a/cmd/malloc.c +++ b/cmd/malloc.c @@ -30,8 +30,18 @@ static int do_malloc_info(struct cmd_tbl *cmdtp, int flag, int argc, return 0; } +static int do_malloc_dump(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + malloc_dump(); + + return 0; +} + U_BOOT_LONGHELP(malloc, - "info - display malloc statistics\n"); + "info - display malloc statistics\n" + "malloc dump - dump heap chunks (address, size, status)\n"); U_BOOT_CMD_WITH_SUBCMDS(malloc, "malloc information", malloc_help_text, - U_BOOT_SUBCMD_MKENT(info, 1, 1, do_malloc_info)); + U_BOOT_SUBCMD_MKENT(info, 1, 1, do_malloc_info), + U_BOOT_SUBCMD_MKENT(dump, 1, 1, do_malloc_dump)); diff --git a/common/Kconfig b/common/Kconfig index 3bd11f44c51..0aa32145710 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -26,6 +26,7 @@ config CONSOLE_RECORD_INIT_F config CONSOLE_RECORD_OUT_SIZE hex "Output buffer size" depends on CONSOLE_RECORD + default 0x2000 if CMD_MALLOC && UNIT_TEST default 0x1000 if UNIT_TEST default 0x400 help diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 420bed1c480..b40963604e4 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -635,7 +635,7 @@ DECLARE_GLOBAL_DATA_PTR; #if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) || CONFIG_IS_ENABLED(MALLOC_DEBUG) #define STATIC_IF_MCHECK static -#ifdef CONFIG_MCHECK_HEAP_PROTECTION +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) #undef MALLOC_COPY #undef MALLOC_ZERO static inline void MALLOC_ZERO(void *p, size_t sz) { memset(p, 0, sz); } @@ -7021,6 +7021,75 @@ void malloc_disable_testing(void) malloc_testing = false; } +void malloc_dump(void) +{ + mchunkptr q; + msegmentptr s; + size_t used = 0, free_space = 0; + int used_count = 0, free_count = 0; + + if (!is_initialized(gm)) { + printf("dlmalloc not initialized\n"); + return; + } + + printf("Heap dump: %lx - %lx\n", mem_malloc_start, mem_malloc_end); + printf("%12s %10s %s\n", "Address", "Size", "Status"); + printf("----------------------------------\n"); + + s = &gm->seg; + while (s != 0) { + q = align_as_chunk(s->base); + + /* Show chunk header before first allocation */ + printf("%12lx %10zx (chunk header)\n", (ulong)s->base, + (size_t)((char *)chunk2mem(q) - (char *)s->base)); + + while (segment_holds(s, q) && + q != gm->top && q->head != FENCEPOST_HEAD) { + size_t sz = chunksize(q); + void *mem = chunk2mem(q); + + if (is_inuse(q)) { +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) + struct mcheck_hdr *hdr = (struct mcheck_hdr *)mem; + + if (hdr->caller[0]) + printf("%12lx %10zx %s\n", + (ulong)mem, sz, hdr->caller); + else + printf("%12lx %10zx\n", + (ulong)mem, sz); +#else + printf("%12lx %10zx\n", + (ulong)mem, sz); +#endif + used += sz; + used_count++; + } else { + printf("%12lx %10zx <free>\n", + (ulong)mem, sz); + free_space += sz; + free_count++; + } + q = next_chunk(q); + } + s = s->next; + } + + /* Print top chunk (wilderness) */ + if (gm->top && gm->topsize > 0) { + printf("%12lx %10zx top\n", + (ulong)chunk2mem(gm->top), gm->topsize); + free_space += gm->topsize; + } + + printf("%12lx %10s end\n", mem_malloc_end, ""); + printf("----------------------------------\n"); + printf("Used: %zx bytes in %d chunks\n", used, used_count); + printf("Free: %zx bytes in %d chunks + top\n", free_space, free_count); +} + int initf_malloc(void) { #if CONFIG_IS_ENABLED(SYS_MALLOC_F) diff --git a/doc/usage/cmd/malloc.rst b/doc/usage/cmd/malloc.rst index 7b08b358258..fac9bb29aac 100644 --- a/doc/usage/cmd/malloc.rst +++ b/doc/usage/cmd/malloc.rst @@ -12,6 +12,7 @@ Synopsis :: malloc info + malloc dump Description ----------- @@ -23,6 +24,13 @@ info amount currently in use, and call counts for malloc(), free(), and realloc(). +dump + Walks the heap and prints each chunk's address, size (in hex), and status. + In-use chunks show no status label, while free chunks show ``<free>``. + Special entries show ``(chunk header)``, ``top``, or ``end``. This is useful + for debugging memory allocation issues. When CONFIG_MCHECK_HEAP_PROTECTION + is enabled, the caller string is also shown if available. + The total heap size is set by ``CONFIG_SYS_MALLOC_LEN``. Example @@ -37,6 +45,20 @@ Example free count = 567 realloc count = 89 + => malloc dump + Heap dump: 19a0e000 - 1fa10000 + Address Size Status + ---------------------------------- + 19a0e000 10 (chunk header) + 19a0e010 a0 + 19a0e0b0 6070 + 19adfc30 60 <free> + 19adff90 5f3f030 top + 1fa10000 end + ---------------------------------- + Used: c2ef0 bytes in 931 chunks + Free: 5f3f0c0 bytes in 2 chunks + top + Configuration ------------- diff --git a/include/malloc.h b/include/malloc.h index 0d8a2a43f2a..a4d588936ec 100644 --- a/include/malloc.h +++ b/include/malloc.h @@ -704,6 +704,14 @@ void malloc_enable_testing(int max_allocs); */ void malloc_disable_testing(void); +/** + * malloc_dump() - Print a dump of all heap chunks + * + * Walks the dlmalloc heap from start to end, printing each chunk's + * address, size, and status (used/free/top). + */ +void malloc_dump(void); + /** * mem_malloc_init() - Initialize the malloc() heap * diff --git a/test/cmd/malloc.c b/test/cmd/malloc.c index f9d41a36cad..3c1a44bcacf 100644 --- a/test/cmd/malloc.c +++ b/test/cmd/malloc.c @@ -32,3 +32,23 @@ static int cmd_test_malloc_info(struct unit_test_state *uts) return 0; } CMD_TEST(cmd_test_malloc_info, UTF_CONSOLE); + +/* Test 'malloc dump' command */ +static int cmd_test_malloc_dump(struct unit_test_state *uts) +{ + /* this takes a long time to dump, with truetype enabled, so skip it */ + return -EAGAIN; + + ut_assertok(run_command("malloc dump", 0)); + ut_assert_nextlinen("Heap dump: "); + ut_assert_nextline("%12s %10s %s", "Address", "Size", "Status"); + ut_assert_nextline("----------------------------------"); + ut_assert_nextline("%12lx %10x (chunk header)", mem_malloc_start, 0x10); + ut_assert_skip_to_line("----------------------------------"); + ut_assert_nextlinen("Used: "); + ut_assert_nextlinen("Free: "); + ut_assert_console_end(); + + return 0; +} +CMD_TEST(cmd_test_malloc_dump, UTF_CONSOLE); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> For memalign allocations, the mcheck header is placed at an offset from the chunk start to maintain alignment. The current assumption is that the header is always at the start of the chunk, but this is not true for memalign allocations. Add find_mcheck_hdr_in_chunk() which looks up the header in the mcheck registry and validates: - The header falls within the chunk's memory range - The aln_skip field is consistent with the header position - The canary is MAGICWORD (active), not MAGICFREE (freed) This ensures malloc_dump correctly displays caller info for all allocations including those made via memalign. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index b40963604e4..14515e423cc 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -7021,6 +7021,53 @@ void malloc_disable_testing(void) malloc_testing = false; } +/** + * find_mcheck_hdr_in_chunk() - find mcheck header within a chunk + * + * For memalign allocations, the mcheck header may be at an offset from + * the chunk start to maintain alignment. Look up the header in the + * mcheck registry, which stores pointers to all active headers. + * + * @mem: chunk memory pointer (from chunk2mem) + * @sz: chunk size + * Return: pointer to mcheck header if found, NULL otherwise + */ +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) +static struct mcheck_hdr *find_mcheck_hdr_in_chunk(void *mem, size_t sz) +{ + struct mcheck_hdr *hdr; + char *start = (char *)mem; + char *end = start + sz; + int i, j; + + for (i = 0; i < REGISTRY_SZ; i++) { + hdr = mcheck_registry[i]; + if (!hdr) + continue; + + /* Check if this header falls within our chunk */ + if ((char *)hdr < start || (char *)hdr >= end) + continue; + + /* Validate the aln_skip is consistent with position */ + if ((char *)hdr != start + hdr->aln_skip) + continue; + + /* Verify canary is valid (not freed) */ + for (j = 0; j < CANARY_DEPTH; j++) { + if (hdr->canary.elems[j] != MAGICWORD) + goto next; + } + + return hdr; +next: + continue; + } + + return NULL; +} +#endif + void malloc_dump(void) { mchunkptr q; @@ -7052,9 +7099,10 @@ void malloc_dump(void) if (is_inuse(q)) { #if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) - struct mcheck_hdr *hdr = (struct mcheck_hdr *)mem; + struct mcheck_hdr *hdr; - if (hdr->caller[0]) + hdr = find_mcheck_hdr_in_chunk(mem, sz); + if (hdr && hdr->caller[0]) printf("%12lx %10zx %s\n", (ulong)mem, sz, hdr->caller); else -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When CONFIG_MCHECK_HEAP_PROTECTION is enabled, use backtrace_str() to capture the caller information for each malloc/calloc/realloc/memalign call. This information is stored in the mcheck header and can be viewed with 'malloc dump'. Add a flag to disable the backtrace when the stack is corrupted, since the backtrace code tries to walks the invalid stack frames and will crash. Note: A few allocations made during libbacktrace initialisation may not have caller info since they occur during the first backtrace call. Example output showing caller info: 18a1d010 90 used log_init:453 <-board_init_r:774 18a1d0a0 6060 used membuf_new:420 <-console_record 18a3b840 90 used of_alias_scan:911 <-board_init_ Fix up the backtrace test to avoid recursion. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 35 +++++++++++++++++++++++++++++++---- doc/usage/cmd/malloc.rst | 12 ++++++++++++ include/malloc.h | 15 +++++++++++++++ test/lib/backtrace.c | 7 ++++--- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 14515e423cc..c294b355ff2 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -5953,8 +5953,35 @@ size_t dlmalloc_usable_size(const void* mem) { } #if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) +#include <backtrace.h> #include "mcheck_core.inc.h" +/* Guard against recursive backtrace calls during malloc */ +static bool in_backtrace __section(".data"); + +/* + * Flag to disable backtrace collection when the stack is known to be corrupt. + * Set via malloc_backtrace_skip() before calling panic(). + */ +static bool mcheck_skip_backtrace __section(".data"); + +void malloc_backtrace_skip(bool skip) +{ + mcheck_skip_backtrace = skip; +} + +static const char *mcheck_caller(void) +{ + const char *caller = NULL; + + if (!in_backtrace && !mcheck_skip_backtrace) { + in_backtrace = true; + caller = backtrace_str(2); + in_backtrace = false; + } + return caller; +} + void *dlmalloc(size_t bytes) { mcheck_pedantic_prehook(); @@ -5963,7 +5990,7 @@ void *dlmalloc(size_t bytes) if (!p) return p; - return mcheck_alloc_posthook(p, bytes, NULL); + return mcheck_alloc_posthook(p, bytes, mcheck_caller()); } void dlfree(void *mem) { dlfree_impl(mcheck_free_prehook(mem)); } @@ -5988,7 +6015,7 @@ void *dlrealloc(void *oldmem, size_t bytes) p = dlrealloc_impl(p, newsz); if (!p) return p; - return mcheck_alloc_noclean_posthook(p, bytes, NULL); + return mcheck_alloc_noclean_posthook(p, bytes, mcheck_caller()); } void *dlmemalign(size_t alignment, size_t bytes) @@ -5999,7 +6026,7 @@ void *dlmemalign(size_t alignment, size_t bytes) if (!p) return p; - return mcheck_memalign_posthook(alignment, p, bytes, NULL); + return mcheck_memalign_posthook(alignment, p, bytes, mcheck_caller()); } /* dlpvalloc, dlvalloc redirect to dlmemalign, so they need no wrapping */ @@ -6013,7 +6040,7 @@ void *dlcalloc(size_t n, size_t elem_size) if (!p) return p; - return mcheck_alloc_noclean_posthook(p, n * elem_size, NULL); + return mcheck_alloc_noclean_posthook(p, n * elem_size, mcheck_caller()); } /* mcheck API */ diff --git a/doc/usage/cmd/malloc.rst b/doc/usage/cmd/malloc.rst index fac9bb29aac..3693034b41e 100644 --- a/doc/usage/cmd/malloc.rst +++ b/doc/usage/cmd/malloc.rst @@ -59,6 +59,18 @@ Example Used: c2ef0 bytes in 931 chunks Free: 5f3f0c0 bytes in 2 chunks + top +With CONFIG_MCHECK_HEAP_PROTECTION enabled, the caller backtrace is shown:: + + => malloc dump + Heap dump: 18a1d000 - 1ea1f000 + Address Size Status + ---------------------------------- + 18a1d000 10 (chunk header) + 18a1d010 90 used log_init:453 <-board_init_r:774 + 18a1d0a0 6060 used membuf_new:420 <-console_record + 18a3b840 90 used of_alias_scan:911 <-board_init_ + ... + Configuration ------------- diff --git a/include/malloc.h b/include/malloc.h index a4d588936ec..3327bdcb44f 100644 --- a/include/malloc.h +++ b/include/malloc.h @@ -712,6 +712,21 @@ void malloc_disable_testing(void); */ void malloc_dump(void); +/** + * malloc_backtrace_skip() - Control backtrace collection in malloc + * + * When the stack is corrupted (e.g., by a stack overflow), collecting + * a backtrace during malloc can crash. Use this function to disable + * backtrace collection before corrupting the stack. + * + * @skip: true to skip backtrace collection, false to enable it + */ +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) +void malloc_backtrace_skip(bool skip); +#else +static inline void malloc_backtrace_skip(bool skip) {} +#endif + /** * mem_malloc_init() - Initialize the malloc() heap * diff --git a/test/lib/backtrace.c b/test/lib/backtrace.c index 3f20b7854bf..155e5b13af8 100644 --- a/test/lib/backtrace.c +++ b/test/lib/backtrace.c @@ -68,16 +68,17 @@ static int lib_test_backtrace_str(struct unit_test_state *uts) line); ut_asserteq_regex(pattern, str); - /* Test backtrace_str() */ + /* Test backtrace_str() - copy result before printf since it may recurse */ line = __LINE__ + 1; cstr = backtrace_str(0); ut_assertnonnull(cstr); + strlcpy(buf, cstr, sizeof(buf)); - printf("backtrace_str: %s\n", cstr); + printf("backtrace_str: %s\n", buf); snprintf(pattern, sizeof(pattern), "lib_test_backtrace_str:%d <-ut_run_test:\\d+ <-ut_run_test_live_flat:\\d+", line); - ut_asserteq_regex(pattern, cstr); + ut_asserteq_regex(pattern, buf); return 0; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When the stack is corrupted (e.g., by the stack protector test), collecting a backtrace during malloc causes a crash because the backtrace code walks the invalid stack frames. Update __stack_chk_fail() to set the flag before calling panic() Also update stackprot_test() to set the flag before intentionally corrupting the stack. This is needed because of the printf() in the test: on sandbox printf() results in truetype allocations due to the console output. These fixes allow the stack protector test to pass with mcheck enabled. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- cmd/stackprot_test.c | 7 +++++++ common/stackprot.c | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c index e7ff4a06158..d7fbc3ecca0 100644 --- a/cmd/stackprot_test.c +++ b/cmd/stackprot_test.c @@ -4,6 +4,7 @@ */ #include <command.h> +#include <malloc.h> static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -14,6 +15,12 @@ static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int argc, */ char a[128]; + /* + * Disable backtrace collection before corrupting the stack. + * Otherwise, any malloc (e.g., from printf/font rendering) will + * attempt to collect a backtrace from the corrupted stack and crash. + */ + malloc_backtrace_skip(true); memset(a, 0xa5, 512); printf("We have smashed our stack as this should not exceed 128: sizeof(a) = %zd\n", diff --git a/common/stackprot.c b/common/stackprot.c index 4e3297b7d00..408cd6d1e05 100644 --- a/common/stackprot.c +++ b/common/stackprot.c @@ -4,6 +4,7 @@ */ #include <asm/global_data.h> +#include <malloc.h> DECLARE_GLOBAL_DATA_PTR; @@ -13,6 +14,11 @@ void __stack_chk_fail(void) { void *ra; + /* + * When the stack is corrupted, backtrace collection will crash. + * Skip it before calling panic(). + */ + malloc_backtrace_skip(true); ra = __builtin_extract_return_addr(__builtin_return_address(0)); panic("Stack smashing detected in function:\n%p relocated from %p", ra, ra - gd->reloc_off); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When CONFIG_MCHECK_HEAP_PROTECTION is enabled, show the original caller info for freed chunks. This helps identify what code allocated memory that is now free. The mcheck header's canary and caller fields survive after free since dlmalloc only overwrites the first 16 bytes (size, aln_skip) with its free list pointers (fd, bk). We detect freed headers by looking for the MAGICFREE canary pattern. For small chunks or coalesced chunks where the header is not preserved, the caller info is simply omitted. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/dlmalloc.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/common/dlmalloc.c b/common/dlmalloc.c index c294b355ff2..62e1e0a77da 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -7093,6 +7093,39 @@ next: return NULL; } + +/** + * find_freed_mcheck_hdr() - find a freed mcheck header in a free chunk + * + * For freed chunks, the mcheck header remains with MAGICFREE canaries. + * Since freed entries are removed from the registry, scan the chunk + * for the MAGICFREE pattern. Only check offset 0 since free chunks + * may have been coalesced and we want the first (original) allocation. + * + * Note: dlmalloc overwrites the first 16 bytes (size, aln_skip) with + * free list pointers (fd, bk), but the canary and caller remain intact. + * + * @mem: chunk memory pointer (from chunk2mem) + * @sz: chunk size + * Return: pointer to mcheck header if found, NULL otherwise + */ +static struct mcheck_hdr *find_freed_mcheck_hdr(void *mem, size_t sz) +{ + struct mcheck_hdr *hdr = (struct mcheck_hdr *)mem; + int i; + + /* Only check at offset 0 - coalesced chunks lose alignment info */ + if (sz < sizeof(struct mcheck_hdr)) + return NULL; + + /* Check for MAGICFREE canary pattern */ + for (i = 0; i < CANARY_DEPTH; i++) { + if (hdr->canary.elems[i] != MAGICFREE) + return NULL; + } + + return hdr; +} #endif void malloc_dump(void) @@ -7142,8 +7175,20 @@ void malloc_dump(void) used += sz; used_count++; } else { - printf("%12lx %10zx <free>\n", +#if CONFIG_IS_ENABLED(MCHECK_HEAP_PROTECTION) + struct mcheck_hdr *hdr; + + hdr = find_freed_mcheck_hdr(mem, sz); + if (hdr && hdr->caller[0]) + printf("%12lx %10zx free %s\n", + (ulong)mem, sz, hdr->caller); + else + printf("%12lx %10zx free\n", + (ulong)mem, sz); +#else + printf("%12lx %10zx free\n", (ulong)mem, sz); +#endif free_space += sz; free_count++; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Use a static bool flag to ensure the overflow warning is printed only once, avoiding repeated messages when the registry is full. Make sure to set the flag before calling printf(), which can itself do allocations with sandbox / Truetype. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- common/mcheck_core.inc.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/common/mcheck_core.inc.h b/common/mcheck_core.inc.h index 63dbeaa5f56..598a5d018ab 100644 --- a/common/mcheck_core.inc.h +++ b/common/mcheck_core.inc.h @@ -201,6 +201,7 @@ static void *mcheck_allocated_helper(void *altoghether_ptr, size_t customer_sz, size_t alignment, int clean_content, const char *caller) { + static bool overflow_msg_shown; const size_t slop = alignment ? mcheck_evaluate_memalign_prefix_size(alignment) - sizeof(struct mcheck_hdr) : 0; struct mcheck_hdr *hdr = (struct mcheck_hdr *)((char *)altoghether_ptr + slop); @@ -239,10 +240,10 @@ static void *mcheck_allocated_helper(void *altoghether_ptr, size_t customer_sz, return payload; // normal end } - static char *overflow_msg = "\n\n\nERROR: mcheck registry overflow, pedantic check would be incomplete!!\n\n\n\n"; - - printf("%s", overflow_msg); - overflow_msg = "(mcheck registry full)"; + if (!overflow_msg_shown) { + overflow_msg_shown = true; + printf("\n\nERROR: mcheck registry overflow, pedantic check would be incomplete!\n\n"); + } return payload; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Update the malloc() with more info about the debugging features: - CONFIG_MALLOC_DEBUG and CONFIG_MCHECK_HEAP_PROTECTION Kconfig options - The malloc command with info and dump subcommands - Caller backtrace display when mcheck is enabled Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- doc/develop/malloc.rst | 103 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 8 deletions(-) diff --git a/doc/develop/malloc.rst b/doc/develop/malloc.rst index 3c6b6ea65a4..8dba2d98afe 100644 --- a/doc/develop/malloc.rst +++ b/doc/develop/malloc.rst @@ -117,6 +117,19 @@ Main U-Boot (post-relocation) compatibility and testing. New boards should use the modern allocator. Default: n +``CONFIG_MALLOC_DEBUG`` + Bool to enable malloc debugging features. This enables the + ``malloc_get_info()`` function to retrieve memory statistics and supports + the ``malloc`` command. Default: y if UNIT_TEST is enabled. + +``CONFIG_MCHECK_HEAP_PROTECTION`` + Bool to enable heap protection using the mcheck library. This adds canary + values before and after each allocation to detect buffer overflows, + underflows, double-frees, and memory corruption. When enabled, caller + backtraces are recorded for each allocation and displayed by + ``malloc dump``. This significantly increases memory overhead and should + only be used for debugging. Default: n + xPL Boot Phases ~~~~~~~~~~~~~~~ @@ -298,17 +311,90 @@ for memory-leak detection. Debugging --------- -For debugging heap issues, consider: +U-Boot provides several features to help debug memory-allocation issues: + +CONFIG_MALLOC_DEBUG +~~~~~~~~~~~~~~~~~~~ + +Enable ``CONFIG_MALLOC_DEBUG`` to activate malloc debugging features. This is +enabled by default when ``CONFIG_UNIT_TEST`` is set. It provides: + +- The ``malloc_get_info()`` function to retrieve memory statistics +- Allocation call counters (malloc, free, realloc counts) +- Support for the ``malloc`` command (see :doc:`/usage/cmd/malloc`) + +The :doc:`/usage/cmd/malloc` command provides two subcommands: + +``malloc info`` + Shows memory-allocation statistics including total heap size, memory in use, + and call counts:: + + => malloc info + total bytes = 96 MiB + in use bytes = 700.9 KiB + malloc count = 1234 + free count = 567 + realloc count = 89 + +``malloc dump`` + Walks the entire heap and prints each chunk's address, size, and status + (used, free, or top). This is useful for understanding heap layout and + finding memory leaks:: + + => malloc dump + Heap dump: 19a0e000 - 1fa10000 + Address Size Status + ---------------------------------- + 19a0e000 10 (chunk header) + 19a0e010 a0 + 19a0e0b0 6070 + 19adfc30 60 <free> + 19adff90 5f3f030 top + 1fa10000 end + ---------------------------------- + Used: c2ef0 bytes in 931 chunks + Free: 5f3f0c0 bytes in 2 chunks + top + +CONFIG_MCHECK_HEAP_PROTECTION +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Enable ``CONFIG_MCHECK_HEAP_PROTECTION`` for heap protection using the mcheck +library. This adds canary values before and after each allocation to detect: + +- Buffer overflows and underflows +- Double-frees +- Memory corruption + +This significantly increases memory overhead and should only be used for +debugging. U-Boot includes mcheck support via mcheck(), mcheck_pedantic(), and +mcheck_check_all(). + +When mcheck is enabled, the ``malloc dump`` command also shows caller +information for each allocation, including a backtrace showing where the +allocation was made:: + + => malloc dump + Heap dump: 18a1d000 - 1ea1f000 + Address Size Status + ---------------------------------- + 18a1d000 10 (chunk header) + 18a1d010 90 used log_init:453 <-board_init_r:774 + 18a1d0a0 6060 used membuf_new:420 <-console_record + 18a3b840 90 used of_alias_scan:911 <-board_init_ + +This caller information makes it easy to track down memory leaks by showing +exactly where each allocation originated. + +Valgrind +~~~~~~~~ -1. **mcheck**: U-Boot includes mcheck support for detecting buffer overruns. - Enable CONFIG_MCHECK to use mcheck(), mcheck_pedantic(), and - mcheck_check_all(). +When running sandbox with Valgrind, the allocator includes annotations to help +detect memory errors. See :ref:`sandbox_valgrind`. -2. **Valgrind**: When running sandbox with Valgrind, the allocator includes - annotations to help detect memory errors. See :ref:`sandbox_valgrind`. +malloc testing +~~~~~~~~~~~~~~ -3. **malloc testing**: Unit tests can use malloc_enable_testing() to simulate - allocation failures. +Unit tests can use malloc_enable_testing() to simulate allocation failures. API Reference ------------- @@ -331,3 +417,4 @@ See Also - :doc:`memory` - Memory management overview - :doc:`global_data` - Global data and the GD_FLG_FULL_MALLOC_INIT flag +- :doc:`/usage/cmd/malloc` - malloc command reference -- 2.43.0
participants (2)
-
Heinrich Schuchardt -
Simon Glass