Re: [PATCH] KVM: selftests: Gracefully handle empty stack traces

Vipin Sharma
2022-09-26T20:43:02Z

On Thu, Sep 22, 2022 at 4:17 PM David Matlack <dmatlack@google.com> wrote:
>
> Bail out of test_dump_stack() if the stack trace is empty rather than
> invoking addr2line with zero addresses. The problem with the latter is
> that addr2line will block waiting for addresses to be passed in via
> stdin, e.g. if running a selftest from an interactive terminal.
>
> Opportunistically fix up the comment that mentions skipping 3 frames
> since only 2 are skipped in the code.
>
> Cc: Vipin Sharma <vipinsh@google.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
> tools/testing/selftests/kvm/lib/assert.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
> index 71ade6100fd3..c1ce54a41eca 100644
> --- a/tools/testing/selftests/kvm/lib/assert.c
> +++ b/tools/testing/selftests/kvm/lib/assert.c
> @@ -42,12 +42,18 @@ static void test_dump_stack(void)
> c = &cmd[0];
> c += sprintf(c, "%s", addr2line);
> /*
> - * Skip the first 3 frames: backtrace, test_dump_stack, and
> - * test_assert. We hope that backtrace isn't inlined and the other two
> - * we've declared noinline.
> + * Skip the first 2 frames, which should be test_dump_stack() and
> + * test_assert(); both of which are declared noinline. Bail if the
> + * resulting stack trace would be empty. Otherwise, addr2line will block
> + * waiting for addresses to be passed in via stdin.
> */
> + if (n <= 2) {
> + fputs(" (stack trace empty)\n", stderr);
> + return;
> + }

Shouldn't this condition be put immediately after
n = backtrace(stack,n)

It is more natural to check the return value when an API has returned.

Verified that this change does fix the issue. Thanks for the fix.

> for (i = 2; i < n; i++)
> c += sprintf(c, " %lx", ((unsigned long) stack[i]) - 1);
> +
> c += sprintf(c, "%s", pipeline);
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wunused-result"
>
> base-commit: 372d07084593dc7a399bf9bee815711b1fb1bcf2
> prerequisite-patch-id: 2e3661ba8856c29b769499bac525b6943d9284b8
> prerequisite-patch-id: 1a148d98d96d73a520ed070260608ddf1bdd0f08
> --
> 2.37.3.998.g577e59143f-goog
>