2003-07-14 12:37:46

by Jakub Jelinek

[permalink] [raw]
Subject: sizeof (siginfo_t) problem

Hi!

When siginfo_t was added, the intent obviously was that its size
be 128 bytes on all arches.
This is true on all 32-bit arches and what glibc has in its siginfo_t
definition on all arches:
# if __WORDSIZE == 64
# define __SI_PAD_SIZE ((__SI_MAX_SIZE / sizeof (int)) - 4)
# else
# define __SI_PAD_SIZE ((__SI_MAX_SIZE / sizeof (int)) - 3)
# endif

typedef struct siginfo
{
int si_signo; /* Signal number. */
int si_errno; /* If non-zero, an errno value associated with
this signal, as defined in <errno.h>. */
int si_code; /* Signal code. */

union
{
int _pad[__SI_PAD_SIZE];
...
struct
{
void *si_addr; /* Faulting insn/memory ref. */
} _sigfault;
...
} _sifields;
} siginfo_t;

The kernel unfortunately does this right on sparc64 and alpha from 64-bit
arches only; ia64, s390x, ppc64 etc. got it wrong.

This is a bad problem, since e.g. sigwaitinfo(3) is supported ABI,
and apps - as they are compiled against glibc headers, not kernel ones -
will thus reserve on the stack 8 bytes less than the kernel fills
(this is hidden in copy_siginfo_to_user, so usually only if si_code >= 0
all 136 bytes will be copied).
Note that glibc had such siginfo_t definitions from the beggining,
ie. it is not something changed recently. E.g. on s390x,
such definition was already in -r1.1 checkin in March 2001, x86_64/ia64
never had its own siginfo.h but used a generic one which had the
above __SI_PAD_SIZE definition since early 2000 (x86_64 was added in 2001,
ia64 about 5 months later than that change).

I have noticed this on s390x in a different place:
MD_FALLBACK_FRAME_STATE_FOR in gcc, in order to unwind through
signal frames, needs to locate ucontext, and looking at
stack pointer + 8 + sizeof(siginfo_t) (which works on s390 32-bit)
did not work at all, everything was shifted by 8 bytes.
This though means that the kernel siginfo_t change cannot be done
just in asm-*/siginfo.h headers - at least places where siginfo_t
is present within some structures ever visible to userland a dummy
8 byte pad needs to be inserted.

Jakub


2003-07-14 13:44:01

by Jamie Lokier

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

Jakub Jelinek wrote:
> When siginfo_t was added, the intent obviously was that its size
> be 128 bytes on all arches.
>
> The kernel unfortunately does this right on sparc64 and alpha from 64-bit
> arches only; ia64, s390x, ppc64 etc. got it wrong.

That's not the only siginfo_t problem:

- Take a look at the placement of the _uid32 field on m68k.
It varies from sub-structure to sub-structure - yet it is
always written to the same offset by the kernel. Borken!

Other cleanups:

- SI_* codes on S390 are identical to the generic ones.
They can be removed from the S390 file.

- Comment in MIPS siginfo about IRIX compatibility is no
longer true; the layout was changed when uid32 added.

- __ARCH_SI_UID_T can be removed. Only Sparc sets it,
and to the same type as uid_t (unsigned int).

-- Jamie

2003-07-14 14:47:55

by Andreas Schwab

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

Jamie Lokier <[email protected]> writes:

|> Jakub Jelinek wrote:
|> > When siginfo_t was added, the intent obviously was that its size
|> > be 128 bytes on all arches.
|> >
|> > The kernel unfortunately does this right on sparc64 and alpha from 64-bit
|> > arches only; ia64, s390x, ppc64 etc. got it wrong.
|>
|> That's not the only siginfo_t problem:
|>
|> - Take a look at the placement of the _uid32 field on m68k.
|> It varies from sub-structure to sub-structure - yet it is
|> always written to the same offset by the kernel. Borken!

It should probably use the one from asm-generic as well. I could not
find anything that actually uses that field.

Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 N?rnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2003-07-14 15:33:48

by David Mosberger

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

>>>>> On Mon, 14 Jul 2003 08:40:00 -0400, Jakub Jelinek <[email protected]> said:

Jakub> # if __WORDSIZE == 64
Jakub> # define __SI_PAD_SIZE ((__SI_MAX_SIZE / sizeof (int)) - 4)
Jakub> # else
Jakub> # define __SI_PAD_SIZE ((__SI_MAX_SIZE / sizeof (int)) - 3)
Jakub> # endif

