From 88d9ce43ae90177d2c6211c2a523aa9732f606a3 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 12 Jan 2017 11:11:12 +0100 Subject: [PATCH 1/5] Remove the type parameter from GCMethods. Fixes #256. --- src/lib.rs | 4 ++-- src/rust.rs | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index af366e41d..53e2232a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,7 +27,7 @@ pub mod jsapi { #[repr(C)] #[derive(Debug)] - pub struct Heap + Copy> { + pub struct Heap { pub ptr: ::std::cell::UnsafeCell, } @@ -124,7 +124,7 @@ pub unsafe fn JS_CALLEE(_cx: *mut JSContext, vp: *mut JSVal) -> JSVal { } // This is measured properly by the heap measurement implemented in SpiderMonkey. -impl> HeapSizeOf for Heap { +impl HeapSizeOf for Heap { fn heap_size_of_children(&self) -> usize { 0 } diff --git a/src/rust.rs b/src/rust.rs index 3ed69d7dc..8eca377cf 100644 --- a/src/rust.rs +++ b/src/rust.rs @@ -564,17 +564,17 @@ const ChunkSize: usize = 1 << ChunkShift; #[cfg(target_pointer_width = "32")] const ChunkLocationOffset: usize = ChunkSize - 2 * 4 - 8; -pub trait GCMethods { - unsafe fn initial() -> T; - unsafe fn post_barrier(v: *mut T, prev: T, next: T); +pub trait GCMethods { + unsafe fn initial() -> Self; + unsafe fn post_barrier(v: *mut Self, prev: Self, next: Self); } -impl GCMethods for jsid { +impl GCMethods for jsid { unsafe fn initial() -> jsid { JSID_VOID } unsafe fn post_barrier(_: *mut jsid, _: jsid, _: jsid) {} } -impl GCMethods<*mut JSObject> for *mut JSObject { +impl GCMethods for *mut JSObject { unsafe fn initial() -> *mut JSObject { ptr::null_mut() } unsafe fn post_barrier(v: *mut *mut JSObject, prev: *mut JSObject, next: *mut JSObject) { @@ -582,17 +582,17 @@ impl GCMethods<*mut JSObject> for *mut JSObject { } } -impl GCMethods<*mut JSString> for *mut JSString { +impl GCMethods for *mut JSString { unsafe fn initial() -> *mut JSString { ptr::null_mut() } unsafe fn post_barrier(_: *mut *mut JSString, _: *mut JSString, _: *mut JSString) {} } -impl GCMethods<*mut JSScript> for *mut JSScript { +impl GCMethods for *mut JSScript { unsafe fn initial() -> *mut JSScript { ptr::null_mut() } unsafe fn post_barrier(_: *mut *mut JSScript, _: *mut JSScript, _: *mut JSScript) { } } -impl GCMethods<*mut JSFunction> for *mut JSFunction { +impl GCMethods for *mut JSFunction { unsafe fn initial() -> *mut JSFunction { ptr::null_mut() } unsafe fn post_barrier(v: *mut *mut JSFunction, prev: *mut JSFunction, next: *mut JSFunction) { @@ -601,14 +601,14 @@ impl GCMethods<*mut JSFunction> for *mut JSFunction { } } -impl GCMethods for Value { +impl GCMethods for Value { unsafe fn initial() -> Value { UndefinedValue() } unsafe fn post_barrier(v: *mut Value, prev: Value, next: Value) { HeapValuePostBarrier(v, &prev, &next); } } -impl + Copy> Heap { +impl Heap { pub fn new(v: T) -> Heap where Heap: Default { @@ -641,7 +641,7 @@ impl + Copy> Heap { } } -impl + Copy> Clone for Heap +impl Clone for Heap where Heap: Default { fn clone(&self) -> Self { @@ -650,7 +650,7 @@ impl + Copy> Clone for Heap } impl Default for Heap<*mut T> - where *mut T: GCMethods<*mut T> + Copy + where *mut T: GCMethods + Copy { fn default() -> Heap<*mut T> { Heap { @@ -667,7 +667,7 @@ impl Default for Heap { } } -impl + Copy> Drop for Heap { +impl Drop for Heap { fn drop(&mut self) { unsafe { let ptr = self.ptr.get(); @@ -676,7 +676,7 @@ impl + Copy> Drop for Heap { } } -impl + Copy + PartialEq> PartialEq for Heap { +impl PartialEq for Heap { fn eq(&self, other: &Self) -> bool { self.get() == other.get() } From 00387d595f35c5efaeaa23ee85534d348067e53c Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 12 Jan 2017 13:43:04 +0100 Subject: [PATCH 2/5] Disallow constructing an unrooted Rooted with a value. This should make it harder to use Rooted in an unsound way (when not using the rooted!() macro). --- src/conversions.rs | 2 +- src/rust.rs | 19 ++++++++++++------- src/typedarray.rs | 15 +++++++++------ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/conversions.rs b/src/conversions.rs index a6065b055..ab25ab447 100644 --- a/src/conversions.rs +++ b/src/conversions.rs @@ -576,7 +576,7 @@ impl> FromJSValConvertible for Vec Result>, ()> { let mut iterator = ForOfIterator { cx_: cx, - iterator: RootedObject::new_unrooted(ptr::null_mut()), + iterator: RootedObject::new_unrooted(), index: ::std::u32::MAX, // NOT_ARRAY }; let mut iterator = ForOfIteratorGuard::new(cx, &mut iterator); diff --git a/src/rust.rs b/src/rust.rs index 8eca377cf..c5e22e9df 100644 --- a/src/rust.rs +++ b/src/rust.rs @@ -342,12 +342,14 @@ unsafe impl Trace for Heap { } impl Rooted { - pub fn new_unrooted(initial: T) -> Rooted { + pub fn new_unrooted() -> Rooted + where T: GCMethods, + { Rooted { _base: RootedBase { _phantom0: PhantomData }, stack: ptr::null_mut(), prev: ptr::null_mut(), - ptr: initial, + ptr: unsafe { T::initial() }, } } @@ -375,7 +377,10 @@ pub struct RootedGuard<'a, T: 'a> { } impl<'a, T> RootedGuard<'a, T> { - pub fn new(cx: *mut JSContext, root: &'a mut Rooted) -> Self where T: RootKind { + pub fn new(cx: *mut JSContext, root: &'a mut Rooted, initial: T) -> Self + where T: RootKind + { + root.ptr = initial; unsafe { root.add_to_root_stack(cx); } @@ -429,12 +434,12 @@ impl<'a, T> Drop for RootedGuard<'a, T> { #[macro_export] macro_rules! rooted { (in($cx:expr) let $name:ident = $init:expr) => { - let mut __root = $crate::jsapi::Rooted::new_unrooted($init); - let $name = $crate::rust::RootedGuard::new($cx, &mut __root); + let mut __root = $crate::jsapi::Rooted::new_unrooted(); + let $name = $crate::rust::RootedGuard::new($cx, &mut __root, $init); }; (in($cx:expr) let mut $name:ident = $init:expr) => { - let mut __root = $crate::jsapi::Rooted::new_unrooted($init); - let mut $name = $crate::rust::RootedGuard::new($cx, &mut __root); + let mut __root = $crate::jsapi::Rooted::new_unrooted(); + let mut $name = $crate::rust::RootedGuard::new($cx, &mut __root, $init); } } diff --git a/src/typedarray.rs b/src/typedarray.rs index bf9a0f9e1..00e205a42 100644 --- a/src/typedarray.rs +++ b/src/typedarray.rs @@ -66,9 +66,12 @@ impl<'a, T: TypedArrayElement> TypedArray<'a, T> { /// Create a typed array representation that wraps an existing JS reflector. /// This operation will fail if attempted on a JS object that does not match /// the expected typed array details. - pub fn from(cx: *mut JSContext, root: &'a mut Rooted<*mut JSObject>) -> Result { + pub fn from(cx: *mut JSContext, + root: &'a mut Rooted<*mut JSObject>, + object: *mut JSObject) + -> Result { unsafe { - let mut guard = RootedGuard::new(cx, root); + let mut guard = RootedGuard::new(cx, root, object); let unwrapped = T::unwrap_array(*guard); if unwrapped.is_null() { return Err(()); @@ -297,11 +300,11 @@ pub type ArrayBufferView<'a> = TypedArray<'a, ArrayBufferViewU8>; #[macro_export] macro_rules! typedarray { (in($cx:expr) let $name:ident : $ty:ident = $init:expr) => { - let mut __root = $crate::jsapi::Rooted::new_unrooted($init); - let $name = $crate::typedarray::$ty::from($cx, &mut __root); + let mut __root = $crate::jsapi::Rooted::new_unrooted(); + let $name = $crate::typedarray::$ty::from($cx, &mut __root, $init); }; (in($cx:expr) let mut $name:ident : $ty:ident = $init:expr) => { - let mut __root = $crate::jsapi::Rooted::new_unrooted($init); - let mut $name = $crate::typedarray::$ty::from($cx, &mut __root); + let mut __root = $crate::jsapi::Rooted::new_unrooted(); + let mut $name = $crate::typedarray::$ty::from($cx, &mut __root, $init); } } From bd8b76bd43868ac820ad11310d592e5035935b70 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 12 Jan 2017 13:50:21 +0100 Subject: [PATCH 3/5] Consistently apply the same bounds to RootedGuard's type parameter. --- src/rust.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/rust.rs b/src/rust.rs index c5e22e9df..7a614ab77 100644 --- a/src/rust.rs +++ b/src/rust.rs @@ -372,14 +372,12 @@ impl Rooted { /// Rust API for keeping a Rooted value in the context's root stack. /// Example usage: `rooted!(in(cx) let x = UndefinedValue());`. /// `RootedGuard::new` also works, but the macro is preferred. -pub struct RootedGuard<'a, T: 'a> { +pub struct RootedGuard<'a, T: 'a + RootKind + GCMethods> { root: &'a mut Rooted } -impl<'a, T> RootedGuard<'a, T> { - pub fn new(cx: *mut JSContext, root: &'a mut Rooted, initial: T) -> Self - where T: RootKind - { +impl<'a, T: 'a + RootKind + GCMethods> RootedGuard<'a, T> { + pub fn new(cx: *mut JSContext, root: &'a mut Rooted, initial: T) -> Self { root.ptr = initial; unsafe { root.add_to_root_stack(cx); @@ -410,20 +408,20 @@ impl<'a, T> RootedGuard<'a, T> { } } -impl<'a, T> Deref for RootedGuard<'a, T> { +impl<'a, T: 'a + RootKind + GCMethods> Deref for RootedGuard<'a, T> { type Target = T; fn deref(&self) -> &T { &self.root.ptr } } -impl<'a, T> DerefMut for RootedGuard<'a, T> { +impl<'a, T: 'a + RootKind + GCMethods> DerefMut for RootedGuard<'a, T> { fn deref_mut(&mut self) -> &mut T { &mut self.root.ptr } } -impl<'a, T> Drop for RootedGuard<'a, T> { +impl<'a, T: 'a + RootKind + GCMethods> Drop for RootedGuard<'a, T> { fn drop(&mut self) { unsafe { self.root.remove_from_root_stack(); From 557a1ef9a9330246f7101562ed5f0f2d3aaea49c Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 12 Jan 2017 13:58:23 +0100 Subject: [PATCH 4/5] Clear the value of a Rooted when the RootedGuard goes out of scope. This ensures no unrooted values linger in the Rooted when it is no longer rooted. It should be impossible to access the value after that, but Handles currently lack the lifetime to prevent that (#153). --- src/rust.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rust.rs b/src/rust.rs index 7a614ab77..acfe70503 100644 --- a/src/rust.rs +++ b/src/rust.rs @@ -424,6 +424,7 @@ impl<'a, T: 'a + RootKind + GCMethods> DerefMut for RootedGuard<'a, T> { impl<'a, T: 'a + RootKind + GCMethods> Drop for RootedGuard<'a, T> { fn drop(&mut self) { unsafe { + self.root.ptr = T::initial(); self.root.remove_from_root_stack(); } } From c87c20861b949cb2b09146b55ec1dd4cf1a170cc Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 12 Jan 2017 14:26:36 +0100 Subject: [PATCH 5/5] Simplify add_to_root_stack and remove_from_root_stack. --- src/rust.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/rust.rs b/src/rust.rs index acfe70503..d5ea5ec3e 100644 --- a/src/rust.rs +++ b/src/rust.rs @@ -354,17 +354,19 @@ impl Rooted { } pub unsafe fn add_to_root_stack(&mut self, cx: *mut JSContext) where T: RootKind { - let ctxfriend: &mut ContextFriendFields = mem::transmute(cx); + let ctxfriend = cx as *mut ContextFriendFields; let kind = T::rootKind() as usize; - self.stack = &mut ctxfriend.roots.stackRoots_[kind] as *mut _ as *mut _; - self.prev = ctxfriend.roots.stackRoots_[kind] as *mut _; + let stack = &mut (*ctxfriend).roots.stackRoots_[kind] as *mut _ as *mut _; - ctxfriend.roots.stackRoots_[kind] = self as *mut _ as usize as _; + self.stack = stack; + self.prev = *stack; + + *stack = self as *mut _ as usize as _; } pub unsafe fn remove_from_root_stack(&mut self) { - assert!(*self.stack == mem::transmute(&*self)); + assert!(*self.stack == self as *mut _ as usize as _); *self.stack = self.prev; } }