Clean up the stack implementations

src/generator.rs

@@ -16,7 +16,6 @@ use core::{ptr, mem};
 use core::cell::Cell;
 
 use stack;
-use debug;
 use arch::{self, StackPointer};
 
 #[derive(Debug, Clone, Copy)]
@@ -83,7 +82,6 @@ pub enum State {
 pub struct Generator<'a, Input: Send + 'a, Output: Send + 'a, Stack: stack::Stack> {
   state:     State,
   stack:     Stack,
-  stack_id:  debug::StackId,
   stack_ptr: arch::StackPointer,
   phantom:   PhantomData<(&'a (), *mut Input, *const Output)>
 }
@@ -126,7 +124,6 @@ impl<'a, Input, Output, Stack> Generator<'a, Input, Output, Stack>
       loop { yielder.suspend_bare(None); }
     }
 
-    let stack_id  = debug::StackId::register(&stack);
     let stack_ptr = arch::init(&stack, generator_wrapper::<Input, Output, Stack, F>);
 
     // Transfer environment to the callee.
@@ -136,7 +133,6 @@ impl<'a, Input, Output, Stack> Generator<'a, Input, Output, Stack>
     Generator {
       state:     State::Runnable,
       stack:     stack,
-      stack_id:  stack_id,
       stack_ptr: stack_ptr,
       phantom:   PhantomData
     }

src/lib.rs

@@ -33,30 +33,14 @@
 #[macro_use]
 extern crate std;
 
-pub use stack::Stack;
-pub use stack::GuardedStack;
-pub use slice_stack::SliceStack;
+pub use stack::*;
 pub use generator::Generator;
 
-#[cfg(feature = "alloc")]
-pub use owned_stack::OwnedStack;
-
-#[cfg(unix)]
-pub use os::Stack as OsStack;
-
 mod arch;
 
 /// Minimum alignment of a stack base address on the target platform.
 pub const STACK_ALIGNMENT: usize = arch::STACK_ALIGNMENT;
 
-mod debug;
-
-mod stack;
-mod slice_stack;
 pub mod generator;
 
-#[cfg(feature = "alloc")]
-mod owned_stack;
-
-#[cfg(unix)]
-mod os;
+mod stack;

src/debug/mod.rs → src/stack/debug/mod.rs

