Fixes pthread exit with -O2
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 19 Apr 2010 00:56:28 +0000 (17:56 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:43 +0000 (17:35 -0700)
-O2 was breaking the delicate balance in "set_stack_pointer() and then
call a function."  Setting __pthread_exit to noreturn keeps the compiler
from trying to use the stack to save info.

Saving space at the top of the stack needs to happen in an
arch-dependent manner in set_stack_pointer().  If you have page faults
around 0x20009000, it's because you are walking off the top of a
transition stack, and it's because you don't have enough room.

user/include/i686/arch.h
user/parlib/pthread.c

index 71a88e4..6ba010c 100644 (file)
@@ -10,6 +10,9 @@
 
 #define ARCH_CL_SIZE 64
 
+/* Make sure you subtract off/save enough space at the top of the stack for
+ * whatever you compiler might want to use when calling a noreturn function or
+ * to handle a HW spill or whatever. */
 static __inline void __attribute__((always_inline))
 set_stack_pointer(void* sp)
 {
index 1e49467..71179fe 100644 (file)
@@ -68,7 +68,7 @@ void _pthread_init()
        vcore_request(1);
 }
 
-void vcore_entry()
+void __attribute__((noreturn)) vcore_entry()
 {
        uint32_t vcoreid = vcore_id();
 
@@ -399,7 +399,7 @@ int pthread_equal(pthread_t t1, pthread_t t2)
 /* Need to have this as a separate, non-inlined function since we clobber the
  * stack pointer before calling it, and don't want the compiler to play games
  * with my hart. */
-static void __attribute__((noinline)) internal_function
+static void __attribute__((noinline, noreturn)) 
 __pthread_exit(struct pthread_tcb *t)
 {
        __pthread_free_tls(t);
@@ -446,11 +446,13 @@ void pthread_exit(void* ret)
        set_tls_desc(vcore_thread_control_blocks[vcoreid], vcoreid);
        assert(current_thread == t);    
        /* After this, make sure you don't use local variables.  Also, make sure the
-        * compiler doesn't use them without telling you (TODO).  We take some space
-        * off the top of the stack since the compiler is going to assume it has a
-        * stack frame setup when it pushes (actually moves) the current_thread onto
-        * the stack before calling __pthread_cleanup(). */
-       set_stack_pointer((void*)vcpd->transition_stack - 32);
+        * compiler doesn't use them without telling you (TODO).
+        *
+        * In each arch's set_stack_pointer, make sure you subtract off as much room
+        * as you need to any local vars that might be pushed before calling the
+        * next function, or for whatever other reason the compiler/hardware might
+        * walk up the stack a bit when calling a noreturn function. */
+       set_stack_pointer((void*)vcpd->transition_stack);
        /* Finish exiting in another function.  Ugh. */
        __pthread_exit(current_thread);
 }