Re: [PATCH] perf: Don't show unwind error messages when augmenting frame pointer stack

John Garry
2022-04-08T14:12:27Z

On 06/04/2022 15:56, James Clark wrote:
> Commit b9f6fbb3b2c2 ("perf arm64: Inject missing frames when using 'perf
> record --call-graph=fp'") intended to add a 'best effort' Dwarf unwind
> that improved the frame pointer stack in most scenarios. It's expected
> that the unwind will fail sometimes, but this shouldn't be reported as
> an error. It only works when the return address can be determined from
> the contents of the link register alone.
>
> Fix the error shown when the unwinder requires extra registers by adding
> a new flag that suppresses error messages. This flag is not set in the
> normal --call-graph=dwarf unwind mode so that behavior is not changed.
>
> Reported-by: John Garry <john.garry@huawei.com>
> Fixes: b9f6fbb3b2c2 ("perf arm64: Inject missing frames when using 'perf record --call-graph=fp'")
> Signed-off-by: James Clark <james.clark@arm.com>

Seems ok:
Tested-by: John Garry <john.garry@huawei.com>

Thanks!

> ---
> tools/perf/tests/dwarf-unwind.c | 2 +-
> .../perf/util/arm64-frame-pointer-unwind-support.c | 2 +-
> tools/perf/util/machine.c | 2 +-
> tools/perf/util/unwind-libdw.c | 10 +++++++---
> tools/perf/util/unwind-libdw.h | 1 +
> tools/perf/util/unwind-libunwind-local.c | 10 +++++++---
> tools/perf/util/unwind-libunwind.c | 6 ++++--
> tools/perf/util/unwind.h | 13 ++++++++++---
> 8 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> index 2dab2d262060..afdca7f2959f 100644
> --- a/tools/perf/tests/dwarf-unwind.c
> +++ b/tools/perf/tests/dwarf-unwind.c
> @@ -122,7 +122,7 @@ NO_TAIL_CALL_ATTRIBUTE noinline int test_dwarf_unwind__thread(struct thread *thr
> }
>
> err = unwind__get_entries(unwind_entry, &cnt, thread,
> - &sample, MAX_STACK);
> + &sample, MAX_STACK, false);
> if (err)
> pr_debug("unwind failed\n");
> else if (cnt != MAX_STACK) {
> diff --git a/tools/perf/util/arm64-frame-pointer-unwind-support.c b/tools/perf/util/arm64-frame-pointer-unwind-support.c
> index 2242a885fbd7..4940be4a0569 100644
> --- a/tools/perf/util/arm64-frame-pointer-unwind-support.c
> +++ b/tools/perf/util/arm64-frame-pointer-unwind-support.c
> @@ -53,7 +53,7 @@ u64 get_leaf_frame_caller_aarch64(struct perf_sample *sample, struct thread *thr
> sample->user_regs.cache_regs[PERF_REG_ARM64_SP] = 0;
> }
>
> - ret = unwind__get_entries(add_entry, &entries, thread, sample, 2);
> + ret = unwind__get_entries(add_entry, &entries, thread, sample, 2, true);
> sample->user_regs = old_regs;
>
> if (ret || entries.length != 2)
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index b80048546451..95391236f5f6 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2987,7 +2987,7 @@ static int thread__resolve_callchain_unwind(struct thread *thread,
> return 0;
>
> return unwind__get_entries(unwind_entry, cursor,
> - thread, sample, max_stack);
> + thread, sample, max_stack, false);
> }
>
> int thread__resolve_callchain(struct thread *thread,
> diff --git a/tools/perf/util/unwind-libdw.c b/tools/perf/util/unwind-libdw.c
> index a74b517f7497..94aa40f6e348 100644
> --- a/tools/perf/util/unwind-libdw.c
> +++ b/tools/perf/util/unwind-libdw.c
> @@ -200,7 +200,8 @@ frame_callback(Dwfl_Frame *state, void *arg)
> bool isactivation;
>
> if (!dwfl_frame_pc(state, &pc, NULL)) {
> - pr_err("%s", dwfl_errmsg(-1));
> + if (!ui->best_effort)
> + pr_err("%s", dwfl_errmsg(-1));
> return DWARF_CB_ABORT;
> }
>
> @@ -208,7 +209,8 @@ frame_callback(Dwfl_Frame *state, void *arg)
> report_module(pc, ui);
>
> if (!dwfl_frame_pc(state, &pc, &isactivation)) {
> - pr_err("%s", dwfl_errmsg(-1));
> + if (!ui->best_effort)
> + pr_err("%s", dwfl_errmsg(-1));
> return DWARF_CB_ABORT;
> }
>
> @@ -222,7 +224,8 @@ frame_callback(Dwfl_Frame *state, void *arg)
> int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
> struct thread *thread,
> struct perf_sample *data,
> - int max_stack)
> + int max_stack,
> + bool best_effort)
> {
> struct unwind_info *ui, ui_buf = {
> .sample = data,
> @@ -231,6 +234,7 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
> .cb = cb,
> .arg = arg,
> .max_stack = max_stack,
> + .best_effort = best_effort
> };
> Dwarf_Word ip;
> int err = -EINVAL, i;
> diff --git a/tools/perf/util/unwind-libdw.h b/tools/perf/util/unwind-libdw.h
> index 0cbd2650e280..8c88bc4f2304 100644
> --- a/tools/perf/util/unwind-libdw.h
> +++ b/tools/perf/util/unwind-libdw.h
> @@ -20,6 +20,7 @@ struct unwind_info {
> void *arg;
> int max_stack;
> int idx;
> + bool best_effort;
> struct unwind_entry entries[];
> };
>
> diff --git a/tools/perf/util/unwind-libunwind-local.c b/tools/perf/util/unwind-libunwind-local.c
> index 71a353349181..41e29fc7648a 100644
> --- a/tools/perf/util/unwind-libunwind-local.c
> +++ b/tools/perf/util/unwind-libunwind-local.c
> @@ -96,6 +96,7 @@ struct unwind_info {
> struct perf_sample *sample;
> struct machine *machine;
> struct thread *thread;
> + bool best_effort;