* [PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers
@ 2022-05-03 22:51 Ilya Leoshkevich
2022-05-03 22:51 ` [PATCH 1/2] " Ilya Leoshkevich
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Ilya Leoshkevich @ 2022-05-03 22:51 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Laurent Vivier
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand,
Ilya Leoshkevich
Hi,
This is the fix for the issue discussed in [1].
Patch 1 fixes the issue itself, patch 2 adds a test.
[1] https://lists.nongnu.org/archive/html/qemu-devel/2022-04/msg05127.html
Best regards,
Ilya
Ilya Leoshkevich (2):
linux-user/s390x: Fix unwinding from signal handlers
tests/tcg/s390x: Test unwinding from signal handlers
linux-user/s390x/signal.c | 5 +++
tests/tcg/s390x/signals-s390x.c | 69 ++++++++++++++++++++++++++-------
2 files changed, 60 insertions(+), 14 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers
2022-05-03 22:51 [PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers Ilya Leoshkevich
@ 2022-05-03 22:51 ` Ilya Leoshkevich
2022-05-08 0:38 ` David Hildenbrand
2022-05-08 2:17 ` Richard Henderson
2022-05-03 22:51 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
2022-05-23 20:52 ` [PATCH 0/2] linux-user/s390x: Fix " Laurent Vivier
2 siblings, 2 replies; 9+ messages in thread
From: Ilya Leoshkevich @ 2022-05-03 22:51 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Laurent Vivier
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand,
Ilya Leoshkevich
Commit 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp")
removed an unused field from rt_sigframe, disturbing offsets of other
fields and breaking unwinding from signal handlers (e.g. libgcc's
s390_fallback_frame() relies on this struct having a specific layout).
Restore the field and add a comment.
Reported-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Fixes: 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp")
---
linux-user/s390x/signal.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index f47713e04a..4979c4b017 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -84,6 +84,11 @@ struct target_ucontext {
typedef struct {
uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
+ /*
+ * This field is no longer initialized by the kernel, but it's still a part
+ * of the ABI.
+ */
+ uint16_t svc_insn;
struct target_siginfo info;
struct target_ucontext uc;
} rt_sigframe;
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers
2022-05-03 22:51 [PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers Ilya Leoshkevich
2022-05-03 22:51 ` [PATCH 1/2] " Ilya Leoshkevich
@ 2022-05-03 22:51 ` Ilya Leoshkevich
2022-05-19 11:34 ` PING: " Ilya Leoshkevich
2022-05-23 20:52 ` [PATCH 0/2] linux-user/s390x: Fix " Laurent Vivier
2 siblings, 1 reply; 9+ messages in thread
From: Ilya Leoshkevich @ 2022-05-03 22:51 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Laurent Vivier
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand,
Ilya Leoshkevich
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/s390x/signals-s390x.c | 69 ++++++++++++++++++++++++++-------
1 file changed, 55 insertions(+), 14 deletions(-)
diff --git a/tests/tcg/s390x/signals-s390x.c b/tests/tcg/s390x/signals-s390x.c
index dc2f8ee59a..48c3b6cdfd 100644
--- a/tests/tcg/s390x/signals-s390x.c
+++ b/tests/tcg/s390x/signals-s390x.c
@@ -1,4 +1,5 @@
#include <assert.h>
+#include <execinfo.h>
#include <signal.h>
#include <string.h>
#include <sys/mman.h>
@@ -11,22 +12,28 @@
* inline asm is used instead.
*/
+#define DEFINE_ASM_FUNCTION(name, body) \
+ asm(".globl " #name "\n" \
+ #name ":\n" \
+ ".cfi_startproc\n" \
+ body "\n" \
+ "br %r14\n" \
+ ".cfi_endproc");
+
void illegal_op(void);
-void after_illegal_op(void);
-asm(".globl\tillegal_op\n"
- "illegal_op:\t.byte\t0x00,0x00\n"
- "\t.globl\tafter_illegal_op\n"
- "after_illegal_op:\tbr\t%r14");
+extern const char after_illegal_op;
+DEFINE_ASM_FUNCTION(illegal_op,
+ ".byte 0x00,0x00\n"
+ ".globl after_illegal_op\n"
+ "after_illegal_op:")
void stg(void *dst, unsigned long src);
-asm(".globl\tstg\n"
- "stg:\tstg\t%r3,0(%r2)\n"
- "\tbr\t%r14");
+DEFINE_ASM_FUNCTION(stg, "stg %r3,0(%r2)")
void mvc_8(void *dst, void *src);
-asm(".globl\tmvc_8\n"
- "mvc_8:\tmvc\t0(8,%r2),0(%r3)\n"
- "\tbr\t%r14");
+DEFINE_ASM_FUNCTION(mvc_8, "mvc 0(8,%r2),0(%r3)")
+
+extern const char return_from_main_1;
static void safe_puts(const char *s)
{
@@ -49,8 +56,9 @@ static struct {
static void handle_signal(int sig, siginfo_t *info, void *ucontext)
{
+ int err, i, n_frames;
+ void *frames[16];
void *page;
- int err;
if (sig != expected.sig) {
safe_puts("[ FAILED ] wrong signal");
@@ -86,6 +94,17 @@ static void handle_signal(int sig, siginfo_t *info, void *ucontext)
default:
break;
}
+
+ n_frames = backtrace(frames, sizeof(frames) / sizeof(frames[0]));
+ for (i = 0; i < n_frames; i++) {
+ if (frames[i] == &return_from_main_1) {
+ break;
+ }
+ }
+ if (i == n_frames) {
+ safe_puts("[ FAILED ] backtrace() is broken");
+ _exit(1);
+ }
}
static void check_sigsegv(void *func, enum exception exception,
@@ -122,7 +141,7 @@ static void check_sigsegv(void *func, enum exception exception,
assert(err == 0);
}
-int main(void)
+int main_1(void)
{
struct sigaction act;
int err;
@@ -138,7 +157,7 @@ int main(void)
safe_puts("[ RUN ] Operation exception");
expected.sig = SIGILL;
expected.addr = illegal_op;
- expected.psw_addr = (unsigned long)after_illegal_op;
+ expected.psw_addr = (unsigned long)&after_illegal_op;
expected.exception = exception_operation;
illegal_op();
safe_puts("[ OK ]");
@@ -163,3 +182,25 @@ int main(void)
_exit(0);
}
+
+/*
+ * Define main() in assembly in order to test that unwinding from signal
+ * handlers until main() works. This way we can define a specific point that
+ * the unwinder should reach. This is also better than defining main() in C
+ * and using inline assembly to call main_1(), since it's not easy to get all
+ * the clobbers right.
+ */
+
+DEFINE_ASM_FUNCTION(main,
+ "stmg %r14,%r15,112(%r15)\n"
+ ".cfi_offset 14,-48\n"
+ ".cfi_offset 15,-40\n"
+ "lay %r15,-160(%r15)\n"
+ ".cfi_def_cfa_offset 320\n"
+ "brasl %r14,main_1\n"
+ ".globl return_from_main_1\n"
+ "return_from_main_1:\n"
+ "lmg %r14,%r15,272(%r15)\n"
+ ".cfi_restore 15\n"
+ ".cfi_restore 14\n"
+ ".cfi_def_cfa_offset 160");
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers
2022-05-03 22:51 ` [PATCH 1/2] " Ilya Leoshkevich
@ 2022-05-08 0:38 ` David Hildenbrand
2022-05-08 2:17 ` Richard Henderson
1 sibling, 0 replies; 9+ messages in thread
From: David Hildenbrand @ 2022-05-08 0:38 UTC (permalink / raw)
To: Ilya Leoshkevich, Richard Henderson, Laurent Vivier
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand
On 04.05.22 00:51, Ilya Leoshkevich wrote:
> Commit 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp")
> removed an unused field from rt_sigframe, disturbing offsets of other
> fields and breaking unwinding from signal handlers (e.g. libgcc's
> s390_fallback_frame() relies on this struct having a specific layout).
> Restore the field and add a comment.
>
> Reported-by: Ulrich Weigand <ulrich.weigand@de.ibm.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Fixes: 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp")
> ---
> linux-user/s390x/signal.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
> index f47713e04a..4979c4b017 100644
> --- a/linux-user/s390x/signal.c
> +++ b/linux-user/s390x/signal.c
> @@ -84,6 +84,11 @@ struct target_ucontext {
>
> typedef struct {
> uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
> + /*
> + * This field is no longer initialized by the kernel, but it's still a part
> + * of the ABI.
> + */
> + uint16_t svc_insn;
> struct target_siginfo info;
> struct target_ucontext uc;
> } rt_sigframe;
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers
2022-05-03 22:51 ` [PATCH 1/2] " Ilya Leoshkevich
2022-05-08 0:38 ` David Hildenbrand
@ 2022-05-08 2:17 ` Richard Henderson
1 sibling, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2022-05-08 2:17 UTC (permalink / raw)
To: Ilya Leoshkevich, David Hildenbrand, Laurent Vivier
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand
On 5/3/22 17:51, Ilya Leoshkevich wrote:
> Commit 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp")
> removed an unused field from rt_sigframe, disturbing offsets of other
> fields and breaking unwinding from signal handlers (e.g. libgcc's
> s390_fallback_frame() relies on this struct having a specific layout).
> Restore the field and add a comment.
>
> Reported-by: Ulrich Weigand<ulrich.weigand@de.ibm.com>
> Signed-off-by: Ilya Leoshkevich<iii@linux.ibm.com>
> Fixes: 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp")
> ---
> linux-user/s390x/signal.c | 5 +++++
> 1 file changed, 5 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* PING: [PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers
2022-05-03 22:51 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
@ 2022-05-19 11:34 ` Ilya Leoshkevich
2022-05-24 9:56 ` Thomas Huth
0 siblings, 1 reply; 9+ messages in thread
From: Ilya Leoshkevich @ 2022-05-19 11:34 UTC (permalink / raw)
To: Richard Henderson, David Hildenbrand, Laurent Vivier
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand
On Wed, 2022-05-04 at 00:51 +0200, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tests/tcg/s390x/signals-s390x.c | 69 ++++++++++++++++++++++++++-----
> --
> 1 file changed, 55 insertions(+), 14 deletions(-)
>
> diff --git a/tests/tcg/s390x/signals-s390x.c
> b/tests/tcg/s390x/signals-s390x.c
> index dc2f8ee59a..48c3b6cdfd 100644
> --- a/tests/tcg/s390x/signals-s390x.c
> +++ b/tests/tcg/s390x/signals-s390x.c
> @@ -1,4 +1,5 @@
> #include <assert.h>
> +#include <execinfo.h>
> #include <signal.h>
> #include <string.h>
> #include <sys/mman.h>
> @@ -11,22 +12,28 @@
> * inline asm is used instead.
> */
>
> +#define DEFINE_ASM_FUNCTION(name, body) \
> + asm(".globl " #name "\n" \
> + #name ":\n" \
> + ".cfi_startproc\n" \
> + body "\n" \
> + "br %r14\n" \
> + ".cfi_endproc");
> +
> void illegal_op(void);
> -void after_illegal_op(void);
> -asm(".globl\tillegal_op\n"
> - "illegal_op:\t.byte\t0x00,0x00\n"
> - "\t.globl\tafter_illegal_op\n"
> - "after_illegal_op:\tbr\t%r14");
> +extern const char after_illegal_op;
> +DEFINE_ASM_FUNCTION(illegal_op,
> + ".byte 0x00,0x00\n"
> + ".globl after_illegal_op\n"
> + "after_illegal_op:")
>
> void stg(void *dst, unsigned long src);
> -asm(".globl\tstg\n"
> - "stg:\tstg\t%r3,0(%r2)\n"
> - "\tbr\t%r14");
> +DEFINE_ASM_FUNCTION(stg, "stg %r3,0(%r2)")
>
> void mvc_8(void *dst, void *src);
> -asm(".globl\tmvc_8\n"
> - "mvc_8:\tmvc\t0(8,%r2),0(%r3)\n"
> - "\tbr\t%r14");
> +DEFINE_ASM_FUNCTION(mvc_8, "mvc 0(8,%r2),0(%r3)")
> +
> +extern const char return_from_main_1;
>
> static void safe_puts(const char *s)
> {
> @@ -49,8 +56,9 @@ static struct {
>
> static void handle_signal(int sig, siginfo_t *info, void *ucontext)
> {
> + int err, i, n_frames;
> + void *frames[16];
> void *page;
> - int err;
>
> if (sig != expected.sig) {
> safe_puts("[ FAILED ] wrong signal");
> @@ -86,6 +94,17 @@ static void handle_signal(int sig, siginfo_t
> *info, void *ucontext)
> default:
> break;
> }
> +
> + n_frames = backtrace(frames, sizeof(frames) /
> sizeof(frames[0]));
> + for (i = 0; i < n_frames; i++) {
> + if (frames[i] == &return_from_main_1) {
> + break;
> + }
> + }
> + if (i == n_frames) {
> + safe_puts("[ FAILED ] backtrace() is broken");
> + _exit(1);
> + }
> }
>
> static void check_sigsegv(void *func, enum exception exception,
> @@ -122,7 +141,7 @@ static void check_sigsegv(void *func, enum
> exception exception,
> assert(err == 0);
> }
>
> -int main(void)
> +int main_1(void)
> {
> struct sigaction act;
> int err;
> @@ -138,7 +157,7 @@ int main(void)
> safe_puts("[ RUN ] Operation exception");
> expected.sig = SIGILL;
> expected.addr = illegal_op;
> - expected.psw_addr = (unsigned long)after_illegal_op;
> + expected.psw_addr = (unsigned long)&after_illegal_op;
> expected.exception = exception_operation;
> illegal_op();
> safe_puts("[ OK ]");
> @@ -163,3 +182,25 @@ int main(void)
>
> _exit(0);
> }
> +
> +/*
> + * Define main() in assembly in order to test that unwinding from
> signal
> + * handlers until main() works. This way we can define a specific
> point that
> + * the unwinder should reach. This is also better than defining
> main() in C
> + * and using inline assembly to call main_1(), since it's not easy
> to get all
> + * the clobbers right.
> + */
> +
> +DEFINE_ASM_FUNCTION(main,
> + "stmg %r14,%r15,112(%r15)\n"
> + ".cfi_offset 14,-48\n"
> + ".cfi_offset 15,-40\n"
> + "lay %r15,-160(%r15)\n"
> + ".cfi_def_cfa_offset 320\n"
> + "brasl %r14,main_1\n"
> + ".globl return_from_main_1\n"
> + "return_from_main_1:\n"
> + "lmg %r14,%r15,272(%r15)\n"
> + ".cfi_restore 15\n"
> + ".cfi_restore 14\n"
> + ".cfi_def_cfa_offset 160");
Ping.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers
2022-05-03 22:51 [PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers Ilya Leoshkevich
2022-05-03 22:51 ` [PATCH 1/2] " Ilya Leoshkevich
2022-05-03 22:51 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
@ 2022-05-23 20:52 ` Laurent Vivier
2 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2022-05-23 20:52 UTC (permalink / raw)
To: Ilya Leoshkevich, Richard Henderson, David Hildenbrand
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand
Le 04/05/2022 à 00:51, Ilya Leoshkevich a écrit :
> Hi,
>
> This is the fix for the issue discussed in [1].
> Patch 1 fixes the issue itself, patch 2 adds a test.
>
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-04/msg05127.html
>
> Best regards,
> Ilya
>
> Ilya Leoshkevich (2):
> linux-user/s390x: Fix unwinding from signal handlers
> tests/tcg/s390x: Test unwinding from signal handlers
>
> linux-user/s390x/signal.c | 5 +++
> tests/tcg/s390x/signals-s390x.c | 69 ++++++++++++++++++++++++++-------
> 2 files changed, 60 insertions(+), 14 deletions(-)
>
Series applied to my linux-user-for-7.1 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PING: [PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers
2022-05-19 11:34 ` PING: " Ilya Leoshkevich
@ 2022-05-24 9:56 ` Thomas Huth
2022-05-24 16:07 ` Laurent Vivier
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Huth @ 2022-05-24 9:56 UTC (permalink / raw)
To: Ilya Leoshkevich, Richard Henderson, David Hildenbrand, Laurent Vivier
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand
On 19/05/2022 13.34, Ilya Leoshkevich wrote:
> On Wed, 2022-05-04 at 00:51 +0200, Ilya Leoshkevich wrote:
>> Add a small test to prevent regressions.
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>> tests/tcg/s390x/signals-s390x.c | 69 ++++++++++++++++++++++++++-----
>> --
>> 1 file changed, 55 insertions(+), 14 deletions(-)
>>
>> diff --git a/tests/tcg/s390x/signals-s390x.c
>> b/tests/tcg/s390x/signals-s390x.c
>> index dc2f8ee59a..48c3b6cdfd 100644
>> --- a/tests/tcg/s390x/signals-s390x.c
>> +++ b/tests/tcg/s390x/signals-s390x.c
>> @@ -1,4 +1,5 @@
>> #include <assert.h>
>> +#include <execinfo.h>
>> #include <signal.h>
>> #include <string.h>
>> #include <sys/mman.h>
>> @@ -11,22 +12,28 @@
>> * inline asm is used instead.
>> */
>>
>> +#define DEFINE_ASM_FUNCTION(name, body) \
>> + asm(".globl " #name "\n" \
>> + #name ":\n" \
>> + ".cfi_startproc\n" \
>> + body "\n" \
>> + "br %r14\n" \
>> + ".cfi_endproc");
>> +
>> void illegal_op(void);
>> -void after_illegal_op(void);
>> -asm(".globl\tillegal_op\n"
>> - "illegal_op:\t.byte\t0x00,0x00\n"
>> - "\t.globl\tafter_illegal_op\n"
>> - "after_illegal_op:\tbr\t%r14");
>> +extern const char after_illegal_op;
>> +DEFINE_ASM_FUNCTION(illegal_op,
>> + ".byte 0x00,0x00\n"
>> + ".globl after_illegal_op\n"
>> + "after_illegal_op:")
>>
>> void stg(void *dst, unsigned long src);
>> -asm(".globl\tstg\n"
>> - "stg:\tstg\t%r3,0(%r2)\n"
>> - "\tbr\t%r14");
>> +DEFINE_ASM_FUNCTION(stg, "stg %r3,0(%r2)")
>>
>> void mvc_8(void *dst, void *src);
>> -asm(".globl\tmvc_8\n"
>> - "mvc_8:\tmvc\t0(8,%r2),0(%r3)\n"
>> - "\tbr\t%r14");
>> +DEFINE_ASM_FUNCTION(mvc_8, "mvc 0(8,%r2),0(%r3)")
>> +
>> +extern const char return_from_main_1;
>>
>> static void safe_puts(const char *s)
>> {
>> @@ -49,8 +56,9 @@ static struct {
>>
>> static void handle_signal(int sig, siginfo_t *info, void *ucontext)
>> {
>> + int err, i, n_frames;
>> + void *frames[16];
>> void *page;
>> - int err;
>>
>> if (sig != expected.sig) {
>> safe_puts("[ FAILED ] wrong signal");
>> @@ -86,6 +94,17 @@ static void handle_signal(int sig, siginfo_t
>> *info, void *ucontext)
>> default:
>> break;
>> }
>> +
>> + n_frames = backtrace(frames, sizeof(frames) /
>> sizeof(frames[0]));
>> + for (i = 0; i < n_frames; i++) {
>> + if (frames[i] == &return_from_main_1) {
>> + break;
>> + }
>> + }
>> + if (i == n_frames) {
>> + safe_puts("[ FAILED ] backtrace() is broken");
>> + _exit(1);
>> + }
>> }
>>
>> static void check_sigsegv(void *func, enum exception exception,
>> @@ -122,7 +141,7 @@ static void check_sigsegv(void *func, enum
>> exception exception,
>> assert(err == 0);
>> }
>>
>> -int main(void)
>> +int main_1(void)
>> {
>> struct sigaction act;
>> int err;
>> @@ -138,7 +157,7 @@ int main(void)
>> safe_puts("[ RUN ] Operation exception");
>> expected.sig = SIGILL;
>> expected.addr = illegal_op;
>> - expected.psw_addr = (unsigned long)after_illegal_op;
>> + expected.psw_addr = (unsigned long)&after_illegal_op;
>> expected.exception = exception_operation;
>> illegal_op();
>> safe_puts("[ OK ]");
>> @@ -163,3 +182,25 @@ int main(void)
>>
>> _exit(0);
>> }
>> +
>> +/*
>> + * Define main() in assembly in order to test that unwinding from
>> signal
>> + * handlers until main() works. This way we can define a specific
>> point that
>> + * the unwinder should reach. This is also better than defining
>> main() in C
>> + * and using inline assembly to call main_1(), since it's not easy
>> to get all
>> + * the clobbers right.
>> + */
>> +
>> +DEFINE_ASM_FUNCTION(main,
>> + "stmg %r14,%r15,112(%r15)\n"
>> + ".cfi_offset 14,-48\n"
>> + ".cfi_offset 15,-40\n"
>> + "lay %r15,-160(%r15)\n"
>> + ".cfi_def_cfa_offset 320\n"
>> + "brasl %r14,main_1\n"
>> + ".globl return_from_main_1\n"
>> + "return_from_main_1:\n"
>> + "lmg %r14,%r15,272(%r15)\n"
>> + ".cfi_restore 15\n"
>> + ".cfi_restore 14\n"
>> + ".cfi_def_cfa_offset 160");
>
> Ping.
Acked-by: Thomas Huth <thuth@redhat.com>
Laurent, do you want to take these two patches through your linux-user
branch, or shall I take them via the s390x branch?
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PING: [PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers
2022-05-24 9:56 ` Thomas Huth
@ 2022-05-24 16:07 ` Laurent Vivier
0 siblings, 0 replies; 9+ messages in thread
From: Laurent Vivier @ 2022-05-24 16:07 UTC (permalink / raw)
To: Thomas Huth, Ilya Leoshkevich, Richard Henderson, David Hildenbrand
Cc: qemu-s390x, qemu-devel, Christian Borntraeger, Ulrich Weigand
Le 24/05/2022 à 11:56, Thomas Huth a écrit :
> On 19/05/2022 13.34, Ilya Leoshkevich wrote:
>> On Wed, 2022-05-04 at 00:51 +0200, Ilya Leoshkevich wrote:
>>> Add a small test to prevent regressions.
>>>
>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>> ---
>>> tests/tcg/s390x/signals-s390x.c | 69 ++++++++++++++++++++++++++-----
>>> --
>>> 1 file changed, 55 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/tests/tcg/s390x/signals-s390x.c
>>> b/tests/tcg/s390x/signals-s390x.c
>>> index dc2f8ee59a..48c3b6cdfd 100644
>>> --- a/tests/tcg/s390x/signals-s390x.c
>>> +++ b/tests/tcg/s390x/signals-s390x.c
>>> @@ -1,4 +1,5 @@
>>> #include <assert.h>
>>> +#include <execinfo.h>
>>> #include <signal.h>
>>> #include <string.h>
>>> #include <sys/mman.h>
>>> @@ -11,22 +12,28 @@
>>> * inline asm is used instead.
>>> */
>>> +#define DEFINE_ASM_FUNCTION(name, body) \
>>> + asm(".globl " #name "\n" \
>>> + #name ":\n" \
>>> + ".cfi_startproc\n" \
>>> + body "\n" \
>>> + "br %r14\n" \
>>> + ".cfi_endproc");
>>> +
>>> void illegal_op(void);
>>> -void after_illegal_op(void);
>>> -asm(".globl\tillegal_op\n"
>>> - "illegal_op:\t.byte\t0x00,0x00\n"
>>> - "\t.globl\tafter_illegal_op\n"
>>> - "after_illegal_op:\tbr\t%r14");
>>> +extern const char after_illegal_op;
>>> +DEFINE_ASM_FUNCTION(illegal_op,
>>> + ".byte 0x00,0x00\n"
>>> + ".globl after_illegal_op\n"
>>> + "after_illegal_op:")
>>> void stg(void *dst, unsigned long src);
>>> -asm(".globl\tstg\n"
>>> - "stg:\tstg\t%r3,0(%r2)\n"
>>> - "\tbr\t%r14");
>>> +DEFINE_ASM_FUNCTION(stg, "stg %r3,0(%r2)")
>>> void mvc_8(void *dst, void *src);
>>> -asm(".globl\tmvc_8\n"
>>> - "mvc_8:\tmvc\t0(8,%r2),0(%r3)\n"
>>> - "\tbr\t%r14");
>>> +DEFINE_ASM_FUNCTION(mvc_8, "mvc 0(8,%r2),0(%r3)")
>>> +
>>> +extern const char return_from_main_1;
>>> static void safe_puts(const char *s)
>>> {
>>> @@ -49,8 +56,9 @@ static struct {
>>> static void handle_signal(int sig, siginfo_t *info, void *ucontext)
>>> {
>>> + int err, i, n_frames;
>>> + void *frames[16];
>>> void *page;
>>> - int err;
>>> if (sig != expected.sig) {
>>> safe_puts("[ FAILED ] wrong signal");
>>> @@ -86,6 +94,17 @@ static void handle_signal(int sig, siginfo_t
>>> *info, void *ucontext)
>>> default:
>>> break;
>>> }
>>> +
>>> + n_frames = backtrace(frames, sizeof(frames) /
>>> sizeof(frames[0]));
>>> + for (i = 0; i < n_frames; i++) {
>>> + if (frames[i] == &return_from_main_1) {
>>> + break;
>>> + }
>>> + }
>>> + if (i == n_frames) {
>>> + safe_puts("[ FAILED ] backtrace() is broken");
>>> + _exit(1);
>>> + }
>>> }
>>> static void check_sigsegv(void *func, enum exception exception,
>>> @@ -122,7 +141,7 @@ static void check_sigsegv(void *func, enum
>>> exception exception,
>>> assert(err == 0);
>>> }
>>> -int main(void)
>>> +int main_1(void)
>>> {
>>> struct sigaction act;
>>> int err;
>>> @@ -138,7 +157,7 @@ int main(void)
>>> safe_puts("[ RUN ] Operation exception");
>>> expected.sig = SIGILL;
>>> expected.addr = illegal_op;
>>> - expected.psw_addr = (unsigned long)after_illegal_op;
>>> + expected.psw_addr = (unsigned long)&after_illegal_op;
>>> expected.exception = exception_operation;
>>> illegal_op();
>>> safe_puts("[ OK ]");
>>> @@ -163,3 +182,25 @@ int main(void)
>>> _exit(0);
>>> }
>>> +
>>> +/*
>>> + * Define main() in assembly in order to test that unwinding from
>>> signal
>>> + * handlers until main() works. This way we can define a specific
>>> point that
>>> + * the unwinder should reach. This is also better than defining
>>> main() in C
>>> + * and using inline assembly to call main_1(), since it's not easy
>>> to get all
>>> + * the clobbers right.
>>> + */
>>> +
>>> +DEFINE_ASM_FUNCTION(main,
>>> + "stmg %r14,%r15,112(%r15)\n"
>>> + ".cfi_offset 14,-48\n"
>>> + ".cfi_offset 15,-40\n"
>>> + "lay %r15,-160(%r15)\n"
>>> + ".cfi_def_cfa_offset 320\n"
>>> + "brasl %r14,main_1\n"
>>> + ".globl return_from_main_1\n"
>>> + "return_from_main_1:\n"
>>> + "lmg %r14,%r15,272(%r15)\n"
>>> + ".cfi_restore 15\n"
>>> + ".cfi_restore 14\n"
>>> + ".cfi_def_cfa_offset 160");
>>
>> Ping.
>
> Acked-by: Thomas Huth <thuth@redhat.com>
>
> Laurent, do you want to take these two patches through your linux-user branch, or shall I take them
> via the s390x branch?
I will take both.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-05-24 16:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-03 22:51 [PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers Ilya Leoshkevich
2022-05-03 22:51 ` [PATCH 1/2] " Ilya Leoshkevich
2022-05-08 0:38 ` David Hildenbrand
2022-05-08 2:17 ` Richard Henderson
2022-05-03 22:51 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
2022-05-19 11:34 ` PING: " Ilya Leoshkevich
2022-05-24 9:56 ` Thomas Huth
2022-05-24 16:07 ` Laurent Vivier
2022-05-23 20:52 ` [PATCH 0/2] linux-user/s390x: Fix " Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).