Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 08:21:42PM +0100, Paul Fleischer wrote: > Is the usage of STACKALIGN indeed incorrect in this situation, or am I > missing the big picture? I stumbled across this when revamping execve1 for posix_spawn recently. The intention seems to be to align the stack on a 8 byte boundary (where arm usualy only requires 4 byte alignment). I did not dig in the ARM ABI docs deep enough to see why this would be needed. However, the current implementation seems to be broken - the macro works on the stack pointer but not on a length variable, as you noted. Can anyone explain why arm would need 8 byte alignment? Martin
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 12:16:49PM -0800, Stephen M. Rumble wrote: > The ARM EABI requires 8-byte stack alignment at function entry. I don't know > if we're using the EABI, but this bug and its fix might indicate as much. I don't think we do yet, but we should ;-) Anyway, let's make a up-rounding (ALIGN-like) copy of STACKALIGN() and use that (the original macro is correclty used in signalcode and commpat). Martin
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, 24 Jan 2012 21:01:49 +0100 Martin Husemann wrote: > On Tue, Jan 24, 2012 at 08:21:42PM +0100, Paul Fleischer wrote: > > Is the usage of STACKALIGN indeed incorrect in this situation, or am I > > missing the big picture? > > I stumbled across this when revamping execve1 for posix_spawn recently. > > The intention seems to be to align the stack on a 8 byte boundary > (where arm usualy only requires 4 byte alignment). I did not dig in the > ARM ABI docs deep enough to see why this would be needed. > > However, the current implementation seems to be broken - the macro works > on the stack pointer but not on a length variable, as you noted. > > Can anyone explain why arm would need 8 byte alignment? Do some architectures (i.e. x86) have better performance if the stack is 16-bytes aligned? If so, perhaps that this could be MI, satisfying both 8-bytes (or 4-bytes) alignment, by aligning stacks at 16-bytes? Would this be considered wasteful? Of course, x86-64 MD code could also be used... There is also a related PR but which is for threads stack alignment: lib/39465 Thanks, -- Matt
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 03:30:37PM -0500, Matthew Mondor wrote: > Would this be considered wasteful? Of course, x86-64 MD code could > also be used... I would prefer the x86 MD code, using the same technique as the (fixed) arm code. Maybe it could even depend on actual CPU type (i.e. SSE2 availabiblity). Martin
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 03:30:37PM -0500, Matthew Mondor wrote: > There is also a related PR but which is for threads stack alignment: > lib/39465 That bug is wrong. NetBSD uses SYSV ABI and that mandates 4 Bytes alignment for the stack. GCC versions before ~4.5 or so are just completely broken in this regard. Joerg
Re: Possible incorrect usage of STACKALIGN in kern_exec
In article , Paul Fleischer wrote: >Hello there, > >I am currently working on a NetBSD port for the FriendlyARM MINI2440, >and have run into a situation where the arguments to user space >programs is garbled. >I've noticed it for the init-process and for the getty process. >Booting with rc_configured=NO in /etc/rc.conf, mounting procfs, and >issuing a cat /proc/1/cmdline will yields "init" followed by >"garbage". Always 0x5C 0xEF >0xFF 0xBF 0x02 > >Digging a bit around the kernel, I have noticed that line 796 in >kern_exec.c (execve1) has a special case for ARM only (as far as I can >tell): > >len = STACKALIGN(len); /* make the stack "safely" aligned */ > >STACKALIGN aligns is implemented in arch/arm/include/param.h: > > 96 /* ARM-specific macro to align a stack pointer (downwards). */ > 97 #define STACKALIGNBYTES (8 - 1) > 98 #define STACKALIGN(p) ((u_int)(p) &~ STACKALIGNBYTES) > >So it's basically truncates to proper alignment, rounding down to an >aligned number. Rounding down lengths seems like a dangerous thing to >do. > >Removing the usage of STACKALIGN and always using ALIGN, solves the >problem for the MINI2440-port. > >Looking a the history of kern_exec.c, the following code was commited >in 1.243 in March 2007: > >+#ifdef STACKLALIGN /* arm, etc. */ >+ len = STACKALIGN(len); /* make the stack "safely" aligned */ >+#else > len = ALIGN(len); /* make the stack "safely" aligned */ >+#endif > >Notice that the #ifdef checks for STACK*L*ALIGN, which is not defined >anywhere in the kernel. >In August 2011 the typo was fixed in r.1.323, meaning that the >STACKALIGN macro has not been used until 4 months ago. This indicates >to me that STACKALIGN is not really supposed to be used in this >situation (and neither OpenBSD nor FreeBSD have it, although much of >the surrounding code is similar). > >Is the usage of STACKALIGN indeed incorrect in this situation, or am I >missing the big picture? Yes it is. I will commit a fix shortly. christos
Re: Possible incorrect usage of STACKALIGN in kern_exec
On 2012-01-24, at 12:01 PM, Martin Husemann wrote: > Can anyone explain why arm would need 8 byte alignment? The ARM EABI requires 8-byte stack alignment at function entry. I don't know if we're using the EABI, but this bug and its fix might indicate as much. Steve
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, 24 Jan 2012 21:55:30 +0100 Joerg Sonnenberger wrote: > On Tue, Jan 24, 2012 at 03:30:37PM -0500, Matthew Mondor wrote: > > There is also a related PR but which is for threads stack alignment: > > lib/39465 > > That bug is wrong. NetBSD uses SYSV ABI and that mandates 4 Bytes > alignment for the stack. GCC versions before ~4.5 or so are just > completely broken in this regard. I know that some OSs do it for i386 to reduce the overhead of SSE instructions setup, but I wasn't aware that this could be problematic with the i386 SYSV ABI (since 16 is a multiple of 4). Having just looked at both i386 and x86-64 SYSV ABI docs now, I think that you're right for i386. It's in the x86-64 case that stack frames are 16-byte aligned, with arrays larger than 16 bytes also needing to be 16-byte aligned (possibly including the stack)... Thanks, -- Matt
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 04:53:53PM -0500, Matthew Mondor wrote: > It's in the x86-64 case that stack frames are 16-byte aligned, with > arrays larger than 16 bytes also needing to be 16-byte aligned > (possibly including the stack)... Depending on when you look, the stack frame is either 8 mod 16 or 0 mod 16. Arrays don't have any special alignment, but it is the responsibility of the compiler to add padding for on-stack arrays as needed to ensure the alignment of each component. Joerg
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 09:55:30PM +0100, Joerg Sonnenberger wrote: > > There is also a related PR but which is for threads stack alignment: > > lib/39465 > > That bug is wrong. NetBSD uses SYSV ABI and that mandates 4 Bytes > alignment for the stack. GCC versions before ~4.5 or so are just > completely broken in this regard. And yet there have been several reports that it still doesn't work with gcc 4.5... -- David A. Holland dholl...@netbsd.org
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 10:13:39PM +, David Holland wrote: > On Tue, Jan 24, 2012 at 09:55:30PM +0100, Joerg Sonnenberger wrote: > > > There is also a related PR but which is for threads stack alignment: > > > lib/39465 > > > > That bug is wrong. NetBSD uses SYSV ABI and that mandates 4 Bytes > > alignment for the stack. GCC versions before ~4.5 or so are just > > completely broken in this regard. > > And yet there have been several reports that it still doesn't work > with gcc 4.5... Of course. You don't expect GCC to default to correct behavior (-mstackrealign), do you? Joerg
Re: Possible incorrect usage of STACKALIGN in kern_exec
On Tue, Jan 24, 2012 at 11:30:54PM +0100, Joerg Sonnenberger wrote: > > > > There is also a related PR but which is for threads stack alignment: > > > > lib/39465 > > > > > > That bug is wrong. NetBSD uses SYSV ABI and that mandates 4 Bytes > > > alignment for the stack. GCC versions before ~4.5 or so are just > > > completely broken in this regard. > > > > And yet there have been several reports that it still doesn't work > > with gcc 4.5... > > Of course. You don't expect GCC to default to correct behavior > (-mstackrealign), do you? No, but if we need to change our gcc specs so this works in -6, perhaps we should do it this week... -- David A. Holland dholl...@netbsd.org
