Remove HandleValueArray::from_rooted_slice footgun (#533)

* Remove HandleValueArray::from_rooted_slice footgun. Signed-off-by: Josh Matthews <[email protected]> * Add iterator constructor for RootedVec. Signed-off-by: Josh Matthews <[email protected]> * Introduce optional crown annotations. Signed-off-by: Josh Matthews <[email protected]> * Formatting. Signed-off-by: Josh Matthews <[email protected]> * Update wasm example. Signed-off-by: Josh Matthews <[email protected]> * Formatting. Signed-off-by: Josh Matthews <[email protected]> --------- Signed-off-by: Josh Matthews <[email protected]>
servo · Dec 13, 2024 · d6c3bd9 · d6c3bd9
1 parent e299b24
commit d6c3bd9
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 23 deletions.
diff --git a/mozjs-sys/src/jsgc.rs b/mozjs-sys/src/jsgc.rs
@@ -249,13 +249,6 @@ impl<const N: usize> ValueArray<N> {
         Self { elements }
     }
 
-    pub fn to_handle_value_array(&self) -> JS::HandleValueArray {
-        JS::HandleValueArray {
-            length_: N,
-            elements_: self.elements.as_ptr(),
-        }
-    }
-
     pub unsafe fn get_ptr(&self) -> *const JS::Value {
         self.elements.as_ptr()
     }

diff --git a/mozjs-sys/src/jsimpls.rs b/mozjs-sys/src/jsimpls.rs
@@ -20,9 +20,9 @@ use crate::jsapi::JSPropertySpec_Kind;
 use crate::jsapi::JSPropertySpec_Name;
 use crate::jsapi::JS;
 use crate::jsapi::JS::Scalar::Type;
-use crate::jsgc::{RootKind, RootedBase};
+use crate::jsgc::{RootKind, Rooted, RootedBase, ValueArray};
 use crate::jsid::VoidId;
-use crate::jsval::UndefinedValue;
+use crate::jsval::{JSVal, UndefinedValue};
 
 use std::marker::PhantomData;
 use std::ops::Deref;
@@ -132,17 +132,37 @@ impl JS::HandleValue {
 }
 
 impl JS::HandleValueArray {
-    pub fn new() -> JS::HandleValueArray {
+    pub fn empty() -> JS::HandleValueArray {
         JS::HandleValueArray {
             length_: 0,
             elements_: ptr::null(),
         }
     }
+}
+
+impl<const N: usize> From<&Rooted<ValueArray<N>>> for JS::HandleValueArray {
+    fn from(array: &Rooted<ValueArray<N>>) -> JS::HandleValueArray {
+        JS::HandleValueArray {
+            length_: N,
+            elements_: unsafe { array.ptr.get_ptr() },
+        }
+    }
+}
+
+impl From<&JS::CallArgs> for JS::HandleValueArray {
+    fn from(args: &JS::CallArgs) -> JS::HandleValueArray {
+        JS::HandleValueArray {
+            length_: args.argc_ as usize,
+            elements_: args.argv_ as *const _,
+        }
+    }
+}
 
-    pub unsafe fn from_rooted_slice(values: &[JS::Value]) -> JS::HandleValueArray {
+impl From<JS::Handle<JSVal>> for JS::HandleValueArray {
+    fn from(handle: JS::Handle<JSVal>) -> JS::HandleValueArray {
         JS::HandleValueArray {
-            length_: values.len(),
-            elements_: values.as_ptr(),
+            length_: 1,
+            elements_: handle.ptr,
         }
     }
 }

diff --git a/mozjs/Cargo.toml b/mozjs/Cargo.toml
@@ -15,6 +15,7 @@ debugmozjs = ['mozjs_sys/debugmozjs']
 jitspew = ['mozjs_sys/jitspew']
 profilemozjs = ['mozjs_sys/profilemozjs']
 streams = ['mozjs_sys/streams']
+crown = []
 
 [dependencies]
 lazy_static = "1"

diff --git a/mozjs/examples/wasm.rs b/mozjs/examples/wasm.rs
@@ -16,7 +16,7 @@ use mozjs::jsval::UndefinedValue;
 use mozjs::rooted;
 use mozjs::rust::jsapi_wrapped::{Construct1, JS_GetProperty, JS_SetProperty};
 use mozjs::rust::SIMPLE_GLOBAL_CLASS;
-use mozjs::rust::{JSEngine, RealmOptions, Runtime};
+use mozjs::rust::{IntoHandle, JSEngine, RealmOptions, Runtime};
 use mozjs_sys::jsgc::ValueArray;
 
 #[repr(align(8))]
@@ -94,10 +94,7 @@ fn run(rt: Runtime) {
             assert!(!array_buffer.is_null());
 
             rooted!(in(rt.cx()) let val = ObjectValue(array_buffer));
-            let args = HandleValueArray {
-                length_: 1,
-                elements_: &*val,
-            };
+            let args = HandleValueArray::from(val.handle().into_handle());
 
             assert!(Construct1(
                 rt.cx(),
@@ -134,12 +131,11 @@ fn run(rt: Runtime) {
             ));
 
             rooted!(in(rt.cx()) let mut args = ValueArray::new([ObjectValue(module.get()), ObjectValue(imports.get())]));
-            let handle = args.handle();
 
             assert!(Construct1(
                 rt.cx(),
                 wasm_instance.handle(),
-                &handle.to_handle_value_array(),
+                &HandleValueArray::from(&args),
                 &mut instance.handle_mut()
             ));
         }
@@ -169,7 +165,7 @@ fn run(rt: Runtime) {
             rt.cx(),
             JS::UndefinedHandleValue,
             foo.handle().into(),
-            &HandleValueArray::new(),
+            &HandleValueArray::empty(),
             rval.handle_mut().into()
         ));
 

diff --git a/mozjs/src/gc/collections.rs b/mozjs/src/gc/collections.rs
@@ -1,11 +1,18 @@
 use crate::gc::{RootedTraceableSet, Traceable};
 use crate::jsapi::{Heap, JSTracer};
 use crate::rust::Handle;
+use mozjs_sys::jsapi::JS;
 use mozjs_sys::jsgc::GCMethods;
+use mozjs_sys::jsval::JSVal;
 use std::ops::{Deref, DerefMut};
 
 /// A vector of items to be rooted with `RootedVec`.
 /// Guaranteed to be empty when not rooted.
+#[cfg_attr(feature = "crown", allow(crown::unrooted_must_root))]
+#[cfg_attr(
+    feature = "crown",
+    crown::unrooted_must_root_lint::allow_unrooted_interior
+)]
 pub struct RootableVec<T: Traceable> {
     v: Vec<T>,
 }
@@ -24,17 +31,41 @@ unsafe impl<T: Traceable> Traceable for RootableVec<T> {
 }
 
 /// A vector of items rooted for the lifetime 'a.
+#[cfg_attr(
+    feature = "crown",
+    crown::unrooted_must_root_lint::allow_unrooted_interior
+)]
 pub struct RootedVec<'a, T: Traceable + 'static> {
     root: &'a mut RootableVec<T>,
 }
 
