From: Vincent Stehlé Date: Tue, 2 Apr 2024 11:29:16 +0000 (+0200) Subject: trace: use dynamic string buffer in make_flamegraph() X-Git-Url: http://git.dujemihanovic.xyz/%7B%7B%20%24style.RelPermalink%20%7D%7D?a=commitdiff_plain;h=b782be47f12b1957c67e612e7d3b0ef7b3f32b36;p=u-boot.git trace: use dynamic string buffer in make_flamegraph() The str[] buffer declared in make_flamegraph() is used to hold strings representing the full call-stacks recorded in traces. The size of this buffer is currently 500 characters and this works well for the documented examples. However, it is possible to exhaust this buffer when processing traces captured when running the UEFI shell on aarch64 sandbox for example. Indeed, the maximum length needed for such traces can reach 780 characters. As it is difficult to evaluate the maximum size that would ever be needed for all the possible traces, let's use a dynamically allocated `abuf' instead, which we reallocate when needed. This fixes the following error: String too short (500 chars) While at it, fix a few typos in strings and comments. Signed-off-by: Vincent Stehlé Cc: Tom Rini Cc: Simon Glass Cc: Michal Simek --- diff --git a/tools/proftool.c b/tools/proftool.c index fca45e4a5a..c2e3809935 100644 --- a/tools/proftool.c +++ b/tools/proftool.c @@ -290,7 +290,7 @@ static void usage(void) "Options:\n" " -c \tSpecify config file\n" " -f \tSpecify output subtype\n" - " -m \tSpecify Systen.map file\n" + " -m \tSpecify System.map file\n" " -o \tSpecify output file\n" " -t \tSpecify trace data file (from U-Boot 'trace calls')\n" " -v <0-4>\tSpecify verbosity\n" @@ -306,7 +306,7 @@ static void usage(void) } /** - * h_cmp_offset - bsearch() function to compare two functions bny their offset + * h_cmp_offset - bsearch() function to compare two functions by their offset * * @v1: Pointer to first function (struct func_info) * @v2: Pointer to second function (struct func_info) @@ -431,7 +431,7 @@ static struct func_info *find_func_by_offset(uint offset) static struct func_info *find_caller_by_offset(uint offset) { int low; /* least function that could be a match */ - int high; /* greated function that could be a match */ + int high; /* greatest function that could be a match */ struct func_info key; low = 0; @@ -1352,7 +1352,7 @@ static int write_pages(struct twriter *tw, enum out_format_t out_format, } if (!(func->flags & FUNCF_TRACE)) { - debug("Funcion '%s' is excluded from trace\n", + debug("Function '%s' is excluded from trace\n", func->name); skip_count++; continue; @@ -1781,7 +1781,8 @@ static int make_flame_tree(enum out_format_t out_format, * * This works by maintaining a string shared across all recursive calls. The * function name for this node is added to the existing string, to make up the - * full call-stack description. For example, on entry, @str might contain: + * full call-stack description. For example, on entry, @str_buf->data might + * contain: * * "initf_bootstage;bootstage_mark_name" * ^ @base @@ -1795,18 +1796,18 @@ static int make_flame_tree(enum out_format_t out_format, * @fout: Output file * @out_format: Output format to use * @node: Node to output (pass the whole tree at first) - * @str: String to use to build the output line (e.g. 500 charas long) - * @maxlen: Maximum length of string + * @str_buf: String buffer to use to build the output line * @base: Current base position in the string * @treep: Returns the resulting flamegraph tree * Returns 0 if OK, -1 on error */ static int output_tree(FILE *fout, enum out_format_t out_format, - const struct flame_node *node, char *str, int maxlen, + const struct flame_node *node, struct abuf *str_buf, int base) { const struct flame_node *child; int pos; + char *str = abuf_data(str_buf); if (node->count) { if (out_format == OUT_FMT_FLAMEGRAPH_CALLS) { @@ -1832,18 +1833,29 @@ static int output_tree(FILE *fout, enum out_format_t out_format, if (pos) str[pos++] = ';'; list_for_each_entry(child, &node->child_head, sibling_node) { - int len; + int len, needed; len = strlen(child->func->name); - if (pos + len + 1 >= maxlen) { - fprintf(stderr, "String too short (%d chars)\n", - maxlen); - return -1; + needed = pos + len + 1; + if (needed > abuf_size(str_buf)) { + /* + * We need to re-allocate the string buffer; increase + * its size by multiples of 500 characters. + */ + needed = 500 * ((needed / 500) + 1); + if (!abuf_realloc(str_buf, needed)) + return -1; + str = abuf_data(str_buf); + memset(str + pos, 0, abuf_size(str_buf) - pos); } strcpy(str + pos, child->func->name); - if (output_tree(fout, out_format, child, str, maxlen, - pos + len)) + if (output_tree(fout, out_format, child, str_buf, pos + len)) return -1; + /* + * Update our pointer as the string buffer might have been + * re-allocated. + */ + str = abuf_data(str_buf); } return 0; @@ -1859,16 +1871,24 @@ static int output_tree(FILE *fout, enum out_format_t out_format, static int make_flamegraph(FILE *fout, enum out_format_t out_format) { struct flame_node *tree; - char str[500]; + struct abuf str_buf; + char *str; + int ret = 0; if (make_flame_tree(out_format, &tree)) return -1; - *str = '\0'; - if (output_tree(fout, out_format, tree, str, sizeof(str), 0)) + abuf_init(&str_buf); + if (!abuf_realloc(&str_buf, 500)) return -1; - return 0; + str = abuf_data(&str_buf); + memset(str, 0, abuf_size(&str_buf)); + if (output_tree(fout, out_format, tree, &str_buf, 0)) + ret = -1; + + abuf_uninit(&str_buf); + return ret; } /**