public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] S/390: Fix layout of struct sigaction_t
@ 2020-03-23 17:05 Stefan Liebler
  2020-04-01  6:28 ` Stefan Liebler
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Liebler @ 2020-03-23 17:05 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

Hi,

the ordering of some fields in  struct sigaction on s390x (64bit)
differs compared to s390 and other architectures.
This patch adjusts this order according to the definition of
<glibc-src>/sysdeps/unix/sysv/linux/s390/bits/sigaction.h

Without this fix e.g. the call
sigaction( suspendSignalNumber, &sigusr1, null ) in thread.d
leads to setting the sa_restorer field to 0xffffffffffffffff.
In case a signal, the signal handler returns to this address
and the process stops with a SIGILL.

This was observable in several execution testcases on s390x:
libphobos.druntime/core/thread.d
libphobos.druntime_shared/core/thread.d
libphobos.thread/tlsgc_sections.d
libphobos.allocations/tls_gc_integration.d
libphobos.phobos/std/parallelism.d
libphobos.phobos_shared/std/parallelism.d
libphobos.shared/host.c
libphobos.shared/linkD.c
libphobos.shared/linkDR.c
libphobos.shared/link_linkdep.d
libphobos.shared/load.d
libphobos.shared/loadDR.c
libphobos.shared/load_linkdep.d
libphobos.shared/load_loaddep.d

Bye,
Stefan

--

libphobos/ChangeLog:

2020-03-23  Stefan Liebler  <stli@linux.ibm.com>

	* libphobos/libdruntime/core/sys/posix/signal.d:
	Add struct sigaction_t for SystemZ.

[-- Attachment #2: 20200323_s390_struct_sigaction_t.patch --]
[-- Type: text/x-patch, Size: 3242 bytes --]

commit 0986df3847c287c144b2e38d3ec1d423b934d7a8
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Tue Mar 17 17:00:44 2020 +0100

    S/390: Fix layout of struct sigaction_t
    
    The ordering of some fields in  struct sigaction on s390x (64bit)
    differs compared to s390 and other architectures.
    This patch adjusts this order according to the definition of
    <glibc-src>/sysdeps/unix/sysv/linux/s390/bits/sigaction.h
    
    Without this fix e.g. the call
    sigaction( suspendSignalNumber, &sigusr1, null ) in thread.d
    leads to setting the sa_restorer field to 0xffffffffffffffff.
    In case a signal, the signal handler returns to this address
    and the process stops with a SIGILL.
    
    This was observable in several execution testcases on s390x:
    libphobos.druntime/core/thread.d
    libphobos.druntime_shared/core/thread.d
    libphobos.thread/tlsgc_sections.d
    libphobos.allocations/tls_gc_integration.d
    libphobos.phobos/std/parallelism.d
    libphobos.phobos_shared/std/parallelism.d
    libphobos.shared/host.c
    libphobos.shared/linkD.c
    libphobos.shared/linkDR.c
    libphobos.shared/link_linkdep.d
    libphobos.shared/load.d
    libphobos.shared/loadDR.c
    libphobos.shared/load_linkdep.d
    libphobos.shared/load_loaddep.d
    
    libphobos/ChangeLog:
    
    2020-03-23  Stefan Liebler  <stli@linux.ibm.com>
    
            * libphobos/libdruntime/core/sys/posix/signal.d:
            Add struct sigaction_t for SystemZ.

diff --git a/libphobos/libdruntime/core/sys/posix/signal.d b/libphobos/libdruntime/core/sys/posix/signal.d
index ed3985eee4d..5abcdac1355 100644
--- a/libphobos/libdruntime/core/sys/posix/signal.d
+++ b/libphobos/libdruntime/core/sys/posix/signal.d
@@ -575,24 +575,51 @@ else
 
 version (CRuntime_Glibc)
 {
-    struct sigaction_t
+    version (SystemZ)
     {
-        static if ( true /* __USE_POSIX199309 */ )
+        struct sigaction_t
         {
-            union
+            static if ( true /* __USE_POSIX199309 */ )
+            {
+                union
+                {
+                    sigfn_t     sa_handler;
+                    sigactfn_t  sa_sigaction;
+                }
+            }
+            else
             {
                 sigfn_t     sa_handler;
-                sigactfn_t  sa_sigaction;
             }
+            int             __glibc_reserved0;
+            int             sa_flags;
+
+            void function() sa_restorer;
+
+            sigset_t        sa_mask;
         }
-        else
+    }
+    else
+    {
+        struct sigaction_t
         {
-            sigfn_t     sa_handler;
-        }
-        sigset_t        sa_mask;
-        int             sa_flags;
+            static if ( true /* __USE_POSIX199309 */ )
+            {
+                union
+                {
+                    sigfn_t     sa_handler;
+                    sigactfn_t  sa_sigaction;
+                }
+            }
+            else
+            {
+                sigfn_t     sa_handler;
+            }
+            sigset_t        sa_mask;
+            int             sa_flags;
 
-        void function() sa_restorer;
+            void function() sa_restorer;
+        }
     }
 }
 else version (CRuntime_Musl)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] S/390: Fix layout of struct sigaction_t
  2020-03-23 17:05 [PATCH] S/390: Fix layout of struct sigaction_t Stefan Liebler
@ 2020-04-01  6:28 ` Stefan Liebler
  2020-04-01 10:54   ` Iain Buclaw
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Liebler @ 2020-04-01  6:28 UTC (permalink / raw)
  To: GCC Patches

ping

On 3/23/20 6:05 PM, Stefan Liebler wrote:
> Hi,
> 
> the ordering of some fields in  struct sigaction on s390x (64bit)
> differs compared to s390 and other architectures.
> This patch adjusts this order according to the definition of
> <glibc-src>/sysdeps/unix/sysv/linux/s390/bits/sigaction.h
> 
> Without this fix e.g. the call
> sigaction( suspendSignalNumber, &sigusr1, null ) in thread.d
> leads to setting the sa_restorer field to 0xffffffffffffffff.
> In case a signal, the signal handler returns to this address
> and the process stops with a SIGILL.
> 
> This was observable in several execution testcases on s390x:
> libphobos.druntime/core/thread.d
> libphobos.druntime_shared/core/thread.d
> libphobos.thread/tlsgc_sections.d
> libphobos.allocations/tls_gc_integration.d
> libphobos.phobos/std/parallelism.d
> libphobos.phobos_shared/std/parallelism.d
> libphobos.shared/host.c
> libphobos.shared/linkD.c
> libphobos.shared/linkDR.c
> libphobos.shared/link_linkdep.d
> libphobos.shared/load.d
> libphobos.shared/loadDR.c
> libphobos.shared/load_linkdep.d
> libphobos.shared/load_loaddep.d
> 
> Bye,
> Stefan
> 
> -- 
> 
> libphobos/ChangeLog:
> 
> 2020-03-23  Stefan Liebler  <stli@linux.ibm.com>
> 
>      * libphobos/libdruntime/core/sys/posix/signal.d:
>      Add struct sigaction_t for SystemZ.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] S/390: Fix layout of struct sigaction_t
  2020-04-01  6:28 ` Stefan Liebler
