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> 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> 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

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> 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> 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> 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> 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> 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 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> 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> 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> 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> 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> 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> 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 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> 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