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

Paolo Bonzini
2022-09-27T13:51:32Z

On 9/26/22 23:47, David Matlack wrote:
> How about this?
>
> diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
> index 71ade6100fd3..2b56bbff970c 100644
> --- a/tools/testing/selftests/kvm/lib/assert.c
> +++ b/tools/testing/selftests/kvm/lib/assert.c
> @@ -38,16 +38,28 @@ 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.
> - */
> - for (i = 2; i < n; i++)
> - c += sprintf(c, " %lx", ((unsigned long) stack[i]) - 1);
> +
> + n = backtrace(stack, n);
> + if (n > 2) {
> + /*
> + * Skip the first 2 frames, which should be test_dump_stack()
> + * and test_assert(); both of which are declared noinline.
> + */
> + for (i = 2; i < n; i++)
> + c += sprintf(c, " %lx", ((unsigned long) stack[i]) - 1);
> + } else {
> + /*
> + * Bail if the resulting stack trace would be empty. Otherwise,
> + * addr2line will block waiting for addresses to be passed in
> + * via stdin.
> + */
> + fputs(" (stack trace missing)\n", stderr);
> + return;
> + }
> +
> c += sprintf(c, "%s", pipeline);
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wunused-result"
>

I think your original patch is better, just with

diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
index c1ce54a41eca..be2cab00f541 100644
--- a/tools/testing/selftests/kvm/lib/assert.c
+++ b/tools/testing/selftests/kvm/lib/assert.c
@@ -36,11 +36,9 @@ static void test_dump_stack(void)
n * (((sizeof(void *)) * 2) + 1) +
/* Null terminator: */
1];
- char *c;
+ char *c = cmd;

n = backtrace(stack, n);
- c = &cmd[0];
- c += sprintf(c, "%s", addr2line);
/*
* Skip the first 2 frames, which should be test_dump_stack() and
* test_assert(); both of which are declared noinline. Bail if the
@@ -51,6 +49,8 @@ static void test_dump_stack(void)
fputs(" (stack trace empty)\n", stderr);
return;
}
+
+ c += sprintf(c, "%s", addr2line);
for (i = 2; i < n; i++)
c += sprintf(c, " %lx", ((unsigned long) stack[i]) - 1);

squashed in to keep the "if" and backtrace() call as close as possible.

Paolo