---

yaml
---
r: 6870
b: refs/heads/master
c: 96d7f83
h: refs/heads/master
v: v3

Author: graydon

[refs]

@@ -1,2 +1,2 @@
 ---
-refs/heads/master: b957916e06ea70b70aef1e404b089292aad42d7b
+refs/heads/master: 96d7f83eb0c87453d57e5a71959d0d74a9e500ca

trunk/src/comp/middle/ty.rs

@@ -1158,7 +1158,8 @@ fn type_is_signed(cx: ctxt, ty: t) -> bool {
     }
 }
 
-// Whether a type is Plain Old Data (i.e. can be safely memmoved).
+// Whether a type is Plain Old Data -- meaning it does not contain pointers
+// that the cycle collector might care about.
 fn type_is_pod(cx: ctxt, ty: t) -> bool {
     let result = true;
     alt struct(cx, ty) {

trunk/src/rt/rust_task.cpp

@@ -18,11 +18,11 @@
 // The amount of extra space at the end of each stack segment, available
 // to the rt, compiler and dynamic linker for running small functions
 // FIXME: We want this to be 128 but need to slim the red zone calls down
-#define RZ_LINUX_32 1024*20
-#define RZ_LINUX_64 1024*20
-#define RZ_MAC_32   1024*20
-#define RZ_MAC_64   1024*20
-#define RZ_WIN_32   1024*20
+#define RZ_LINUX_32 (1024*20)
+#define RZ_LINUX_64 (1024*20)
+#define RZ_MAC_32   (1024*20)
+#define RZ_MAC_64   (1024*20)
+#define RZ_WIN_32   (1024*20)
 
 #ifdef __linux__
 #ifdef __i386__
@@ -49,6 +49,12 @@
 #endif
 #endif
 
+// A value that goes at the end of the stack and must not be touched
+const uint8_t stack_canary[] = {0xAB, 0xCD, 0xAB, 0xCD,
+                                0xAB, 0xCD, 0xAB, 0xCD,
+                                0xAB, 0xCD, 0xAB, 0xCD,
+                                0xAB, 0xCD, 0xAB, 0xCD};
+
 // Stack size
 size_t g_custom_min_stack_size = 0;
 
@@ -95,7 +101,8 @@ config_valgrind_stack(stk_seg *stk) {
     // old stack segments, since the act of popping the stack previously
     // caused valgrind to consider the whole thing inaccessible.
     size_t sz = stk->end - (uintptr_t)&stk->data[0];
-    VALGRIND_MAKE_MEM_UNDEFINED(stk->data, sz);
+    VALGRIND_MAKE_MEM_UNDEFINED(stk->data + sizeof(stack_canary),
+                                sz - sizeof(stack_canary));
 #endif
 }
 
@@ -110,6 +117,18 @@ free_stk(rust_task *task, stk_seg *stk) {
     task->free(stk);
 }
 
+static void
+add_stack_canary(stk_seg *stk) {
+    memcpy(stk->data, stack_canary, sizeof(stack_canary));
+    assert(sizeof(stack_canary) == 16 && "Stack canary was not the expected size");
+}
+
+static void
+check_stack_canary(stk_seg *stk) {
+    assert(!memcmp(stk->data, stack_canary, sizeof(stack_canary))
+      && "Somebody killed the canary");
+}
+
 static stk_seg*
 new_stk(rust_scheduler *sched, rust_task *task, size_t requested_sz)
 {
@@ -151,6 +170,7 @@ new_stk(rust_scheduler *sched, rust_task *task, size_t requested_sz)
     stk_seg *stk = (stk_seg *)task->malloc(sz, "stack");
     LOGPTR(task->sched, "new stk", (uintptr_t)stk);
     memset(stk, 0, sizeof(stk_seg));
+    add_stack_canary(stk);
     stk->prev = NULL;
     stk->next = task->stk;
     stk->end = (uintptr_t) &stk->data[rust_stk_sz + RED_ZONE_SIZE];
@@ -165,6 +185,7 @@ static void
 del_stk(rust_task *task, stk_seg *stk)
 {
     assert(stk == task->stk && "Freeing stack segments out of order!");
+    check_stack_canary(stk);
 
     task->stk = stk->next;
 
@@ -268,25 +289,18 @@ struct rust_closure_env {
     type_desc *td;
 };
 
-// This runs on the Rust stack
-extern "C" CDECL
-void task_start_wrapper(spawn_args *a)
-{
-    rust_task *task = a->task;
-    int rval = 42;
+struct cleanup_args {
+    spawn_args *spargs;
+    bool failed;
+};
 
-    bool failed = false;
-    try {
-        a->f(&rval, a->a3, a->a4);
-    } catch (rust_task *ex) {
-        A(task->sched, ex == task,
-          "Expected this task to be thrown for unwinding");
-        failed = true;
-    }
+void
+cleanup_task(cleanup_args *args) {
+    spawn_args *a = args->spargs;
+    bool failed = args->failed;
+    rust_task *task = a->task;
 
-    // We're on the Rust stack and the cycle collector may recurse arbitrarily
-    // deep, so switch to the C stack
-    task->sched->c_context.call_shim_on_c_stack(task, (void*)cc::do_cc);
+    cc::do_cc(task);
 
     rust_closure_env* env = (rust_closure_env*)a->a3;
     if(env) {
@@ -313,6 +327,29 @@ void task_start_wrapper(spawn_args *a)
         A(task->sched, false, "Shouldn't happen");
 #endif
     }
+}
+
+// This runs on the Rust stack
+extern "C" CDECL
+void task_start_wrapper(spawn_args *a)
+{
+    rust_task *task = a->task;
+    int rval = 42;
+
+    bool failed = false;
+    try {
+        a->f(&rval, a->a3, a->a4);
+    } catch (rust_task *ex) {
+        A(task->sched, ex == task,
+          "Expected this task to be thrown for unwinding");
+        failed = true;
+    }
+
+    cleanup_args ca = {a, failed};
+
+    // The cleanup work needs lots of stack
+    task->sched->c_context.call_shim_on_c_stack(&ca, (void*)cleanup_task);
+
     task->ctx.next->swap(task->ctx);
 }
 
@@ -768,6 +805,11 @@ rust_task::on_rust_stack() {
     return sp_in_stk_seg(get_sp(), stk);
 }
 
+void
+rust_task::check_stack_canary() {
+    ::check_stack_canary(stk);
+}
+
 //
 // Local Variables:
 // mode: C++

trunk/src/rt/rust_task.h

@@ -203,6 +203,7 @@ rust_task : public kernel_owned<rust_task>, rust_cond
     void record_stack_limit();
     void reset_stack_limit();
     bool on_rust_stack();
+    void check_stack_canary();
 };
 
 //

trunk/src/rt/rust_upcall.cpp

@@ -27,14 +27,21 @@ check_stack_alignment() __attribute__ ((aligned (16)));
 static void check_stack_alignment() { }
 #endif
 
+static inline void
+do_sanity_check(rust_task *task) {
+    check_stack_alignment();
+    task->check_stack_canary();
+}
+
 #define UPCALL_SWITCH_STACK(A, F) call_upcall_on_c_stack((void*)A, (void*)F)
 
 inline void
 call_upcall_on_c_stack(void *args, void *fn_ptr) {
-    check_stack_alignment();
     rust_task *task = rust_scheduler::get_task();
+    do_sanity_check(task);
     rust_scheduler *sched = task->sched;
     sched->c_context.call_shim_on_c_stack(args, fn_ptr);
+    do_sanity_check(task);
 }
 
 extern "C" void record_sp(void *limit);
@@ -48,8 +55,8 @@ extern "C" void record_sp(void *limit);
  */
 extern "C" CDECL void
 upcall_call_shim_on_c_stack(void *args, void *fn_ptr) {
-    check_stack_alignment();
     rust_task *task = rust_scheduler::get_task();
+    do_sanity_check(task);
 
     // FIXME (1226) - The shim functions generated by rustc contain the
     // morestack prologue, so we need to let them know they have enough
@@ -65,6 +72,7 @@ upcall_call_shim_on_c_stack(void *args, void *fn_ptr) {
 
     task = rust_scheduler::get_task();
     task->record_stack_limit();
+    do_sanity_check(task);
 }
 
 /**********************************************************************/
@@ -634,9 +642,10 @@ upcall_del_stack() {
 // needs to acquire the value of the stack pointer
 extern "C" CDECL void
 upcall_reset_stack_limit() {
-    check_stack_alignment();
     rust_task *task = rust_scheduler::get_task();
+    do_sanity_check(task);
     task->reset_stack_limit();
+    do_sanity_check(task);
 }
 
 //