Jakub> typedef struct siginfo
Jakub> {
Jakub> int si_signo; /* Signal number. */
Jakub> int si_errno; /* If non-zero, an errno value associated with
Jakub> this signal, as defined in <errno.h>. */
Jakub> int si_code; /* Signal code. */

Jakub> union
Jakub> {
Jakub> int _pad[__SI_PAD_SIZE];
Jakub> ...
Jakub> struct
Jakub> {
Jakub> void *si_addr; /* Faulting insn/memory ref. */
Jakub> } _sigfault;
Jakub> ...
Jakub> } _sifields;
Jakub> } siginfo_t;

Jakub> The kernel unfortunately does this right on sparc64 and alpha
Jakub> from 64-bit arches only; ia64, s390x, ppc64 etc. got it
Jakub> wrong.

The ia64 kernel defines in asm-ia64/siginfo.h:

#define SI_PAD_SIZE ((SI_MAX_SIZE/sizeof(int)) - 4)

typedef struct siginfo {
int si_signo;
int si_errno;
int si_code;
int __pad0;

What's wrong with that?

--david

2003-07-14 15:42:42

by Jakub Jelinek

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Mon, Jul 14, 2003 at 08:48:00AM -0700, David Mosberger wrote:
> Jakub> The kernel unfortunately does this right on sparc64 and alpha
> Jakub> from 64-bit arches only; ia64, s390x, ppc64 etc. got it
> Jakub> wrong.
>
> The ia64 kernel defines in asm-ia64/siginfo.h:
>
> #define SI_PAD_SIZE ((SI_MAX_SIZE/sizeof(int)) - 4)
>
> typedef struct siginfo {
> int si_signo;
> int si_errno;
> int si_code;
> int __pad0;
>
> What's wrong with that?

Oops, sorry, you're right, dunno where I was looking.
So, the bug seems to exist on s390x, amd64, maybe parisc64 if such thing
exists.

Jakub

2003-07-14 16:43:44

by Stephen Rothwell

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Mon, 14 Jul 2003 08:40:00 -0400 Jakub Jelinek <[email protected]> wrote:
>
> The kernel unfortunately does this right on sparc64 and alpha from 64-bit
> arches only; ia64, s390x, ppc64 etc. got it wrong.

David Mosberger is correct that ia64 is OK (because it basically uses its
own siginfo.h as does mips64). The following is an update of a patch that
was lost a while ago (when __ARCH_SI_PREAMBLE_SIZE was invented).

I am not sure if the s390 fix is correct (since s390x has been merged) or
if x86_64 needs this (as I cannot remember the alignment needs of the
x86_64 compiler - though I suspect it is needed).

I have no idea if parisc is correct on 64 bit platforms (probably not).

This has been neither compiled or tested, but should be close.
--
Cheers,
Stephen Rothwell [email protected]

diff -ruN 2.6.0-test1/include/asm-generic/siginfo.h 2.6.0-test1-sfr.1/include/asm-generic/siginfo.h
--- 2.6.0-test1/include/asm-generic/siginfo.h 2003-04-20 14:12:49.000000000 +1000
+++ 2.6.0-test1-sfr.1/include/asm-generic/siginfo.h 2003-07-15 02:41:47.000000000 +1000
@@ -142,7 +142,6 @@
#define SI_FROMUSER(siptr) ((siptr)->si_code <= 0)
#define SI_FROMKERNEL(siptr) ((siptr)->si_code > 0)

-#ifndef HAVE_ARCH_SI_CODES
/*
* SIGILL si_codes
*/
@@ -213,8 +212,6 @@
#define POLL_HUP (__SI_POLL|6) /* device disconnected */
#define NSIGPOLL 6

-#endif
-
/*
* sigevent definitions
*
diff -ruN 2.6.0-test1/include/asm-ppc64/siginfo.h 2.6.0-test1-sfr.1/include/asm-ppc64/siginfo.h
--- 2.6.0-test1/include/asm-ppc64/siginfo.h 2002-11-05 17:00:34.000000000 +1100
+++ 2.6.0-test1-sfr.1/include/asm-ppc64/siginfo.h 2003-07-15 02:28:22.000000000 +1000
@@ -8,8 +8,8 @@
* 2 of the License, or (at your option) any later version.
*/

-#define SI_PAD_SIZE ((SI_MAX_SIZE/sizeof(int)) - 4)
-#define SI_PAD_SIZE32 ((SI_MAX_SIZE/sizeof(int)) - 3)
+#define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
+#define SI_PAD_SIZE32 ((SI_MAX_SIZE/sizeof(int)) - 3)

#include <asm-generic/siginfo.h>

diff -ruN 2.6.0-test1/include/asm-s390/siginfo.h 2.6.0-test1-sfr.1/include/asm-s390/siginfo.h
--- 2.6.0-test1/include/asm-s390/siginfo.h 2002-11-05 16:58:19.000000000 +1100
+++ 2.6.0-test1-sfr.1/include/asm-s390/siginfo.h 2003-07-15 02:34:55.000000000 +1000
@@ -9,78 +9,12 @@
#ifndef _S390_SIGINFO_H
#define _S390_SIGINFO_H

-#define HAVE_ARCH_SI_CODES
+#include <linux/config.h>

-#include <asm-generic/siginfo.h>
-
-/*
- * SIGILL si_codes
- */
-#define ILL_ILLOPC (__SI_FAULT|1) /* illegal opcode */
-#define ILL_ILLOPN (__SI_FAULT|2) /* illegal operand */
-#define ILL_ILLADR (__SI_FAULT|3) /* illegal addressing mode */
-#define ILL_ILLTRP (__SI_FAULT|4) /* illegal trap */
-#define ILL_PRVOPC (__SI_FAULT|5) /* privileged opcode */
-#define ILL_PRVREG (__SI_FAULT|6) /* privileged register */
-#define ILL_COPROC (__SI_FAULT|7) /* coprocessor error */
-#define ILL_BADSTK (__SI_FAULT|8) /* internal stack error */
-#define NSIGILL 8
-
-/*
- * SIGFPE si_codes
- */
-#define FPE_INTDIV (__SI_FAULT|1) /* integer divide by zero */
-#define FPE_INTOVF (__SI_FAULT|2) /* integer overflow */
-#define FPE_FLTDIV (__SI_FAULT|3) /* floating point divide by zero */
-#define FPE_FLTOVF (__SI_FAULT|4) /* floating point overflow */
-#define FPE_FLTUND (__SI_FAULT|5) /* floating point underflow */
-#define FPE_FLTRES (__SI_FAULT|6) /* floating point inexact result */
-#define FPE_FLTINV (__SI_FAULT|7) /* floating point invalid operation */
-#define FPE_FLTSUB (__SI_FAULT|8) /* subscript out of range */
-#define NSIGFPE 8
-
-/*
- * SIGSEGV si_codes
- */
-#define SEGV_MAPERR (__SI_FAULT|1) /* address not mapped to object */
-#define SEGV_ACCERR (__SI_FAULT|2) /* invalid permissions for mapped object */
-#define NSIGSEGV 2
-
-/*
- * SIGBUS si_codes
- */
-#define BUS_ADRALN (__SI_FAULT|1) /* invalid address alignment */
-#define BUS_ADRERR (__SI_FAULT|2) /* non-existant physical address */
-#define BUS_OBJERR (__SI_FAULT|3) /* object specific hardware error */
-#define NSIGBUS 3
-
-/*
- * SIGTRAP si_codes
- */
-#define TRAP_BRKPT (__SI_FAULT|1) /* process breakpoint */
-#define TRAP_TRACE (__SI_FAULT|2) /* process trace trap */
-#define NSIGTRAP 2
-
-/*
- * SIGCHLD si_codes
- */
-#define CLD_EXITED (__SI_CHLD|1) /* child has exited */
-#define CLD_KILLED (__SI_CHLD|2) /* child was killed */
-#define CLD_DUMPED (__SI_CHLD|3) /* child terminated abnormally */
-#define CLD_TRAPPED (__SI_CHLD|4) /* traced child has trapped */
-#define CLD_STOPPED (__SI_CHLD|5) /* child has stopped */
-#define CLD_CONTINUED (__SI_CHLD|6) /* stopped child has continued */
-#define NSIGCHLD 6
+#ifdef CONFIG_ARCH_S390X
+#define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
+#endif

-/*
- * SIGPOLL si_codes
- */
-#define POLL_IN (__SI_POLL|1) /* data input available */
-#define POLL_OUT (__SI_POLL|2) /* output buffers available */
-#define POLL_MSG (__SI_POLL|3) /* input message available */
-#define POLL_ERR (__SI_POLL|4) /* i/o error */
-#define POLL_PRI (__SI_POLL|5) /* high priority input available */
-#define POLL_HUP (__SI_POLL|6) /* device disconnected */
-#define NSIGPOLL 6
+#include <asm-generic/siginfo.h>

#endif
diff -ruN 2.6.0-test1/include/asm-x86_64/siginfo.h 2.6.0-test1-sfr.1/include/asm-x86_64/siginfo.h
--- 2.6.0-test1/include/asm-x86_64/siginfo.h 2002-11-05 16:58:09.000000000 +1100
+++ 2.6.0-test1-sfr.1/include/asm-x86_64/siginfo.h 2003-07-15 02:38:49.000000000 +1000
@@ -1,6 +1,9 @@
#ifndef _X8664_SIGINFO_H
#define _X8664_SIGINFO_H

+#define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
+#define SIGEV_PAD_SIZE ((SIGEV_MAX_SIZE/sizeof(int)) - 4)
+
#include <asm-generic/siginfo.h>

#endif

2003-07-14 16:53:07

by Jakub Jelinek

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Tue, Jul 15, 2003 at 02:52:52AM +1000, Stephen Rothwell wrote:
> diff -ruN 2.6.0-test1/include/asm-s390/siginfo.h 2.6.0-test1-sfr.1/include/asm-s390/siginfo.h
> --- 2.6.0-test1/include/asm-s390/siginfo.h 2002-11-05 16:58:19.000000000 +1100
> +++ 2.6.0-test1-sfr.1/include/asm-s390/siginfo.h 2003-07-15 02:34:55.000000000 +1000
> @@ -9,78 +9,12 @@
> #ifndef _S390_SIGINFO_H
> #define _S390_SIGINFO_H
>
> -#define HAVE_ARCH_SI_CODES
> +#include <linux/config.h>
...
> +#ifdef CONFIG_ARCH_S390X
> +#define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
> +#endif

This is not correct for the merged header.
It needs to be:
#ifdef __s390x__
#define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
#endif

Furthermore, there needs to be a pad inserted fo arch/s390x/kernel/signal.c
(rt_sigframe right after info member) to keep binary compatibility.

Jakub

2003-07-14 16:59:11

by Stephen Rothwell

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Mon, 14 Jul 2003 13:00:24 -0400 Jakub Jelinek <[email protected]> wrote:
>
> This is not correct for the merged header.
> It needs to be:
> #ifdef __s390x__
> #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
> #endif

OK, I can see that (although is __s390x__ defined when building a
32 (31?) bit kernel on a 64bit s390?).

> Furthermore, there needs to be a pad inserted fo arch/s390x/kernel/signal.c
^^^^^
s390? (there is no arch/s390x in 2.6.0-test1)

> (rt_sigframe right after info member) to keep binary compatibility.

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

2003-07-14 17:00:58

by Jakub Jelinek

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Tue, Jul 15, 2003 at 03:11:23AM +1000, Stephen Rothwell wrote:
> > This is not correct for the merged header.
> > It needs to be:
> > #ifdef __s390x__
> > #define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
> > #endif
>
> OK, I can see that (although is __s390x__ defined when building a
> 32 (31?) bit kernel on a 64bit s390?).

__s390x__ is defined when doing 64-bit compile targetted to s390.
Ie. gcc -m64 defines it, gcc -m31 does not.

> > Furthermore, there needs to be a pad inserted fo arch/s390x/kernel/signal.c
> ^^^^^
> s390? (there is no arch/s390x in 2.6.0-test1)

Then that pad needs to be #ifdef __s390x__ as well.
>
> > (rt_sigframe right after info member) to keep binary compatibility.

Jakub

2003-07-14 17:18:52

by Stephen Rothwell

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Mon, 14 Jul 2003 13:14:01 -0400 Jakub Jelinek <[email protected]> wrote:
>
> __s390x__ is defined when doing 64-bit compile targetted to s390.
> Ie. gcc -m64 defines it, gcc -m31 does not.

That makes sense (since I now see that CONFIG_ARCH_S390X => -m64).

> Then that pad needs to be #ifdef __s390x__ as well.

But why pad at all since we have now increased the size of the siginfo
structure in the 64bit case (maybe I am being thick as it is 3:25am here
:-)).

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

2003-07-14 17:35:08

by Jakub Jelinek

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Tue, Jul 15, 2003 at 03:25:52AM +1000, Stephen Rothwell wrote:
> > Then that pad needs to be #ifdef __s390x__ as well.
>
> But why pad at all since we have now increased the size of the siginfo
> structure in the 64bit case (maybe I am being thick as it is 3:25am here
> :-)).

