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

Sean Christopherson
2022-09-26T20:54:03Z

On Mon, Sep 26, 2022, Vipin Sharma wrote:
> 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.

How does this bug occur? Does backtrace() get inlined?

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

Agreed, that would be more intuitive.