[1/5] m68k: Fix sigaction kernel definition (BZ #23960)

Message ID 20181211195554.3377-1-adhemerval.zanella@linaro.org
State New
Headers show
Series
  • [1/5] m68k: Fix sigaction kernel definition (BZ #23960)
Related show

Commit Message

Adhemerval Zanella Dec. 11, 2018, 7:55 p.m.
Commit b4a5d26d883 (linux: Consolidate sigaction implementation) added
a wrong kernel_sigaction definition for m68k, meant for __NR_sigaction
instead of __NR_rt_sigaction as used on generic Linux sigaction
implementation.  This patch fixes it by using the Linux generic
definition meant for the RT kernel ABI.

Checked the signal tests on emulated m68-linux-gnu (Aranym).  It fixes
the faulty signal/tst-sigaction and man works as expected.

	James Clarke  <jrtc27@jrtc27.com>
	Adhemerval Zanella  <adhemerval.zanella@linaro.org>

	[BZ #23960]
	* sysdeps/unix/sysv/linux/kernel_sigaction.h (kernel_sigaction): Add
	comment about the difference due glibc definition.
	* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (kernel_sigaction):
	Use Linux generic definition.
---
 ChangeLog                                      |  9 +++++++++
 sysdeps/unix/sysv/linux/kernel_sigaction.h     |  2 ++
 .../unix/sysv/linux/m68k/kernel_sigaction.h    | 18 ++----------------
 3 files changed, 13 insertions(+), 16 deletions(-)

-- 
2.17.1

Comments

Andreas Schwab Dec. 11, 2018, 9:26 p.m. | #1
On Dez 11 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h

> index 54972feb13..eef4bb9b65 100644

> --- a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h

> +++ b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h

> @@ -1,22 +1,8 @@

> -#ifndef _KERNEL_SIGACTION_H

> -# define _KERNEL_SIGACTION_H

> -

> -#include <signal.h>

> -

> +/* m68k uses the generic Linux UAPI but defines SA_RESTORER.  */

>  #define SA_RESTORER 0x04000000

> -

> -/* This is the sigaction structure from the Linux 3.2 kernel.  */

> -struct kernel_sigaction

> -{

> -  __sighandler_t k_sa_handler;

> -  sigset_t sa_mask;

> -  unsigned long sa_flags;

> -  void (*sa_restorer) (void);

> -};

> +#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>

>  

>  #define SET_SA_RESTORER(kact, act)			\

>    (kact)->sa_restorer = (act)->sa_restorer

>  #define RESET_SA_RESTORER(act, kact)			\

>    (act)->sa_restorer = (kact)->sa_restorer

> -

> -#endif


There should be no need to read or set sa_restorer.  The kernel does not
use it, and it also doesn't define SA_RESTORER.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Adhemerval Zanella Dec. 12, 2018, 10:26 a.m. | #2
On 11/12/2018 19:26, Andreas Schwab wrote:
> On Dez 11 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> 

>> diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h

>> index 54972feb13..eef4bb9b65 100644

>> --- a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h

>> +++ b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h

>> @@ -1,22 +1,8 @@

>> -#ifndef _KERNEL_SIGACTION_H

>> -# define _KERNEL_SIGACTION_H

>> -

>> -#include <signal.h>

>> -

>> +/* m68k uses the generic Linux UAPI but defines SA_RESTORER.  */

>>  #define SA_RESTORER 0x04000000

>> -

>> -/* This is the sigaction structure from the Linux 3.2 kernel.  */

>> -struct kernel_sigaction

>> -{

>> -  __sighandler_t k_sa_handler;

>> -  sigset_t sa_mask;

>> -  unsigned long sa_flags;

>> -  void (*sa_restorer) (void);

>> -};

>> +#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>

>>  

>>  #define SET_SA_RESTORER(kact, act)			\

>>    (kact)->sa_restorer = (act)->sa_restorer

>>  #define RESET_SA_RESTORER(act, kact)			\

>>    (act)->sa_restorer = (kact)->sa_restorer

>> -

>> -#endif

> 

> There should be no need to read or set sa_restorer.  The kernel does not

> use it, and it also doesn't define SA_RESTORER.


The m68k kernel does define SA_RESTORER on arch/m68k/include/asm/signal.h
(__ARCH_HAS_SA_RESTORER), but the only difference it makes for m68k is at
flush_signal_handlers where kernel sets the sa_restorer to NULL. So it
indeed should be safe to just use default kernel_sigaction.h (I don't see
any signal test issues). Updated patch below.

--

	James Clarke  <jrtc27@jrtc27.com>
	Adhemerval Zanella  <adhemerval.zanella@linaro.org>

	[BZ #23960]
	* sysdeps/unix/sysv/linux/kernel_sigaction.h (kernel_sigaction): Add
	comment about the difference due glibc definition.
	* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h: Remove file.

---

diff --git a/sysdeps/unix/sysv/linux/kernel_sigaction.h b/sysdeps/unix/sysv/linux/kernel_sigaction.h
index 2dbec08099..b7359054b2 100644
--- a/sysdeps/unix/sysv/linux/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/kernel_sigaction.h
@@ -9,6 +9,8 @@ struct kernel_sigaction
 #ifdef SA_RESTORER
   void (*sa_restorer) (void);
 #endif
+  /* glibc sigset is larger than kernel expected one, however sigaction
+     passes the kernel expected size on rt_sigaction syscall.  */
   sigset_t sa_mask;
 };
 
diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
deleted file mode 100644
index 54972feb13..0000000000
--- a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
+++ /dev/null
@@ -1,22 +0,0 @@
-#ifndef _KERNEL_SIGACTION_H
-# define _KERNEL_SIGACTION_H
-
-#include <signal.h>
-
-#define SA_RESTORER 0x04000000
-
-/* This is the sigaction structure from the Linux 3.2 kernel.  */
-struct kernel_sigaction
-{
-  __sighandler_t k_sa_handler;
-  sigset_t sa_mask;
-  unsigned long sa_flags;
-  void (*sa_restorer) (void);
-};
-
-#define SET_SA_RESTORER(kact, act)			\
-  (kact)->sa_restorer = (act)->sa_restorer
-#define RESET_SA_RESTORER(act, kact)			\
-  (act)->sa_restorer = (kact)->sa_restorer
-
-#endif
Andreas Schwab Dec. 12, 2018, 10:41 a.m. | #3
On Dez 12 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> The m68k kernel does define SA_RESTORER on arch/m68k/include/asm/signal.h

> (__ARCH_HAS_SA_RESTORER),


These are not the same.  The latter only tells <linux/signal_types.h> to
add the sa_restorer member, independent of SA_RESTORER (which m68k
doesn't define).

> but the only difference it makes for m68k is at flush_signal_handlers

> where kernel sets the sa_restorer to NULL. So it indeed should be safe

> to just use default kernel_sigaction.h (I don't see any signal test

> issues). Updated patch below.


This fails to add the sa_restorer member, causing the signal mask to be
mishandled.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Adhemerval Zanella Dec. 12, 2018, 6:52 p.m. | #4
On 12/12/2018 08:41, Andreas Schwab wrote:
> On Dez 12 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> 

>> The m68k kernel does define SA_RESTORER on arch/m68k/include/asm/signal.h

>> (__ARCH_HAS_SA_RESTORER),

> 

> These are not the same.  The latter only tells <linux/signal_types.h> to

> add the sa_restorer member, independent of SA_RESTORER (which m68k

> doesn't define).

> 

>> but the only difference it makes for m68k is at flush_signal_handlers

>> where kernel sets the sa_restorer to NULL. So it indeed should be safe

>> to just use default kernel_sigaction.h (I don't see any signal test

>> issues). Updated patch below.

> 

> This fails to add the sa_restorer member, causing the signal mask to be

> mishandled.

> 

> Andreas.

> 


Updated patch below.

--

	[BZ #23960]
	* sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):
	Define if SA_RESTORER is defined.
	(kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.
	(SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not
	already defined.
	* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,
	kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove
	definitions.
	(HAS_SA_RESTORER): Define.
	* sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,
	SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.
	(HAS_SA_RESTORER): Define.
	* sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic
	kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.
	* sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.
	* sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.

---

diff --git a/sysdeps/unix/sysv/linux/kernel_sigaction.h b/sysdeps/unix/sysv/linux/kernel_sigaction.h
index 2dbec08099..1c36146d46 100644
--- a/sysdeps/unix/sysv/linux/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/kernel_sigaction.h
@@ -1,19 +1,27 @@
 #ifndef _KERNEL_SIGACTION_H
 # define _KERNEL_SIGACTION_H
 
+#ifdef SA_RESTORER
+# define HAS_SA_RESTORER 1
+#endif
+
 /* This is the sigaction structure from the Linux 3.2 kernel.  */
 struct kernel_sigaction
 {
   __sighandler_t k_sa_handler;
   unsigned long sa_flags;
-#ifdef SA_RESTORER
+#ifdef HAS_SA_RESTORER
   void (*sa_restorer) (void);
 #endif
+  /* glibc sigset is larger than kernel expected one, however sigaction
+     passes the kernel expected size on rt_sigaction syscall.  */
   sigset_t sa_mask;
 };
 
-#ifndef SA_RESTORER
+#ifndef SET_SA_RESTORER
 # define SET_SA_RESTORER(kact, act)
+#endif
+#ifndef RESET_SA_RESTORER
 # define RESET_SA_RESTORER(act, kact)
 #endif
 
diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
index 54972feb13..464b351d6d 100644
--- a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
@@ -1,22 +1,4 @@
-#ifndef _KERNEL_SIGACTION_H
-# define _KERNEL_SIGACTION_H
-
-#include <signal.h>
-
-#define SA_RESTORER 0x04000000
-
-/* This is the sigaction structure from the Linux 3.2 kernel.  */
-struct kernel_sigaction
-{
-  __sighandler_t k_sa_handler;
-  sigset_t sa_mask;
-  unsigned long sa_flags;
-  void (*sa_restorer) (void);
-};
-
-#define SET_SA_RESTORER(kact, act)			\
-  (kact)->sa_restorer = (act)->sa_restorer
-#define RESET_SA_RESTORER(act, kact)			\
-  (act)->sa_restorer = (kact)->sa_restorer
-
-#endif
+/* m68k does not define SA_RESTORER, but does have sa_restorer member
+   on kernel sigaction struct.  */
+#define HAS_SA_RESTORER 1
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
diff --git a/sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h b/sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h
index 4ada322104..89f9bcedfd 100644
--- a/sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h
@@ -1,8 +1,9 @@
 /* NIOS2 uses the generic Linux UAPI but defines SA_RESTORER.  */
 #define SA_RESTORER 0x04000000
-#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
 
 #define SET_SA_RESTORER(kact, act)             \
   (kact)->sa_restorer = (act)->sa_restorer
 #define RESET_SA_RESTORER(act, kact)           \
   (act)->sa_restorer = (kact)->sa_restorer
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
diff --git a/sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h b/sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h
index aef3d5a3b3..bac03ee45d 100644
--- a/sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h
@@ -1,9 +1,10 @@
 /* powerpc kernel sigaction is similar to generic Linux UAPI one,
    but the architecture also defines SA_RESTORER.  */
 #define SA_RESTORER 0x04000000
-#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
 
 #define SET_SA_RESTORER(kact, act)             \
   (kact)->sa_restorer = (act)->sa_restorer
 #define RESET_SA_RESTORER(act, kact)           \
   (act)->sa_restorer = (kact)->sa_restorer
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
diff --git a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
index 7a6a2c4f29..14f44c200b 100644
--- a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h
@@ -30,3 +30,5 @@ struct kernel_sigaction
   (kact)->sa_restorer = (act)->sa_restorer
 #define RESET_SA_RESTORER(act, kact)           \
   (act)->sa_restorer = (kact)->sa_restorer
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
diff --git a/sysdeps/unix/sysv/linux/sh/kernel_sigaction.h b/sysdeps/unix/sysv/linux/sh/kernel_sigaction.h
index 7ebcd08d62..c8dc77a02b 100644
--- a/sysdeps/unix/sysv/linux/sh/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/sh/kernel_sigaction.h
@@ -1,8 +1,9 @@
 /* SH uses the generic Linux UAPI but defines SA_RESTORER.  */
 #define SA_RESTORER 0x04000000
-#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
 
 #define SET_SA_RESTORER(kact, act)             \
   (kact)->sa_restorer = (act)->sa_restorer
 #define RESET_SA_RESTORER(act, kact)           \
   (act)->sa_restorer = (kact)->sa_restorer
+
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
diff --git a/sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h b/sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h
index bee7e9cd03..eb4a522453 100644
--- a/sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h
@@ -1,10 +1,5 @@
 /* SPARC 'struct __new_sigaction' is similar to generic Linux UAPI with
    a sa_restorer field, even though function is passed as an argument
    to rt_sigaction syscall.  */
-#define SA_RESTORER 0x04000000
+#define HAS_SA_RESTORER 1
 #include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
-
-#define SET_SA_RESTORER(kact, act)             \
-  (kact)->sa_restorer = NULL
-#define RESET_SA_RESTORER(act, kact)           \
-  (act)->sa_restorer = (kact)->sa_restorer
diff --git a/sysdeps/unix/sysv/linux/x86_64/sigaction.c b/sysdeps/unix/sysv/linux/x86_64/sigaction.c
index 4e6d9cc32e..9aa2c7f860 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sigaction.c
+++ b/sysdeps/unix/sysv/linux/x86_64/sigaction.c
@@ -18,7 +18,6 @@
 
 #include <signal.h>
 #define SA_RESTORER 0x04000000
-#include <kernel_sigaction.h>
 
 extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
 
@@ -29,6 +28,8 @@ extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
 #define RESET_SA_RESTORER(act, kact) 			\
   (act)->sa_restorer = (kact)->sa_restorer
 
+#include <kernel_sigaction.h>
+
 #include <sysdeps/unix/sysv/linux/sigaction.c>
 
 /* NOTE: Please think twice before making any changes to the bits of
Andreas Schwab Dec. 14, 2018, 5:18 p.m. | #5
On Dez 12 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> 	[BZ #23960]

> 	* sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):

> 	Define if SA_RESTORER is defined.

> 	(kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.

> 	(SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not

> 	already defined.

> 	* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,

> 	kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove

> 	definitions.

> 	(HAS_SA_RESTORER): Define.

> 	* sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,

> 	SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.

> 	(HAS_SA_RESTORER): Define.

> 	* sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic

> 	kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.

> 	* sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.

> 	* sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.

> 	* sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.


Ok.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Aurelien Jarno Dec. 16, 2018, 8:06 p.m. | #6
Hi,

On 2018-12-12 16:52, Adhemerval Zanella wrote:
> 

> 

> On 12/12/2018 08:41, Andreas Schwab wrote:

> > On Dez 12 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> > 

> >> The m68k kernel does define SA_RESTORER on arch/m68k/include/asm/signal.h

> >> (__ARCH_HAS_SA_RESTORER),

> > 

> > These are not the same.  The latter only tells <linux/signal_types.h> to

> > add the sa_restorer member, independent of SA_RESTORER (which m68k

> > doesn't define).

> > 

> >> but the only difference it makes for m68k is at flush_signal_handlers

> >> where kernel sets the sa_restorer to NULL. So it indeed should be safe

> >> to just use default kernel_sigaction.h (I don't see any signal test

> >> issues). Updated patch below.

> > 

> > This fails to add the sa_restorer member, causing the signal mask to be

> > mishandled.

> > 

> > Andreas.

> > 

> 

> Updated patch below.

> 

> --

> 

> 	[BZ #23960]

> 	* sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):

> 	Define if SA_RESTORER is defined.

> 	(kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.

> 	(SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not

> 	already defined.

> 	* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,

> 	kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove

> 	definitions.

> 	(HAS_SA_RESTORER): Define.

> 	* sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,

> 	SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.

> 	(HAS_SA_RESTORER): Define.

> 	* sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic

> 	kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.

> 	* sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.

> 	* sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.

> 	* sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.


[ snip ]

> diff --git a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h

> index 7a6a2c4f29..14f44c200b 100644

> --- a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h

> +++ b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h

> @@ -30,3 +30,5 @@ struct kernel_sigaction

>    (kact)->sa_restorer = (act)->sa_restorer

>  #define RESET_SA_RESTORER(act, kact)           \

>    (act)->sa_restorer = (kact)->sa_restorer

> +

> +#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>


This is not correct, you should also remove struct kernel_sigaction from
this file, like it was done in the initial version of the patchset
(patch 5).

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net
Adhemerval Zanella Dec. 17, 2018, 11:08 a.m. | #7
On 16/12/2018 18:06, Aurelien Jarno wrote:
> Hi,

> 

> On 2018-12-12 16:52, Adhemerval Zanella wrote:

>>

>>

>> On 12/12/2018 08:41, Andreas Schwab wrote:

>>> On Dez 12 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

>>>

>>>> The m68k kernel does define SA_RESTORER on arch/m68k/include/asm/signal.h

>>>> (__ARCH_HAS_SA_RESTORER),

>>>

>>> These are not the same.  The latter only tells <linux/signal_types.h> to

>>> add the sa_restorer member, independent of SA_RESTORER (which m68k

>>> doesn't define).

>>>

>>>> but the only difference it makes for m68k is at flush_signal_handlers

>>>> where kernel sets the sa_restorer to NULL. So it indeed should be safe

>>>> to just use default kernel_sigaction.h (I don't see any signal test

>>>> issues). Updated patch below.

>>>

>>> This fails to add the sa_restorer member, causing the signal mask to be

>>> mishandled.

>>>

>>> Andreas.

>>>

>>

>> Updated patch below.

>>

>> --

>>

>> 	[BZ #23960]

>> 	* sysdeps/unix/sysv/linux/kernel_sigaction.h (HAS_SA_RESTORER):

>> 	Define if SA_RESTORER is defined.

>> 	(kernel_sigaction): Define sa_restorer if HAS_SA_RESTORER is defined.

>> 	(SET_SA_RESTORER, RESET_SA_RESTORER): Define iff the macro are not

>> 	already defined.

>> 	* sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h (SA_RESTORER,

>> 	kernel_sigaction, SET_SA_RESTORER, RESET_SA_RESTORER): Remove

>> 	definitions.

>> 	(HAS_SA_RESTORER): Define.

>> 	* sysdeps/unix/sysv/linux/sparc/kernel_sigaction.h (SA_RESTORER,

>> 	SET_SA_RESTORER, RESET_SA_RESTORER): Remove definition.

>> 	(HAS_SA_RESTORER): Define.

>> 	* sysdeps/unix/sysv/linux/nios2/kernel_sigaction.h: Include generic

>> 	kernel_sigaction after define SET_SA_RESTORER and RESET_SA_RESTORER.

>> 	* sysdeps/unix/sysv/linux/powerpc/kernel_sigaction.h: Likewise.

>> 	* sysdeps/unix/sysv/linux/s390/kernel_sigaction.h: Likewise.

>> 	* sysdeps/unix/sysv/linux/x86_64/sigaction.c: Likewise.

> 

> [ snip ]

> 

>> diff --git a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h

>> index 7a6a2c4f29..14f44c200b 100644

>> --- a/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h

>> +++ b/sysdeps/unix/sysv/linux/s390/kernel_sigaction.h

>> @@ -30,3 +30,5 @@ struct kernel_sigaction

>>    (kact)->sa_restorer = (act)->sa_restorer

>>  #define RESET_SA_RESTORER(act, kact)           \

>>    (act)->sa_restorer = (kact)->sa_restorer

>> +

>> +#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>

> 

> This is not correct, you should also remove struct kernel_sigaction from

> this file, like it was done in the initial version of the patchset

> (patch 5).

> 

> Aurelien

> 


Thanks for catching it, this is an mistake with my local rebase. I remove
this change for s390 kernel_sigaction.h for this patch.

Patch

diff --git a/sysdeps/unix/sysv/linux/kernel_sigaction.h b/sysdeps/unix/sysv/linux/kernel_sigaction.h
index 2dbec08099..b7359054b2 100644
--- a/sysdeps/unix/sysv/linux/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/kernel_sigaction.h
@@ -9,6 +9,8 @@  struct kernel_sigaction
 #ifdef SA_RESTORER
   void (*sa_restorer) (void);
 #endif
+  /* glibc sigset is larger than kernel expected one, however sigaction
+     passes the kernel expected size on rt_sigaction syscall.  */
   sigset_t sa_mask;
 };
 
diff --git a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
index 54972feb13..eef4bb9b65 100644
--- a/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
+++ b/sysdeps/unix/sysv/linux/m68k/kernel_sigaction.h
@@ -1,22 +1,8 @@ 
-#ifndef _KERNEL_SIGACTION_H
-# define _KERNEL_SIGACTION_H
-
-#include <signal.h>
-
+/* m68k uses the generic Linux UAPI but defines SA_RESTORER.  */
 #define SA_RESTORER 0x04000000
-
-/* This is the sigaction structure from the Linux 3.2 kernel.  */
-struct kernel_sigaction
-{
-  __sighandler_t k_sa_handler;
-  sigset_t sa_mask;
-  unsigned long sa_flags;
-  void (*sa_restorer) (void);
-};
+#include <sysdeps/unix/sysv/linux/kernel_sigaction.h>
 
 #define SET_SA_RESTORER(kact, act)			\
   (kact)->sa_restorer = (act)->sa_restorer
 #define RESET_SA_RESTORER(act, kact)			\
   (act)->sa_restorer = (kact)->sa_restorer
-
-#endif