Decreased, from 136 bytes when __ARCH_SI_PREAMBLE_SIZE is (3 * sizeof(int))
to 128 bytes with (4 * sizeof(int)).
The pad in rt_sigframe is certainly open for discussion. GCC does on s390x:
struct ucontext_ \
{ \
unsigned long uc_flags; \
struct ucontext_ *uc_link; \
unsigned long uc_stack[3]; \
sigregs_ uc_mcontext; \
} *uc_ = (CONTEXT)->cfa + 8 + 128; \
\
regs_ = &uc_->uc_mcontext; \
(128 stands for sizeof(siginfo_t)).
This means it does not work on any kernels so far, if we don't add a pad
to the kernel and just fix __ARCH_SI_PREAMBLE_SIZE on s390x, then GCC
will suddenly work with all newer kernels but will never work with older
kernels.
If pad is added to the kernel at the same time as __ARCH_SI_PREAMBLE_SIZE
is increased, it would need to be added to GCC as well, so older GCCs
would not work on any kernels while patched/newer GCCs would work on all
kernels.

Jakub

2003-07-14 18:17:10

by Ulrich Weigand

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

Jakub Jelinek wrote:

>I have noticed this on s390x in a different place:
>MD_FALLBACK_FRAME_STATE_FOR in gcc, in order to unwind through
>signal frames, needs to locate ucontext, and looking at
>stack pointer + 8 + sizeof(siginfo_t) (which works on s390 32-bit)
>did not work at all, everything was shifted by 8 bytes.

