Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#14902 Write a MIR lint for rooting analysis #20264

Closed
wants to merge 15 commits into from
Closed
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Fixed more violations.

  • Loading branch information
mrowqa committed May 10, 2018
commit 2416333a287c73f337ae43ae9637866e9baea774
@@ -12,14 +12,15 @@ use style::thread_state::{self, ThreadState};
/// This extends the API of `std::cell::RefCell` to allow unsafe access in
/// certain situations, with dynamic checking in debug builds.
#[derive(Clone, Debug, Default, MallocSizeOf, PartialEq)]
pub struct DomRefCell<T> {
#[must_root]
pub struct DomRefCell<#[must_root] T> {
value: RefCell<T>,
}

// Functionality specific to Servo's `DomRefCell` type
// ===================================================

impl<T> DomRefCell<T> {
impl<#[must_root] T> DomRefCell<T> {
/// Return a reference to the contents.
///
/// For use in the layout thread only.
@@ -47,7 +48,7 @@ impl<T> DomRefCell<T> {

// Functionality duplicated with `std::cell::RefCell`
// ===================================================
impl<T> DomRefCell<T> {
impl<#[must_root] T> DomRefCell<T> {
/// Create a new `DomRefCell` containing `value`.
pub fn new(value: T) -> DomRefCell<T> {
DomRefCell {
@@ -62,6 +62,7 @@ use servo_config::opts;
use std::{char, ffi, ptr, slice};

/// A trait to check whether a given `JSObject` implements an IDL interface.
#[must_root]

This comment has been minimized.

Copy link
@jdm

jdm May 31, 2018

Member

Why is this change required?

This comment has been minimized.

Copy link
@mrowqa

mrowqa Jun 4, 2018

Author

Like mentioned somewhere before - some heuristics. It is a trait that is implemented by #[must_root] types, I believe.

pub trait IDLInterface {
/// Returns whether the given DOM class derives that interface.
fn derives(&'static DOMClass) -> bool;
@@ -105,7 +106,7 @@ impl<T: Float + FromJSValConvertible<Config=()>> FromJSValConvertible for Finite
}
}

impl <T: DomObject + IDLInterface> FromJSValConvertible for DomRoot<T> {
impl <#[must_root] T: DomObject + IDLInterface> FromJSValConvertible for DomRoot<T> {
type Config = ();

unsafe fn from_jsval(_cx: *mut JSContext,
@@ -420,7 +421,7 @@ pub unsafe fn private_from_proto_check<F>(mut obj: *mut JSObject,
}

/// Get a `*const T` for a DOM object accessible from a `JSObject`.
pub fn native_from_object<T>(obj: *mut JSObject) -> Result<*const T, ()>
pub fn native_from_object<#[must_root] T>(obj: *mut JSObject) -> Result<*const T, ()>
where T: DomObject + IDLInterface
{
unsafe {
@@ -434,15 +435,15 @@ pub fn native_from_object<T>(obj: *mut JSObject) -> Result<*const T, ()>
/// Returns Err(()) if `obj` is an opaque security wrapper or if the object is
/// not a reflector for a DOM object of the given type (as defined by the
/// proto_id and proto_depth).
pub fn root_from_object<T>(obj: *mut JSObject) -> Result<DomRoot<T>, ()>
pub fn root_from_object<#[must_root] T>(obj: *mut JSObject) -> Result<DomRoot<T>, ()>
where T: DomObject + IDLInterface
{
native_from_object(obj).map(|ptr| unsafe { DomRoot::from_ref(&*ptr) })
}

/// Get a `*const T` for a DOM object accessible from a `HandleValue`.
/// Caller is responsible for throwing a JS exception if needed in case of error.
pub fn native_from_handlevalue<T>(v: HandleValue) -> Result<*const T, ()>
pub fn native_from_handlevalue<#[must_root] T>(v: HandleValue) -> Result<*const T, ()>
where T: DomObject + IDLInterface
{
if !v.get().is_object() {
@@ -453,7 +454,7 @@ pub fn native_from_handlevalue<T>(v: HandleValue) -> Result<*const T, ()>

/// Get a `DomRoot<T>` for a DOM object accessible from a `HandleValue`.
/// Caller is responsible for throwing a JS exception if needed in case of error.
pub fn root_from_handlevalue<T>(v: HandleValue) -> Result<DomRoot<T>, ()>
pub fn root_from_handlevalue<#[must_root] T>(v: HandleValue) -> Result<DomRoot<T>, ()>
where T: DomObject + IDLInterface
{
if !v.get().is_object() {
@@ -463,13 +464,13 @@ pub fn root_from_handlevalue<T>(v: HandleValue) -> Result<DomRoot<T>, ()>
}

/// Get a `DomRoot<T>` for a DOM object accessible from a `HandleObject`.
pub fn root_from_handleobject<T>(obj: HandleObject) -> Result<DomRoot<T>, ()>
pub fn root_from_handleobject<#[must_root] T>(obj: HandleObject) -> Result<DomRoot<T>, ()>
where T: DomObject + IDLInterface
{
root_from_object(obj.get())
}

impl<T: DomObject> ToJSValConvertible for DomRoot<T> {
impl<#[must_root] T: DomObject> ToJSValConvertible for DomRoot<T> {
unsafe fn to_jsval(&self, cx: *mut JSContext, rval: MutableHandleValue) {
self.reflector().to_jsval(cx, rval);
}
@@ -56,7 +56,7 @@ pub struct TrustedReference(*const libc::c_void);
unsafe impl Send for TrustedReference {}

impl TrustedReference {
fn new<T: DomObject>(ptr: *const T) -> TrustedReference {
fn new<#[must_root] T: DomObject>(ptr: *const T) -> TrustedReference {
TrustedReference(ptr as *const libc::c_void)
}
}
@@ -146,17 +146,18 @@ impl TrustedPromise {
/// DOM object is guaranteed to live at least as long as the last outstanding
/// `Trusted<T>` instance.
#[allow_unrooted_interior]
pub struct Trusted<T: DomObject> {
//#[must_root] // TODO hm, allow_unrooted_interior is not checked well enough
pub struct Trusted<#[must_root] T: DomObject> {
/// A pointer to the Rust DOM object of type T, but void to allow
/// sending `Trusted<T>` between threads, regardless of T's sendability.
refcount: Arc<TrustedReference>,
owner_thread: *const libc::c_void,
phantom: PhantomData<T>,
}

unsafe impl<T: DomObject> Send for Trusted<T> {}
unsafe impl<#[must_root] T: DomObject> Send for Trusted<T> {}

impl<T: DomObject> Trusted<T> {
impl<#[must_root] T: DomObject> Trusted<T> {
/// Create a new `Trusted<T>` instance from an existing DOM pointer. The DOM object will
/// be prevented from being GCed for the duration of the resulting `Trusted<T>` object's
/// lifetime.
@@ -188,7 +189,7 @@ impl<T: DomObject> Trusted<T> {
}
}

impl<T: DomObject> Clone for Trusted<T> {
impl<#[must_root] T: DomObject> Clone for Trusted<T> {
fn clone(&self) -> Trusted<T> {
Trusted {
refcount: self.refcount.clone(),
@@ -224,7 +225,7 @@ impl LiveDOMReferences {
table.entry(&*promise).or_insert(vec![]).push(promise)
}

fn addref<T: DomObject>(&self, ptr: *const T) -> Arc<TrustedReference> {
fn addref<#[must_root] T: DomObject>(&self, ptr: *const T) -> Arc<TrustedReference> {
let mut table = self.reflectable_table.borrow_mut();
let capacity = table.capacity();
let len = table.len();
@@ -81,7 +81,7 @@ pub unsafe trait StableTraceObject {
fn stable_trace_object(&self) -> *const JSTraceable;
}

unsafe impl<T> StableTraceObject for Dom<T>
unsafe impl<#[must_root] T> StableTraceObject for Dom<T>
where
T: DomObject,
{
@@ -128,17 +128,17 @@ where
/// A rooted reference to a DOM object.
pub type DomRoot<#[must_root] T> = Root<Dom<T>>;

impl<T: Castable> DomRoot<T> {
impl<#[must_root] T: Castable> DomRoot<T> {
/// Cast a DOM object root upwards to one of the interfaces it derives from.
pub fn upcast<U>(root: DomRoot<T>) -> DomRoot<U>
pub fn upcast<#[must_root] U>(root: DomRoot<T>) -> DomRoot<U>
where U: Castable,
T: DerivedFrom<U>
{
unsafe { mem::transmute(root) }
}

/// Cast a DOM object root downwards to one of the interfaces it might implement.
pub fn downcast<U>(root: DomRoot<T>) -> Option<DomRoot<U>>
pub fn downcast<#[must_root] U>(root: DomRoot<T>) -> Option<DomRoot<U>>
where U: DerivedFrom<T>
{
if root.is::<U>() {
@@ -149,14 +149,14 @@ impl<T: Castable> DomRoot<T> {
}
}

impl<T: DomObject> DomRoot<T> {
impl<#[must_root] T: DomObject> DomRoot<T> {
/// Generate a new root from a reference
pub fn from_ref(unrooted: &T) -> DomRoot<T> {
unsafe { DomRoot::new(Dom::from_ref(unrooted)) }
}
}

impl<T> MallocSizeOf for DomRoot<T>
impl<#[must_root] T> MallocSizeOf for DomRoot<T>
where
T: DomObject + MallocSizeOf,
{
@@ -165,7 +165,7 @@ where
}
}

impl<T> PartialEq for DomRoot<T>
impl<#[must_root] T> PartialEq for DomRoot<T>
where
T: DomObject,
{
@@ -174,7 +174,7 @@ where
}
}

impl<T> Clone for DomRoot<T>
impl<#[must_root] T> Clone for DomRoot<T>
where
T: DomObject,
{
@@ -183,7 +183,7 @@ where
}
}

unsafe impl<T> JSTraceable for DomRoot<T>
unsafe impl<#[must_root] T> JSTraceable for DomRoot<T>
where
T: DomObject,
{
@@ -267,28 +267,28 @@ pub trait RootedReference<'root> {
fn r(&'root self) -> Self::Ref;
}

impl<'root, T: DomObject + 'root> RootedReference<'root> for DomRoot<T> {
impl<'root, #[must_root] T: DomObject + 'root> RootedReference<'root> for DomRoot<T> {
type Ref = &'root T;
fn r(&'root self) -> &'root T {
self
}
}

impl<'root, T: DomObject + 'root> RootedReference<'root> for Dom<T> {
impl<'root, #[must_root] T: DomObject + 'root> RootedReference<'root> for Dom<T> {
type Ref = &'root T;
fn r(&'root self) -> &'root T {
&self
}
}

impl<'root, T: JSTraceable + DomObject + 'root> RootedReference<'root> for [Dom<T>] {
impl<'root, #[must_root] T: JSTraceable + DomObject + 'root> RootedReference<'root> for [Dom<T>] {
type Ref = &'root [&'root T];
fn r(&'root self) -> &'root [&'root T] {
unsafe { mem::transmute(self) }
}
}

impl<'root, T: DomObject + 'root> RootedReference<'root> for Rc<T> {
impl<'root, #[must_root] T: DomObject + 'root> RootedReference<'root> for Rc<T> {
type Ref = &'root T;
fn r(&'root self) -> &'root T {
self
@@ -343,7 +343,7 @@ impl<#[must_root] T: DomObject> Dom<T> {
}
}

impl<T: DomObject> Deref for Dom<T> {
impl<#[must_root] T: DomObject> Deref for Dom<T> {
type Target = T;

fn deref(&self) -> &T {
@@ -354,7 +354,7 @@ impl<T: DomObject> Deref for Dom<T> {
}
}

unsafe impl<T: DomObject> JSTraceable for Dom<T> {
unsafe impl<#[must_root] T: DomObject> JSTraceable for Dom<T> {
unsafe fn trace(&self, trc: *mut JSTracer) {
#[cfg(all(feature = "unstable", debug_assertions))]
let trace_str = format!("for {} on heap", ::std::intrinsics::type_name::<T>());
@@ -407,7 +407,7 @@ impl<T: Castable> LayoutDom<T> {
}
}

impl<T: DomObject> LayoutDom<T> {
impl<#[must_root] T: DomObject> LayoutDom<T> {
/// Get the reflector.
pub unsafe fn get_jsobject(&self) -> *mut JSObject {
debug_assert!(thread_state::get().is_layout());
@@ -485,11 +485,11 @@ impl LayoutDom<Node> {
/// on `Dom<T>`.
#[must_root]
#[derive(JSTraceable)]
pub struct MutDom<T: DomObject> {
pub struct MutDom<#[must_root] T: DomObject> {
val: UnsafeCell<Dom<T>>,
}

impl<T: DomObject> MutDom<T> {
impl<#[must_root] T: DomObject> MutDom<T> {
/// Create a new `MutDom`.
pub fn new(initial: &T) -> MutDom<T> {
debug_assert!(thread_state::get().is_script());
@@ -515,22 +515,22 @@ impl<T: DomObject> MutDom<T> {
}
}

impl<T: DomObject> MallocSizeOf for MutDom<T> {
impl<#[must_root] T: DomObject> MallocSizeOf for MutDom<T> {
fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
// See comment on MallocSizeOf for Dom<T>.
0
}
}

impl<T: DomObject> PartialEq for MutDom<T> {
impl<#[must_root] T: DomObject> PartialEq for MutDom<T> {
fn eq(&self, other: &Self) -> bool {
unsafe {
*self.val.get() == *other.val.get()
}
}
}

impl<T: DomObject + PartialEq> PartialEq<T> for MutDom<T> {
impl<#[must_root] T: DomObject + PartialEq> PartialEq<T> for MutDom<T> {
fn eq(&self, other: &T) -> bool {
unsafe {
**self.val.get() == *other
@@ -648,11 +648,11 @@ impl<#[must_root] T: DomObject> MallocSizeOf for MutNullableDom<T> {
/// This should only be used as a field in other DOM objects; see warning
/// on `Dom<T>`.
#[must_root]
pub struct DomOnceCell<T: DomObject> {
pub struct DomOnceCell<#[must_root] T: DomObject> {
ptr: OnceCell<Dom<T>>,
}

impl<T> DomOnceCell<T>
impl<#[must_root] T> DomOnceCell<T>
where
T: DomObject
{
@@ -667,7 +667,7 @@ where
}
}

impl<T: DomObject> Default for DomOnceCell<T> {
impl<#[must_root] T: DomObject> Default for DomOnceCell<T> {
#[allow(unrooted_must_root)]
fn default() -> DomOnceCell<T> {
debug_assert!(thread_state::get().is_script());
@@ -677,23 +677,23 @@ impl<T: DomObject> Default for DomOnceCell<T> {
}
}

impl<T: DomObject> MallocSizeOf for DomOnceCell<T> {
impl<#[must_root] T: DomObject> MallocSizeOf for DomOnceCell<T> {
fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
// See comment on MallocSizeOf for Dom<T>.
0
}
}

#[allow(unrooted_must_root)]
unsafe impl<T: DomObject> JSTraceable for DomOnceCell<T> {
#[allow(unrooted_must_root)] // TODO mark must_root?
unsafe impl<#[must_root] T: DomObject> JSTraceable for DomOnceCell<T> {
unsafe fn trace(&self, trc: *mut JSTracer) {
if let Some(ptr) = self.ptr.as_ref() {
ptr.trace(trc);
}
}
}

impl<T: DomObject> LayoutDom<T> {
impl<#[must_root] T: DomObject> LayoutDom<T> {
/// Returns an unsafe pointer to the interior of this JS object. This is
/// the only method that be safely accessed from layout. (The fact that
/// this is unsafe is what necessitates the layout wrappers.)
@@ -172,7 +172,7 @@ impl GlobalScope {
/// Returns the global scope of the realm that the given DOM object's reflector
/// was created in.
#[allow(unsafe_code)]
pub fn from_reflector<T: DomObject>(reflector: &T) -> DomRoot<Self> {
pub fn from_reflector<#[must_root] T: DomObject>(reflector: &T) -> DomRoot<Self> {
unsafe { GlobalScope::from_object(*reflector.reflector().get_jsobject()) }
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.