From 12dc54d23849f6bc90667e2df432e30587e4af49 Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manishsmail@gmail.com>
Date: Tue, 16 Sep 2014 01:28:36 +0530
Subject: [PATCH] Add unrooted_must_root lint for enums and structs containing
 JS<T>, as well as functions with JS<T> in their parameter list

For safe wrappers over JS<T> (eg Temporary<T>) use #[allow(unrooted_must_root)].
For all other types containing a #[must_root] value, annotate the type with #[must_root] to ensure that it is never used unrooted
---
 components/macros/lib.rs                      | 74 ++++++++++++++++++-
 components/script/dom/attr.rs                 |  1 +
 .../dom/bindings/codegen/CodegenRust.py       |  5 +-
 components/script/dom/bindings/global.rs      |  1 +
 components/script/dom/bindings/js.rs          |  2 +
 components/script/dom/browsercontext.rs       |  1 +
 .../script/dom/canvasrenderingcontext2d.rs    |  1 +
 components/script/dom/document.rs             |  1 +
 components/script/dom/domimplementation.rs    |  1 +
 components/script/dom/domparser.rs            |  1 +
 components/script/dom/domrectlist.rs          |  1 +
 components/script/dom/domtokenlist.rs         |  1 +
 components/script/dom/formdata.rs             |  2 +
 components/script/dom/htmlcollection.rs       |  2 +
 components/script/dom/namednodemap.rs         |  1 +
 components/script/dom/node.rs                 |  1 +
 components/script/dom/nodelist.rs             |  4 +
 components/script/dom/performance.rs          |  1 +
 components/script/dom/testbinding.rs          |  1 +
 components/script/dom/worker.rs               |  1 +
 components/script/dom/xmlhttprequest.rs       |  1 +
 components/script/page.rs                     |  1 +
 22 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/components/macros/lib.rs b/components/macros/lib.rs
index 554d9833c98..9191af72ce8 100644
--- a/components/macros/lib.rs
+++ b/components/macros/lib.rs
@@ -16,18 +16,23 @@ extern crate rustc;
 extern crate sync;
 
 use syntax::ast;
+use syntax::attr::AttrMetaMethods;
 use rustc::lint::{Context, LintPass, LintPassObject, LintArray};
 use rustc::plugin::Registry;
 use rustc::middle::ty::expr_ty;
+use rustc::middle::{ty, def};
 use rustc::middle::typeck::astconv::AstConv;
 use rustc::util::ppaux::Repr;
 
 declare_lint!(TRANSMUTE_TYPE_LINT, Allow,
               "Warn and report types being transmuted")
+declare_lint!(UNROOTED_MUST_ROOT, Deny,
+              "Warn and report usage of unrooted jsmanaged objects")
 
-struct Pass;
+struct TransmutePass;
+struct UnrootedPass;
 