@@ -12,12 +12,11 @@ mod imp;
 
 #[cfg(not(feature = "valgrind"))]
 mod imp {
-  use stack;
   #[derive(Debug)]
   pub struct StackId;
   /// No-op since no valgrind
   impl StackId {
-    pub fn register<Stack: stack::Stack>(_stack: &Stack) -> StackId {
+    pub fn register(_limit: *mut u8, _base: *mut u8) -> StackId {
       StackId
     }
   }

src/debug/valgrind.rs → src/stack/debug/valgrind.rs

@@ -6,16 +6,15 @@
 // copied, modified, or distributed except according to those terms.
 extern crate valgrind_request;
 
-use stack;
 use self::valgrind_request::{stack_register, stack_deregister};
 
 #[derive(Debug)]
 pub struct StackId(self::valgrind_request::Value);
 
 impl StackId {
   #[inline(always)]
-  pub fn register<Stack: stack::Stack>(stack: &Stack) -> StackId {
-    StackId(stack_register(stack.limit(), stack.base()))
+  pub fn register(limit: *mut u8, base: *mut u8) -> StackId {
+    StackId(stack_register(limit, base))
   }
 }
 

src/stack.rs → src/stack/mod.rs

@@ -6,6 +6,21 @@
 // copied, modified, or distributed except according to those terms.
 //! Traits for stacks.
 
+mod debug;
+
+mod slice_stack;
+pub use stack::slice_stack::SliceStack;
+
+#[cfg(feature = "alloc")]
+mod owned_stack;
+#[cfg(feature = "alloc")]
+pub use stack::owned_stack::OwnedStack;
+
+#[cfg(unix)]
+mod os;
+#[cfg(unix)]
+pub use stack::os::OsStack;
+
 /// A trait for objects that hold ownership of a stack.
 ///
 /// To preserve memory safety, an implementation of this trait must fulfill
@@ -16,7 +31,7 @@
 ///   * Every address between the base and the limit must be readable and writable.
 ///
 /// [align]: constant.STACK_ALIGNMENT.html
-pub trait Stack {
+pub unsafe trait Stack {
   /// Returns the base address of the stack.
   /// On all modern architectures, the stack grows downwards,
   /// so this is the highest address.

src/os/mod.rs → src/stack/os/mod.rs

@@ -4,28 +4,32 @@
 // http://apache.org/licenses/LICENSE-2.0> or the MIT license <LICENSE-MIT or
 // http://opensource.org/licenses/MIT>, at your option. This file may not be
 // copied, modified, or distributed except according to those terms.
+
 extern crate std;
+
 use self::std::io::Error as IoError;
-use stack;
+use stack::{Stack, GuardedStack};
+use stack::debug::StackId;
 
 mod sys;
 
 /// OsStack holds a guarded stack allocated using the operating system's anonymous
 /// memory mapping facility.
 #[derive(Debug)]
-pub struct Stack {
+pub struct OsStack {
   ptr: *mut u8,
-  len: usize
+  len: usize,
+  id: StackId,
 }
 
-unsafe impl Send for Stack {}
+unsafe impl Send for OsStack {}
 
-impl Stack {
+impl OsStack {
   /// Allocates a new stack with at least `size` accessible bytes.
-  /// `size` is rounded up to an integral number of pages; `Stack::new(0)` is legal
+  /// `size` is rounded up to an integral number of pages; `OsStack::new(0)` is legal
   /// and allocates the smallest possible stack, consisting of one data page and
   /// one guard page.
-  pub fn new(size: usize) -> Result<Stack, IoError> {
+  pub fn new(size: usize) -> Result<OsStack, IoError> {
     let page_size = sys::page_size();
 
     // Stacks have to be at least one page long.
@@ -39,9 +43,11 @@ impl Stack {
     let len = len + page_size;
 
     // Allocate a stack.
-    let stack = Stack {
-      ptr: try!(unsafe { sys::map_stack(len) }),
-      len: len
+    let ptr = try!(unsafe { sys::map_stack(len) });
+    let stack = OsStack {
+      ptr: ptr,
+      len: len,
+      id: StackId::register(ptr, unsafe { ptr.offset(len as isize) }),
     };
 
     // Mark the guard page. If this fails, `stack` will be dropped,
@@ -52,7 +58,7 @@ impl Stack {
   }
 }
 
-impl stack::Stack for Stack {
+unsafe impl Stack for OsStack {
   #[inline(always)]
   fn base(&self) -> *mut u8 {
     unsafe {
@@ -68,9 +74,9 @@ impl stack::Stack for Stack {
   }
 }
 
-unsafe impl stack::GuardedStack for Stack {}
+unsafe impl GuardedStack for OsStack {}
 
-impl Drop for Stack {
+impl Drop for OsStack {
   fn drop(&mut self) {
     unsafe { sys::unmap_stack(self.ptr, self.len) }.expect("cannot unmap stack")
   }

src/owned_stack.rs → src/stack/owned_stack.rs

@@ -1,38 +1,47 @@
 // This file is part of libfringe, a low-level green threading library.
 // Copyright (c) whitequark <whitequark@whitequark.org>
 // See the LICENSE file included in this distribution.
+
 extern crate alloc;
 
 use core::slice;
 use self::alloc::heap;
 use self::alloc::boxed::Box;
+use stack::Stack;
+use stack::debug::StackId;
 
 /// OwnedStack holds a non-guarded, heap-allocated stack.
 #[derive(Debug)]
-pub struct OwnedStack(Box<[u8]>);
+pub struct OwnedStack {
+    data: Box<[u8]>,
+    id: StackId,
+}
 
 impl OwnedStack {
     /// Allocates a new stack with exactly `size` accessible bytes and alignment appropriate
     /// for the current platform using the default Rust allocator.
     pub fn new(size: usize) -> OwnedStack {
         unsafe {
             let ptr = heap::allocate(size, ::STACK_ALIGNMENT);
-            OwnedStack(Box::from_raw(slice::from_raw_parts_mut(ptr, size)))
+            OwnedStack {
+                data: Box::from_raw(slice::from_raw_parts_mut(ptr, size)),
+                id: StackId::register(ptr, ptr.offset(size as isize)),
+            }
         }
     }
 }
 
-impl ::stack::Stack for OwnedStack {
+unsafe impl Stack for OwnedStack {
     #[inline(always)]
     fn base(&self) -> *mut u8 {
         // The slice cannot wrap around the address space, so the conversion from usize
         // to isize will not wrap either.
-        let len: isize = self.0.len() as isize;
+        let len = self.data.len() as isize;
         unsafe { self.limit().offset(len) }
     }
 
     #[inline(always)]
     fn limit(&self) -> *mut u8 {
-        self.0.as_ptr() as *mut u8
+        self.data.as_ptr() as *mut u8
     }
 }

src/stack/slice_stack.rs

@@ -0,0 +1,48 @@
+// This file is part of libfringe, a low-level green threading library.
+// Copyright (c) whitequark <whitequark@whitequark.org>
+// See the LICENSE file included in this distribution.
+
+use stack::Stack;
+use stack::debug::StackId;
+
+/// SliceStack holds a non-guarded stack allocated elsewhere and provided as a mutable slice.
+#[derive(Debug)]
+pub struct SliceStack<'a> {
+    data: &'a mut [u8],
+    id: StackId,
+}
+
+impl<'a> SliceStack<'a> {
+    /// Creates a `SliceStack` from an existing slice.
+    ///
+    /// This function will automatically align the slice to make it suitable for
+    /// use as a stack. However this function may panic if the slice is smaller
+    /// than `STACK_ALIGNMENT`.
+    pub fn new(slice: &'a mut [u8]) -> SliceStack<'a> {
+        // Align the given slice so that it matches platform requirements
+        let ptr = slice.as_ptr() as usize;
+        let adjusted_ptr = (ptr + ::STACK_ALIGNMENT - 1) & !(::STACK_ALIGNMENT - 1);
+        let offset = adjusted_ptr - ptr;
+        let adjusted_len = (slice.len() - offset) & !(::STACK_ALIGNMENT - 1);
+
+        SliceStack {
+            id: StackId::register(adjusted_ptr as *mut u8, (adjusted_ptr + adjusted_len) as *mut u8),
+            data: &mut slice[offset..(offset + adjusted_len)],
+        }
+    }
+}
+
+unsafe impl<'a> Stack for SliceStack<'a> {
+    #[inline(always)]
+    fn base(&self) -> *mut u8 {
+        // The slice cannot wrap around the address space, so the conversion from usize
+        // to isize will not wrap either.
+        let len = self.data.len() as isize;
+        unsafe { self.limit().offset(len) }
+    }
+
+    #[inline(always)]
+    fn limit(&self) -> *mut u8 {
+        self.data.as_ptr() as *mut u8
+    }
+}

tests/generator.rs

@@ -69,7 +69,7 @@ fn panic_safety() {
 #[test]
 fn with_slice_stack() {
   let mut memory = [0; 1024];
-  let stack = SliceStack(&mut memory);
+  let stack = SliceStack::new(&mut memory);
   let mut add_one = unsafe { Generator::unsafe_new(stack, add_one_fn) };
   assert_eq!(add_one.resume(1), Some(2));
   assert_eq!(add_one.resume(2), Some(3));

tests/stack.rs

@@ -11,7 +11,7 @@ use fringe::{Stack, SliceStack, OwnedStack, OsStack};
 #[test]
 fn slice_stack() {
     let mut memory = [0; 1024];
-    let stack = SliceStack(&mut memory);
+    let stack = SliceStack::new(&mut memory);
     assert_eq!(stack.base() as isize - stack.limit() as isize, 1024);
 }