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

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

[David Matlack]

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, and move the call to backtrace()
down to where it is used.

Cc: Vipin Sharma <vipinsh@google.com>
Cc: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
v2:
 - Move backtrace() down to where it is used [Vipin]
 - Change "stack trace empty" to "stack trace missing" [me]

v1: https://lore.kernel.org/kvm/20220922231724.3560211-1-dmatlack@google.com/

 tools/testing/selftests/kvm/lib/assert.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
index 71ade6100fd3..7b92d1aaeda6 100644
--- a/tools/testing/selftests/kvm/lib/assert.c
+++ b/tools/testing/selftests/kvm/lib/assert.c
@@ -38,16 +38,23 @@ static void test_dump_stack(void)
 		 1];
 	char *c;
 
-	n = backtrace(stack, n);
 	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.
 	 */
+	n = backtrace(stack, n);
+	if (n <= 2) {
+		fputs("  (stack trace missing)\n", stderr);
+		return;
+	}
 	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

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

[Vipin Sharma]

On Tue, Sep 27, 2022 at 12:05 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, and move the call to backtrace()
> down to where it is used.
>
> Cc: Vipin Sharma <vipinsh@google.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
> v2:
>  - Move backtrace() down to where it is used [Vipin]
>  - Change "stack trace empty" to "stack trace missing" [me]
>
> v1: https://lore.kernel.org/kvm/20220922231724.3560211-1-dmatlack@google.com/
>
>  tools/testing/selftests/kvm/lib/assert.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
> index 71ade6100fd3..7b92d1aaeda6 100644
> --- a/tools/testing/selftests/kvm/lib/assert.c
> +++ b/tools/testing/selftests/kvm/lib/assert.c
> @@ -38,16 +38,23 @@ static void test_dump_stack(void)
>                  1];
>         char *c;
>
> -       n = backtrace(stack, n);
>         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.
>          */
> +       n = backtrace(stack, n);
> +       if (n <= 2) {
> +               fputs("  (stack trace missing)\n", stderr);
> +               return;
> +       }
>         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
>

Reviewed-by: Vipin Sharma <vipinsh@google.com>

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

[Sean Christopherson]

On Tue, Sep 27, 2022, David Matlack 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, and move the call to backtrace()
> down to where it is used.
> 
> Cc: Vipin Sharma <vipinsh@google.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---

In case Paolo sees the error of his ways ;-)

Reviewed-by: Sean Christopherson <seanjc@google.com>