@ 2020-04-01 10:54   ` Iain Buclaw
  2020-04-01 16:20     ` Stefan Liebler
  0 siblings, 1 reply; 6+ messages in thread
From: Iain Buclaw @ 2020-04-01 10:54 UTC (permalink / raw)
  To: Stefan Liebler, GCC Patches

On 01/04/2020 08:28, Stefan Liebler wrote:
> ping
> 

Thanks, I'll send the patch upstream, as it's the same there.

Looks OK to me.

Regards
Iain.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] S/390: Fix layout of struct sigaction_t
  2020-04-01 10:54   ` Iain Buclaw
@ 2020-04-01 16:20     ` Stefan Liebler
  2020-04-07  7:59       ` Iain Buclaw
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Liebler @ 2020-04-01 16:20 UTC (permalink / raw)
  To: Iain Buclaw, GCC Patches

On 4/1/20 12:54 PM, Iain Buclaw wrote:
> On 01/04/2020 08:28, Stefan Liebler wrote:
>> ping
>>
> 
> Thanks, I'll send the patch upstream, as it's the same there.
> 
> Looks OK to me.
> 
> Regards
> Iain.
> 

Thanks for committing the patch upstream


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] S/390: Fix layout of struct sigaction_t
  2020-04-01 16:20     ` Stefan Liebler
@ 2020-04-07  7:59       ` Iain Buclaw
  2020-04-07 14:16         ` Andreas Krebbel
  0 siblings, 1 reply; 6+ messages in thread
From: Iain Buclaw @ 2020-04-07  7:59 UTC (permalink / raw)
  To: Stefan Liebler, GCC Patches

On 01/04/2020 18:20, Stefan Liebler wrote:
> On 4/1/20 12:54 PM, Iain Buclaw wrote:
>> On 01/04/2020 08:28, Stefan Liebler wrote:
>>> ping
>>>
>>
>> Thanks, I'll send the patch upstream, as it's the same there.
>>
>> Looks OK to me.
>>
>> Regards
>> Iain.
>>
> 
> Thanks for committing the patch upstream
> 

Hi Stefan,

They have been merged in: https://github.com/dlang/druntime/pull/3020

Do you want to commit this yourself?

Iain.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] S/390: Fix layout of struct sigaction_t
  2020-04-07  7:59       ` Iain Buclaw
@ 2020-04-07 14:16         ` Andreas Krebbel
  0 siblings, 0 replies; 6+ messages in thread
From: Andreas Krebbel @ 2020-04-07 14:16 UTC (permalink / raw)
  To: Iain Buclaw, Stefan Liebler, GCC Patches

On 07.04.20 09:59, Iain Buclaw wrote:
> On 01/04/2020 18:20, Stefan Liebler wrote:
>> On 4/1/20 12:54 PM, Iain Buclaw wrote:
>>> On 01/04/2020 08:28, Stefan Liebler wrote:
>>>> ping
>>>>
>>>
>>> Thanks, I'll send the patch upstream, as it's the same there.
>>>
>>> Looks OK to me.
>>>
>>> Regards
>>> Iain.
>>>
>>
>> Thanks for committing the patch upstream
>>
> 
> Hi Stefan,
> 
> They have been merged in: https://github.com/dlang/druntime/pull/3020
> 
> Do you want to commit this yourself?

I've committed the patch for Stefan. Thanks!

Andreas


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-04-07 14:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-23 17:05 [PATCH] S/390: Fix layout of struct sigaction_t Stefan Liebler
2020-04-01  6:28 ` Stefan Liebler
2020-04-01 10:54   ` Iain Buclaw
2020-04-01 16:20     ` Stefan Liebler
2020-04-07  7:59       ` Iain Buclaw
2020-04-07 14:16         ` Andreas Krebbel

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 read-only IMAP folder(s) and NNTP newsgroup(s).