lib/39465: threads stack is not aligned properly for gcc on i386

[anthony.mallet%useless-ficus.net@localhost]

>Number:         39465
>Category:       lib
>Synopsis:       threads stack is not aligned properly for gcc on i386
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Sep 05 20:30:00 +0000 2008
>Originator:     anthony.mallet%useless-ficus.net@localhost
>Release:        NetBSD 4.99.72
>Organization:
        
>Environment:
        
        
System: NetBSD ficus 4.99.72 NetBSD 4.99.72 (FICUS) #8: Sat Aug 30 13:19:16 
CEST 2008 troot@ficus:/usr/obj/sys/arch/i386/compile/FICUS i386
Architecture: i386
Machine: i386
>Description:
gcc expects the stack of functions to be setup so that the first argument of a
function is aligned on 16 bytes (on i386). It takes care of aligning main()'s
stack on a 16 bytes boundary and then generates code that relies on this
assumption.

The current code in makecontext(3) doesn't take care of this. This has the only
(but annoying) consequence that the __attribute__((aligned(xx))) declaration
does not work in functions called from a thread. In particular, SSE variables
on the stack cannot be used. 

The i386 ABI doesn't require that stacks are aligned on more than 4 bytes, so
strictly speaking the current makecontext(3) function is correct. Also, I
didn't check if code compiled with pcc works (I don't know if pcc supports
aligning variables on a particular boundary). However, it would be great if
code compiled with gcc would honor the aligned() attribute (perhaps only
within an #ifdef __GNUC__ ?)

Below is a fix that fixes the problem for me.
>How-To-Repeat:
Here is a small test program that exhibit the problem

#include <stdio.h>
#include <inttypes.h>
#include <stdbool.h>
#include <pthread.h>
#include <strings.h>

static void *
test16(void *data)
{
  char test __attribute__((aligned(16)));
  uintptr_t v = (uintptr_t)&test;
  size_t align;

  align = ffs((int)v);

  printf("aligned(16): %s: address: %p alignement: %d (2^%zu)\n",
         align >= 5 ? "PASS" : "FAIL", &test, 1<<(align-1), (align-1));
  return NULL;
}

int
main()
{
  pthread_t tg;

  printf("Testing on main's stack:\n");
  test16(NULL);

  printf("Testing on thread's stack:\n");

  pthread_create(&tg, NULL, test16, NULL);
  pthread_join(tg, NULL);

  return 0;
}

>Fix:
Index: makecontext.c
===================================================================
RCS file: /cvsroot/src/lib/libc/arch/i386/gen/makecontext.çv
retrieving revision 1.4
diff -u -r1.4 makecontext.c
--- makecontext.c       28 Apr 2008 20:22:56 -0000      1.4
+++ makecontext.c       5 Sep 2008 20:11:07 -0000
@@ -70,7 +70,9 @@
        /* Align on word boundary. */
        /* LINTED uintptr_t is safe */
        sp  = (unsigned int *)((uintptr_t)sp & ~0x3);
-       sp -= argc + 1;                 /* Make room for ret and args. */
+       sp -= argc;                     /* Make room for ret and args. */
+       sp  = (unsigned int *)((uintptr_t)sp & ~0xf);
+       sp --;                  /* Make room for ret and args. */
        /* LINTED __greg_t is safe */
        gr[_REG_UESP] = (__greg_t)sp;
        gr[_REG_EBP] = (__greg_t)0;     /* Wipe out frame pointer. */

>Unformatted: