From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Kan Liang <kan.liang@linux.intel.com>,
James Clark <james.clark@linaro.org>,
Howard Chu <howardchu95@gmail.com>,
Yicong Yang <yangyicong@hisilicon.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] perf debug: Add function symbols to dump_stack
Date: Fri, 20 Jun 2025 13:23:04 -0700 [thread overview]
Message-ID: <aFXDKO-aLWzA3M2H@google.com> (raw)
In-Reply-To: <20250611221521.722045-1-irogers@google.com>
On Wed, Jun 11, 2025 at 03:15:21PM -0700, Ian Rogers wrote:
> Symbolize stack traces by creating a live machine. Add this
> functionality to dump_stack and switch dump_stack users to use
> it. Switch TUI to use it. Add stack traces to the child test function
> which can be useful to diagnose blocked code.
>
> Example output:
> ```
> 8: PERF_RECORD_* events & perf_sample fields : Running (1 active)
> ^C
> Signal (2) while running tests.
> Terminating tests with the same signal
> Internal test harness failure. Completing any started tests:
> : 8: PERF_RECORD_* events & perf_sample fields:
>
> ---- unexpected signal (2) ----
> #0 0x5590fb6209b6 in child_test_sig_handler builtin-test.c:243
> #1 0x7f4a91e49e20 in __restore_rt libc_sigaction.c:0
> #2 0x7f4a91ee4f33 in clock_nanosleep@GLIBC_2.2.5 clock_nanosleep.c:71
> #3 0x7f4a91ef0333 in __nanosleep nanosleep.c:26
> #4 0x7f4a91f01f68 in __sleep sleep.c:55
> #5 0x5590fb638c63 in test__PERF_RECORD perf-record.c:295
> #6 0x5590fb620b43 in run_test_child builtin-test.c:269
> #7 0x5590fb5b83ab in start_command run-command.c:127
> #8 0x5590fb621572 in start_test builtin-test.c:467
> #9 0x5590fb621a47 in __cmd_test builtin-test.c:573
> #10 0x5590fb6225ea in cmd_test builtin-test.c:775
> #11 0x5590fb5a9099 in run_builtin perf.c:351
> #12 0x5590fb5a9340 in handle_internal_command perf.c:404
> #13 0x5590fb5a9499 in run_argv perf.c:451
> #14 0x5590fb5a97e2 in main perf.c:558
> #15 0x7f4a91e33d68 in __libc_start_call_main libc_start_call_main.h:74
> #16 0x7f4a91e33e25 in __libc_start_main@@GLIBC_2.34 libc-start.c:128
> #17 0x5590fb4fd6d1 in _start perf[436d1]
> ```
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/builtin-test.c | 15 +++++++-
> tools/perf/ui/tui/setup.c | 2 +-
> tools/perf/util/debug.c | 64 +++++++++++++++++++++++++++------
> tools/perf/util/debug.h | 1 +
> 4 files changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 45d3d8b3317a..80375ca39a37 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -6,6 +6,9 @@
> */
> #include <fcntl.h>
> #include <errno.h>
> +#ifdef HAVE_BACKTRACE_SUPPORT
> +#include <execinfo.h>
> +#endif
> #include <poll.h>
> #include <unistd.h>
> #include <setjmp.h>
> @@ -231,6 +234,16 @@ static jmp_buf run_test_jmp_buf;
>
> static void child_test_sig_handler(int sig)
> {
> +#ifdef HAVE_BACKTRACE_SUPPORT
> + void *stackdump[32];
> + size_t stackdump_size;
> +#endif
> +
> + fprintf(stderr, "\n---- unexpected signal (%d) ----\n", sig);
> +#ifdef HAVE_BACKTRACE_SUPPORT
> + stackdump_size = backtrace(stackdump, ARRAY_SIZE(stackdump));
> + __dump_stack(stderr, stackdump, stackdump_size);
> +#endif
> siglongjmp(run_test_jmp_buf, sig);
> }
>
> @@ -244,7 +257,7 @@ static int run_test_child(struct child_process *process)
>
> err = sigsetjmp(run_test_jmp_buf, 1);
> if (err) {
> - fprintf(stderr, "\n---- unexpected signal (%d) ----\n", err);
> + /* Received signal. */
> err = err > 0 ? -err : -1;
> goto err_out;
> }
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index 16c6eff4d241..022534eed68c 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -108,7 +108,7 @@ static void ui__signal_backtrace(int sig)
>
> printf("-------- backtrace --------\n");
> size = backtrace(stackdump, ARRAY_SIZE(stackdump));
> - backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
> + __dump_stack(stdout, stackdump, size);
>
> exit(0);
> }
> diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
> index f9ef7d045c92..8987ac250079 100644
> --- a/tools/perf/util/debug.c
> +++ b/tools/perf/util/debug.c
> @@ -14,11 +14,18 @@
> #ifdef HAVE_BACKTRACE_SUPPORT
> #include <execinfo.h>
> #endif
> +#include "addr_location.h"
> #include "color.h"
> -#include "event.h"
> #include "debug.h"
> +#include "event.h"
> +#include "machine.h"
> +#include "map.h"
> #include "print_binary.h"
> +#include "srcline.h"
> +#include "symbol.h"
> +#include "synthetic-events.h"
> #include "target.h"
> +#include "thread.h"
> #include "trace-event.h"
> #include "ui/helpline.h"
> #include "ui/ui.h"
> @@ -298,21 +305,56 @@ void perf_debug_setup(void)
> libapi_set_print(pr_warning_wrapper, pr_warning_wrapper, pr_debug_wrapper);
> }
>
> +void __dump_stack(FILE *file, void **stackdump, size_t stackdump_size)
> +{
> + /* TODO: async safety. printf, malloc, etc. aren't safe inside a signal handler. */
> + pid_t pid = getpid();
> + struct machine *machine = machine__new_live(/*kernel_maps=*/false, pid);
> + struct thread *thread = NULL;
> +
> + if (machine)
> + thread = machine__find_thread(machine, pid, pid);
> +
> + if (!machine || !thread) {
> + /*
> + * Backtrace functions are async signal safe. Fall back on them
> + * if machine/thread creation fails.
> + */
> + backtrace_symbols_fd(stackdump, stackdump_size, fileno(file));
> + machine__delete(machine);
> + return;
> + }
> +
> + for (size_t i = 0; i < stackdump_size; i++) {
> + struct addr_location al;
> + u64 addr = (u64)stackdump[i];
> +
> + addr_location__init(&al);
> + if (!thread__find_map(thread, PERF_RECORD_MISC_USER, addr, &al))
It seems addr_location__exit(&al) is needed here.
Thanks,
Namhyung
> + continue;
> +
> + al.sym = map__find_symbol(al.map, al.addr);
> + if (al.sym)
> + fprintf(file, " #%zd %p in %s ", i, stackdump[i], al.sym->name);
> + else
> + fprintf(file, " #%zd %p ", i, stackdump[i]);
> +
> + map__fprintf_srcline(al.map, al.addr, "", file);
> + fprintf(file, "\n");
> + addr_location__exit(&al);
> + }
> + thread__put(thread);
> + machine__delete(machine);
> +}
> +
> /* Obtain a backtrace and print it to stdout. */
> #ifdef HAVE_BACKTRACE_SUPPORT
> void dump_stack(void)
> {
> - void *array[16];
> - size_t size = backtrace(array, ARRAY_SIZE(array));
> - char **strings = backtrace_symbols(array, size);
> - size_t i;
> -
> - printf("Obtained %zd stack frames.\n", size);
> -
> - for (i = 0; i < size; i++)
> - printf("%s\n", strings[i]);
> + void *stackdump[32];
> + size_t size = backtrace(stackdump, ARRAY_SIZE(stackdump));
>
> - free(strings);
> + __dump_stack(stdout, stackdump, size);
> }
> #else
> void dump_stack(void) {}
> diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
> index a4026d1fd6a3..6b737e195ce1 100644
> --- a/tools/perf/util/debug.h
> +++ b/tools/perf/util/debug.h
> @@ -85,6 +85,7 @@ void debug_set_display_time(bool set);
> void perf_debug_setup(void);
> int perf_quiet_option(void);
>
> +void __dump_stack(FILE *file, void **stackdump, size_t stackdump_size);
> void dump_stack(void);
> void sighandler_dump_stack(int sig);
>
> --
> 2.50.0.rc1.591.g9c95f17f64-goog
>
next prev parent reply other threads:[~2025-06-20 20:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-11 22:15 [PATCH v3] perf debug: Add function symbols to dump_stack Ian Rogers
2025-06-20 20:23 ` Namhyung Kim [this message]
2025-06-23 16:02 ` Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aFXDKO-aLWzA3M2H@google.com \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=howardchu95@gmail.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yangyicong@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.