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

[Sean Christopherson <seanjc@google.com>]

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.