[PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers

[PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers

[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

[PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers

[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

Re: [PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers

[David Hildenbrand]

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

Re: [PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers

[Richard Henderson]

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~

[PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers

[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

PING: [PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers

[Ilya Leoshkevich]

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.

Re: PING: [PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers

[Thomas Huth]

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

Re: PING: [PATCH 2/2] tests/tcg/s390x: Test unwinding from signal handlers

[Laurent Vivier]

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

Re: [PATCH 0/2] linux-user/s390x: Fix unwinding from signal handlers

[Laurent Vivier]

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