The MD_FALLBACK_FRAME_STATE_FOR macro does not use sizeof(siginfo_t)
at all, but hardcodes 128. (This has always been the case, and was
done to avoid the need to include signal.h.) This is still broken
with the current kernel behaviour, though.

It is strange that I didn't notice the problem earlier; apparently
most of the places where unwinding through signal frames is required
use non-RT frames ...

>This though means that the kernel siginfo_t change cannot be done
>just in asm-*/siginfo.h headers - at least places where siginfo_t
>is present within some structures ever visible to userland a dummy
>8 byte pad needs to be inserted.

As userspace expects a size of 128 bytes, and with the change
the size now *is* 128 bytes, why would a pad be required?


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: [email protected]

2003-07-14 18:21:24

by Jakub Jelinek

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Mon, Jul 14, 2003 at 08:31:34PM +0200, Ulrich Weigand wrote:
> The MD_FALLBACK_FRAME_STATE_FOR macro does not use sizeof(siginfo_t)
> at all, but hardcodes 128. (This has always been the case, and was
> done to avoid the need to include signal.h.) This is still broken
> with the current kernel behaviour, though.
>
> It is strange that I didn't notice the problem earlier; apparently
> most of the places where unwinding through signal frames is required
> use non-RT frames ...

But it is crucial for NPTL pthread_cancel.