-impl LintPass for Pass {
+impl LintPass for TransmutePass {
     fn get_lints(&self) -> LintArray {
         lint_array!(TRANSMUTE_TYPE_LINT)
     }
@@ -56,9 +61,72 @@ impl LintPass for Pass {
     }
 }
 
+fn lint_unrooted_ty(cx: &Context, ty: &ast::Ty, warning: &str) {
+        match ty.node {
+            ast::TyPath(_, _, id) => {
+                match cx.tcx.def_map.borrow().get_copy(&id) {
+                    def::DefTy(def_id) => {
+                        if ty::has_attr(cx.tcx, def_id, "must_root") {
+                            cx.span_lint(UNROOTED_MUST_ROOT, ty.span, warning);
+                        }
+                    }
+                    _ => (),
+                }
+            }
+            _ => (),
+    }
+}
+
+impl LintPass for UnrootedPass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(UNROOTED_MUST_ROOT)
+    }
+
+    fn check_struct_def(&mut self, cx: &Context, def: &ast::StructDef, _i: ast::Ident, _gen: &ast::Generics, id: ast::NodeId) {
+        if cx.tcx.map.expect_item(id).attrs.iter().all(|a| !a.check_name("must_root")) {
+            for ref field in def.fields.iter() {
+                lint_unrooted_ty(cx, &*field.node.ty, "Type must be rooted, use #[must_root] on the struct definition to propagate");
+            }
+        }
+    }
+
+    fn check_variant(&mut self, cx: &Context, var: &ast::Variant, _gen: &ast::Generics) {
+        let ref map = cx.tcx.map;
+        if map.expect_item(map.get_parent(var.node.id)).attrs.iter().all(|a| !a.check_name("must_root")) {
+            match var.node.kind {
+                ast::TupleVariantKind(ref vec) => {
+                    for ty in vec.iter() {
+                        lint_unrooted_ty(cx, &*ty.ty, "Type must be rooted, use #[must_root] on the enum definition to propagate")
+                    }
+                }
+                _ => () // Struct variants already caught by check_struct_def
+            }
+        }
+    }
+
+    fn check_fn(&mut self, cx: &Context, kind: &syntax::visit::FnKind, decl: &ast::FnDecl, block: &ast::Block, _span: syntax::codemap::Span, _id: ast::NodeId) {
+        match *kind {
+            syntax::visit::FkItemFn(i, _, _, _) |
+            syntax::visit::FkMethod(i, _, _) if i.as_str() == "new" || i.as_str() == "new_inherited" => {
+
+            }
+            _ => ()
+        }
+        match block.rules {
+            ast::DefaultBlock => {
+                for arg in decl.inputs.iter() {
+                    lint_unrooted_ty(cx, &*arg.ty, "Type must be rooted, use #[must_root] on the struct definition to propagate")
+                }
+            }
+            _ => () // fn is `unsafe`
+        }
+    }
+}
+
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
-    reg.register_lint_pass(box Pass as LintPassObject);
+    reg.register_lint_pass(box TransmutePass as LintPassObject);
+    reg.register_lint_pass(box UnrootedPass as LintPassObject);
 }
 
 #[macro_export]
diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs
index 4da59b96b55..a931ba4e5b3 100644
--- a/components/script/dom/attr.rs
+++ b/components/script/dom/attr.rs
@@ -71,6 +71,7 @@ impl Str for AttrValue {
 }
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct Attr {
     reflector_: Reflector,
     local_name: Atom,
diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py
index 1666589940e..4d0f8bdb682 100644
--- a/components/script/dom/bindings/codegen/CodegenRust.py
+++ b/components/script/dom/bindings/codegen/CodegenRust.py
@@ -2868,7 +2868,10 @@ class CGUnionStruct(CGThing):
         enumConversions = [
             "            e%s(ref inner) => inner.to_jsval(cx)," % v["name"] for v in templateVars
         ]
-        return ("""pub enum %s {
+        # XXXManishearth The following should be #[must_root],
+        # however we currently allow it till #2661 is fixed
+        return ("""#[allow(unrooted_must_root)]
+pub enum %s {
 %s
 }
 
diff --git a/components/script/dom/bindings/global.rs b/components/script/dom/bindings/global.rs
index 35b94d0e472..e3b3f9cca1e 100644
--- a/components/script/dom/bindings/global.rs
+++ b/components/script/dom/bindings/global.rs
@@ -34,6 +34,7 @@ pub enum GlobalRoot<'a, 'b> {
 /// A traced reference to a global object, for use in fields of traced Rust
 /// structures.
 #[deriving(Encodable)]
+#[must_root]
 pub enum GlobalField {
     WindowField(JS<Window>),
     WorkerField(JS<WorkerGlobalScope>),
diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs
index ab8b3e3c7f5..c77ae3cba17 100644
--- a/components/script/dom/bindings/js.rs
+++ b/components/script/dom/bindings/js.rs
@@ -61,6 +61,7 @@ use std::mem;
 /// Importantly, it requires explicit rooting in order to interact with the inner value.
 /// Can be assigned into JS-owned member fields (i.e. `JS<T>` types) safely via the
 /// `JS<T>::assign` method or `OptionalSettable::assign` (for `Option<JS<T>>` fields).
+#[allow(unrooted_must_root)]
 pub struct Temporary<T> {
     inner: JS<T>,
     /// On-stack JS pointer to assuage conservative stack scanner
@@ -106,6 +107,7 @@ impl<T: Reflectable> Temporary<T> {
 }
 
 /// A rooted, JS-owned value. Must only be used as a field in other JS-owned types.
+#[must_root]
 pub struct JS<T> {
     ptr: *const T
 }
diff --git a/components/script/dom/browsercontext.rs b/components/script/dom/browsercontext.rs
index a54477a2ff8..b4cbb6be336 100644
--- a/components/script/dom/browsercontext.rs
+++ b/components/script/dom/browsercontext.rs
@@ -67,6 +67,7 @@ impl BrowserContext {
 }
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct SessionHistoryEntry {
     document: JS<Document>,
     children: Vec<BrowserContext>
diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs
index 6b3f898123f..6297a0718d8 100644
--- a/components/script/dom/canvasrenderingcontext2d.rs
+++ b/components/script/dom/canvasrenderingcontext2d.rs
@@ -17,6 +17,7 @@ use geom::size::Size2D;
 use canvas::canvas_render_task::{CanvasMsg, CanvasRenderTask, ClearRect, Close, FillRect, Recreate, StrokeRect};
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct CanvasRenderingContext2D {
     reflector_: Reflector,
     global: GlobalField,
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index 8f8f8e5c607..97fb575fc01 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -69,6 +69,7 @@ pub enum IsHTMLDocument {
 }
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct Document {
     pub node: Node,
     reflector_: Reflector,
diff --git a/components/script/dom/domimplementation.rs b/components/script/dom/domimplementation.rs
index 0dbb842ebe4..36d74258ca7 100644
--- a/components/script/dom/domimplementation.rs
+++ b/components/script/dom/domimplementation.rs
@@ -23,6 +23,7 @@ use dom::text::Text;
 use servo_util::str::DOMString;
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct DOMImplementation {
     document: JS<Document>,
     reflector_: Reflector,
diff --git a/components/script/dom/domparser.rs b/components/script/dom/domparser.rs
index 65273cab756..d8362fa1e80 100644
--- a/components/script/dom/domparser.rs
+++ b/components/script/dom/domparser.rs
@@ -14,6 +14,7 @@ use dom::window::Window;
 use servo_util::str::DOMString;
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct DOMParser {
     window: JS<Window>, //XXXjdm Document instead?
     reflector_: Reflector
diff --git a/components/script/dom/domrectlist.rs b/components/script/dom/domrectlist.rs
index 0c661c4a51d..450e87eda6d 100644
--- a/components/script/dom/domrectlist.rs
+++ b/components/script/dom/domrectlist.rs
@@ -11,6 +11,7 @@ use dom::domrect::DOMRect;
 use dom::window::Window;
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct DOMRectList {
     reflector_: Reflector,
     rects: Vec<JS<DOMRect>>,
diff --git a/components/script/dom/domtokenlist.rs b/components/script/dom/domtokenlist.rs
index 7c1b1fc3365..e57f0310ec5 100644
--- a/components/script/dom/domtokenlist.rs
+++ b/components/script/dom/domtokenlist.rs
@@ -17,6 +17,7 @@ use servo_util::namespace::Null;
 use servo_util::str::{DOMString, HTML_SPACE_CHARACTERS};
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct DOMTokenList {
     reflector_: Reflector,
     element: JS<Element>,
diff --git a/components/script/dom/formdata.rs b/components/script/dom/formdata.rs
index 0de17b83107..3e39a1009aa 100644
--- a/components/script/dom/formdata.rs
+++ b/components/script/dom/formdata.rs
@@ -19,12 +19,14 @@ use std::cell::RefCell;
 use std::collections::hashmap::HashMap;
 
 #[deriving(Encodable, Clone)]
+#[must_root]
 pub enum FormDatum {
     StringData(DOMString),
     FileData(JS<File>)
 }
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct FormData {
     data: Traceable<RefCell<HashMap<DOMString, Vec<FormDatum>>>>,
     reflector_: Reflector,
diff --git a/components/script/dom/htmlcollection.rs b/components/script/dom/htmlcollection.rs
index 5bef6c56ff5..bd07b82e53e 100644
--- a/components/script/dom/htmlcollection.rs
+++ b/components/script/dom/htmlcollection.rs
@@ -30,12 +30,14 @@ impl<S: Encoder<E>, E> Encodable<S, E> for Box<CollectionFilter> {
 }
 
 #[deriving(Encodable)]
+#[must_root]
 pub enum CollectionTypeId {
     Static(Vec<JS<Element>>),
     Live(JS<Node>, Box<CollectionFilter>)
 }
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct HTMLCollection {
     collection: CollectionTypeId,
     reflector_: Reflector,
diff --git a/components/script/dom/namednodemap.rs b/components/script/dom/namednodemap.rs
index d60160133c9..6faf74aa564 100644
--- a/components/script/dom/namednodemap.rs
+++ b/components/script/dom/namednodemap.rs
@@ -12,6 +12,7 @@ use dom::element::Element;
 use dom::window::Window;
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct NamedNodeMap {
     reflector_: Reflector,
     owner: JS<Element>,
diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs
index f419798a110..90dd959e10f 100644
--- a/components/script/dom/node.rs
+++ b/components/script/dom/node.rs
@@ -850,6 +850,7 @@ impl<'a> Iterator<JSRef<'a, Node>> for TreeIterator<'a> {
     }
 }
 
+#[must_root]
 pub struct NodeIterator {
     pub start_node: JS<Node>,
     pub current_node: Option<JS<Node>>,
diff --git a/components/script/dom/nodelist.rs b/components/script/dom/nodelist.rs
index 424eb09416c..0ebd2e0db25 100644
--- a/components/script/dom/nodelist.rs
+++ b/components/script/dom/nodelist.rs
@@ -11,18 +11,21 @@ use dom::node::{Node, NodeHelpers};
 use dom::window::Window;
 
 #[deriving(Encodable)]
+#[must_root]
 pub enum NodeListType {
     Simple(Vec<JS<Node>>),
     Children(JS<Node>)
 }
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct NodeList {
     list_type: NodeListType,
     reflector_: Reflector,
 }
 
 impl NodeList {
+    #[allow(unrooted_must_root)]
     pub fn new_inherited(list_type: NodeListType) -> NodeList {
         NodeList {
             list_type: list_type,
@@ -30,6 +33,7 @@ impl NodeList {
         }
     }
 
+    #[allow(unrooted_must_root)]
     pub fn new(window: &JSRef<Window>,
                list_type: NodeListType) -> Temporary<NodeList> {
         reflect_dom_object(box NodeList::new_inherited(list_type),
diff --git a/components/script/dom/performance.rs b/components/script/dom/performance.rs
index 82b931b0c2e..fc57eaba869 100644
--- a/components/script/dom/performance.rs
+++ b/components/script/dom/performance.rs
@@ -14,6 +14,7 @@ use time;
 pub type DOMHighResTimeStamp = f64;
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct Performance {
     reflector_: Reflector,
     timing: JS<PerformanceTiming>,
diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs
index b5d83fac620..222223d7f44 100644
--- a/components/script/dom/testbinding.rs
+++ b/components/script/dom/testbinding.rs
@@ -19,6 +19,7 @@ use js::jsapi::JSContext;
 use js::jsval::{JSVal, NullValue};
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct TestBinding {
     reflector: Reflector,
     global: GlobalField,
diff --git a/components/script/dom/worker.rs b/components/script/dom/worker.rs
index 3119b4c96f2..a34d47cef9b 100644
--- a/components/script/dom/worker.rs
+++ b/components/script/dom/worker.rs
@@ -30,6 +30,7 @@ use std::ptr;
 pub struct TrustedWorkerAddress(pub *const c_void);
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct Worker {
     eventtarget: EventTarget,
     refcount: Cell<uint>,
diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs
index b2bfd64eb4e..0b48355a3d8 100644
--- a/components/script/dom/xmlhttprequest.rs
+++ b/components/script/dom/xmlhttprequest.rs
@@ -98,6 +98,7 @@ enum SyncOrAsync<'a, 'b> {
 
 
 #[deriving(Encodable)]
+#[must_root]
 pub struct XMLHttpRequest {
     eventtarget: XMLHttpRequestEventTarget,
     ready_state: Traceable<Cell<XMLHttpRequestState>>,
diff --git a/components/script/page.rs b/components/script/page.rs
index cf532783c8b..6e8f414ef8d 100644
--- a/components/script/page.rs
+++ b/components/script/page.rs
@@ -425,6 +425,7 @@ impl Page {
 
 /// Information for one frame in the browsing context.
 #[deriving(Encodable)]
+#[must_root]
 pub struct Frame {
     /// The document for this frame.
     pub document: JS<Document>,