+impl From<&RootedVec<'_, JSVal>> for JS::HandleValueArray {
+    fn from(vec: &RootedVec<'_, JSVal>) -> JS::HandleValueArray {
+        JS::HandleValueArray {
+            length_: vec.root.v.len(),
+            elements_: vec.root.v.as_ptr(),
+        }
+    }
+}
+
 impl<'a, T: Traceable + 'static> RootedVec<'a, T> {
     pub fn new(root: &'a mut RootableVec<T>) -> RootedVec<'a, T> {
         unsafe {
             RootedTraceableSet::add(root);
         }
         RootedVec { root }
     }
+
+    pub fn from_iter<I>(root: &'a mut RootableVec<T>, iter: I) -> Self
+    where
+        I: Iterator<Item = T>,
+    {
+        unsafe {
+            RootedTraceableSet::add(root);
+        }
+        root.v.extend(iter);
+        RootedVec { root }
+    }
 }
 
 impl<'a, T: Traceable + 'static> Drop for RootedVec<'a, T> {

diff --git a/mozjs/src/gc/root.rs b/mozjs/src/gc/root.rs
@@ -2,14 +2,15 @@ use std::marker::PhantomData;
 use std::ops::{Deref, DerefMut};
 use std::ptr;
 
-use crate::jsapi::{jsid, JSContext, JSFunction, JSObject, JSScript, JSString, Symbol, Value};
+use crate::jsapi::{jsid, JSContext, JSFunction, JSObject, JSScript, JSString, Symbol, Value, JS};
 use mozjs_sys::jsgc::{GCMethods, RootKind, Rooted};
 
 use crate::jsapi::Handle as RawHandle;
 use crate::jsapi::HandleValue as RawHandleValue;
 use crate::jsapi::MutableHandle as RawMutableHandle;
 use mozjs_sys::jsgc::IntoHandle as IntoRawHandle;
 use mozjs_sys::jsgc::IntoMutableHandle as IntoRawMutableHandle;
+use mozjs_sys::jsgc::ValueArray;
 
 /// Rust API for keeping a Rooted value in the context's root stack.
 /// Example usage: `rooted!(in(cx) let x = UndefinedValue());`.
@@ -69,6 +70,12 @@ impl<'a, T: 'a + RootKind + GCMethods> Drop for RootedGuard<'a, T> {
     }
 }
 
+impl<'a, const N: usize> From<&RootedGuard<'a, ValueArray<N>>> for JS::HandleValueArray {
+    fn from(array: &RootedGuard<'a, ValueArray<N>>) -> JS::HandleValueArray {
+        JS::HandleValueArray::from(&*array.root)
+    }
+}
+
 #[derive(Clone, Copy)]
 pub struct Handle<'a, T: 'a> {
     pub(crate) ptr: &'a T,

diff --git a/mozjs/src/gc/trace.rs b/mozjs/src/gc/trace.rs
@@ -1,7 +1,7 @@
 use crate::c_str;
 use crate::glue::{
     CallBigIntTracer, CallFunctionTracer, CallIdTracer, CallObjectTracer, CallScriptTracer,
-    CallStringTracer, CallSymbolTracer, CallValueTracer,
+    CallStringTracer, CallSymbolTracer, CallValueRootTracer, CallValueTracer,
 };
 use crate::jsapi::{
     jsid, JSFunction, JSObject, JSScript, JSString, JSTracer, PropertyDescriptor, Value,
@@ -110,6 +110,13 @@ unsafe impl Traceable for Heap<Value> {
     }
 }
 
+unsafe impl Traceable for Value {
+    #[inline]
+    unsafe fn trace(&self, trc: *mut JSTracer) {
+        CallValueRootTracer(trc, self as *const _ as *mut Self, c_str!("value"));
+    }
+}
+
 unsafe impl Traceable for Heap<jsid> {
     #[inline]
     unsafe fn trace(&self, trc: *mut JSTracer) {

diff --git a/mozjs/src/lib.rs b/mozjs/src/lib.rs
@@ -10,6 +10,8 @@
     non_snake_case,
     improper_ctypes
 )]
+#![cfg_attr(feature = "crown", feature(register_tool))]
+#![cfg_attr(feature = "crown", register_tool(crown))]
 
 //!
 //! This crate contains Rust bindings to the [SpiderMonkey Javascript engine][1]