> >This though means that the kernel siginfo_t change cannot be done
> >just in asm-*/siginfo.h headers - at least places where siginfo_t
> >is present within some structures ever visible to userland a dummy
> >8 byte pad needs to be inserted.
>
> As userspace expects a size of 128 bytes, and with the change
> the size now *is* 128 bytes, why would a pad be required?

As I tried to write, we either can have all GCCs
which will work properly only with new kernels (no pad added),
or we can have new GCCs working with all kernels (if pad is added).
Your choice...

Jakub

2003-07-14 18:37:48

by Ulrich Weigand

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem


Jakub Jelinek wrote:

>As I tried to write, we either can have all GCCs
>which will work properly only with new kernels (no pad added),
>or we can have new GCCs working with all kernels (if pad is added).
>Your choice...

I'll discuss this with Martin tomorrow, but right now I'd tend to
fixing the kernel, because this means you can fix an installed
system by upgrading only the kernel itself (and this upgrade
should not break anything). The alternative would be not only
to upgrade libgcc (and possibly glibc), but all programs statically
linked with it as well as all programs that otherwise have the
signal stack layout hardcoded ...


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: [email protected]

2003-07-15 11:46:11

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem


> This means it does not work on any kernels so far, if we don't add a pad
> to the kernel and just fix __ARCH_SI_PREAMBLE_SIZE on s390x, then GCC
> will suddenly work with all newer kernels but will never work with older
> kernels.

This is a kernel bug and I'm inclined to say that this has to be fixed in
the kernel and only there. If it didn't work at all so far nobody will
complain that it suddenly works with the kernel fix. It is an ABI change
but for an ABI that didn't work so far. I don't see a problem with the
simple fix to use __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int)) for s390x.

blue skies,
Martin


2003-07-20 20:29:16

by Anton Blanchard

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem


Hi Stephen,

> I am not sure if the s390 fix is correct (since s390x has been merged) or
> if x86_64 needs this (as I cannot remember the alignment needs of the
> x86_64 compiler - though I suspect it is needed).

I didnt follow this thread very carefully :) Is ppc64 broken?

Anton

2003-07-20 23:53:25

by Stephen Rothwell

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

Hi Anton,

On Sun, 20 Jul 2003 04:32:54 +1000 Anton Blanchard <[email protected]> wrote:
>
> Hi Stephen,
>
> > I am not sure if the s390 fix is correct (since s390x has been merged) or
> > if x86_64 needs this (as I cannot remember the alignment needs of the
> > x86_64 compiler - though I suspect it is needed).
>
> I didnt follow this thread very carefully :) Is ppc64 broken?

It is broken subtly (but noone would probably notice :-)). It is OK
on the structure size, but copy_siginfo will copy sizeof(int) bytes less
than necessary sometimes (particularly in the case of a SIGCHILD).

You need the following patch (against 2.6.0-test1-bk2 but should apply
to just about anything).
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

diff -ruN 2.6.0-test1-bk2/include/asm-ppc64/siginfo.h 2.6.0-test1-bk2-sfr.1/include/asm-ppc64/siginfo.h
--- 2.6.0-test1-bk2/include/asm-ppc64/siginfo.h 2002-11-05 17:00:34.000000000 +1100
+++ 2.6.0-test1-bk2-sfr.1/include/asm-ppc64/siginfo.h 2003-07-21 09:59:04.000000000 +1000
@@ -8,8 +8,8 @@
* 2 of the License, or (at your option) any later version.
*/

-#define SI_PAD_SIZE ((SI_MAX_SIZE/sizeof(int)) - 4)
-#define SI_PAD_SIZE32 ((SI_MAX_SIZE/sizeof(int)) - 3)
+#define __ARCH_SI_PREAMBLE_SIZE (4 * sizeof(int))
+#define SI_PAD_SIZE32 ((SI_MAX_SIZE/sizeof(int)) - 3)

#include <asm-generic/siginfo.h>

2003-07-22 13:27:20

by Russell King

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Mon, Jul 21, 2003 at 10:08:19AM +1000, Stephen Rothwell wrote:
> On Sun, 20 Jul 2003 04:32:54 +1000 Anton Blanchard <[email protected]> wrote:
> > > I am not sure if the s390 fix is correct (since s390x has been merged) or
> > > if x86_64 needs this (as I cannot remember the alignment needs of the
> > > x86_64 compiler - though I suspect it is needed).
> >
> > I didnt follow this thread very carefully :) Is ppc64 broken?
>
> It is broken subtly (but noone would probably notice :-)). It is OK
> on the structure size, but copy_siginfo will copy sizeof(int) bytes less
> than necessary sometimes (particularly in the case of a SIGCHILD).

Maybe it'd be a good idea to copy the architecture maintainers. I'm
certainly deleting a couple of thousand lkml mails at the moment, so
its pretty lucky that I just read Anton's message.

Is ARM broken?

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-07-22 13:46:31

by Stephen Rothwell

[permalink] [raw]
Subject: Re: sizeof (siginfo_t) problem

On Tue, 22 Jul 2003 14:42:18 +0100 Russell King <[email protected]> wrote:
>
> Maybe it'd be a good idea to copy the architecture maintainers. I'm
> certainly deleting a couple of thousand lkml mails at the moment, so
> its pretty lucky that I just read Anton's message.
>
> Is ARM broken?

None of the 32 bit architectures are broken. It was, I would have copied
you, the consolidation work has made me aware of this. :-)

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/