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
This commit is contained in:
Manish Goregaokar 2014-09-16 01:28:36 +05:30
parent 13ae369dec
commit 12dc54d238
22 changed files with 101 additions and 4 deletions

View file

@ -16,18 +16,23 @@ extern crate rustc;
extern crate sync; extern crate sync;
use syntax::ast; use syntax::ast;
use syntax::attr::AttrMetaMethods;
use rustc::lint::{Context, LintPass, LintPassObject, LintArray}; use rustc::lint::{Context, LintPass, LintPassObject, LintArray};
use rustc::plugin::Registry; use rustc::plugin::Registry;
use rustc::middle::ty::expr_ty; use rustc::middle::ty::expr_ty;
use rustc::middle::{ty, def};
use rustc::middle::typeck::astconv::AstConv; use rustc::middle::typeck::astconv::AstConv;
use rustc::util::ppaux::Repr; use rustc::util::ppaux::Repr;
declare_lint!(TRANSMUTE_TYPE_LINT, Allow, declare_lint!(TRANSMUTE_TYPE_LINT, Allow,
"Warn and report types being transmuted") "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 { fn get_lints(&self) -> LintArray {
lint_array!(TRANSMUTE_TYPE_LINT) 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] #[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) { 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] #[macro_export]

View file

@ -71,6 +71,7 @@ impl Str for AttrValue {
} }
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct Attr { pub struct Attr {
reflector_: Reflector, reflector_: Reflector,
local_name: Atom, local_name: Atom,

View file

@ -2868,7 +2868,10 @@ class CGUnionStruct(CGThing):
enumConversions = [ enumConversions = [
" e%s(ref inner) => inner.to_jsval(cx)," % v["name"] for v in templateVars " 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 %s
} }

View file

@ -34,6 +34,7 @@ pub enum GlobalRoot<'a, 'b> {
/// A traced reference to a global object, for use in fields of traced Rust /// A traced reference to a global object, for use in fields of traced Rust
/// structures. /// structures.
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub enum GlobalField { pub enum GlobalField {
WindowField(JS<Window>), WindowField(JS<Window>),
WorkerField(JS<WorkerGlobalScope>), WorkerField(JS<WorkerGlobalScope>),

View file

@ -61,6 +61,7 @@ use std::mem;
/// Importantly, it requires explicit rooting in order to interact with the inner value. /// 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 /// 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). /// `JS<T>::assign` method or `OptionalSettable::assign` (for `Option<JS<T>>` fields).
#[allow(unrooted_must_root)]
pub struct Temporary<T> { pub struct Temporary<T> {
inner: JS<T>, inner: JS<T>,
/// On-stack JS pointer to assuage conservative stack scanner /// 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. /// A rooted, JS-owned value. Must only be used as a field in other JS-owned types.
#[must_root]
pub struct JS<T> { pub struct JS<T> {
ptr: *const T ptr: *const T
} }

View file

@ -67,6 +67,7 @@ impl BrowserContext {
} }
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct SessionHistoryEntry { pub struct SessionHistoryEntry {
document: JS<Document>, document: JS<Document>,
children: Vec<BrowserContext> children: Vec<BrowserContext>

View file

@ -17,6 +17,7 @@ use geom::size::Size2D;
use canvas::canvas_render_task::{CanvasMsg, CanvasRenderTask, ClearRect, Close, FillRect, Recreate, StrokeRect}; use canvas::canvas_render_task::{CanvasMsg, CanvasRenderTask, ClearRect, Close, FillRect, Recreate, StrokeRect};
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct CanvasRenderingContext2D { pub struct CanvasRenderingContext2D {
reflector_: Reflector, reflector_: Reflector,
global: GlobalField, global: GlobalField,

View file

@ -69,6 +69,7 @@ pub enum IsHTMLDocument {
} }
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct Document { pub struct Document {
pub node: Node, pub node: Node,
reflector_: Reflector, reflector_: Reflector,

View file

@ -23,6 +23,7 @@ use dom::text::Text;
use servo_util::str::DOMString; use servo_util::str::DOMString;
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct DOMImplementation { pub struct DOMImplementation {
document: JS<Document>, document: JS<Document>,
reflector_: Reflector, reflector_: Reflector,

View file

@ -14,6 +14,7 @@ use dom::window::Window;
use servo_util::str::DOMString; use servo_util::str::DOMString;
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct DOMParser { pub struct DOMParser {
window: JS<Window>, //XXXjdm Document instead? window: JS<Window>, //XXXjdm Document instead?
reflector_: Reflector reflector_: Reflector

View file

@ -11,6 +11,7 @@ use dom::domrect::DOMRect;
use dom::window::Window; use dom::window::Window;
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct DOMRectList { pub struct DOMRectList {
reflector_: Reflector, reflector_: Reflector,
rects: Vec<JS<DOMRect>>, rects: Vec<JS<DOMRect>>,

View file

@ -17,6 +17,7 @@ use servo_util::namespace::Null;
use servo_util::str::{DOMString, HTML_SPACE_CHARACTERS}; use servo_util::str::{DOMString, HTML_SPACE_CHARACTERS};
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct DOMTokenList { pub struct DOMTokenList {
reflector_: Reflector, reflector_: Reflector,
element: JS<Element>, element: JS<Element>,

View file

@ -19,12 +19,14 @@ use std::cell::RefCell;
use std::collections::hashmap::HashMap; use std::collections::hashmap::HashMap;
#[deriving(Encodable, Clone)] #[deriving(Encodable, Clone)]
#[must_root]
pub enum FormDatum { pub enum FormDatum {
StringData(DOMString), StringData(DOMString),
FileData(JS<File>) FileData(JS<File>)
} }
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct FormData { pub struct FormData {
data: Traceable<RefCell<HashMap<DOMString, Vec<FormDatum>>>>, data: Traceable<RefCell<HashMap<DOMString, Vec<FormDatum>>>>,
reflector_: Reflector, reflector_: Reflector,

View file

@ -30,12 +30,14 @@ impl<S: Encoder<E>, E> Encodable<S, E> for Box<CollectionFilter> {
} }
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub enum CollectionTypeId { pub enum CollectionTypeId {
Static(Vec<JS<Element>>), Static(Vec<JS<Element>>),
Live(JS<Node>, Box<CollectionFilter>) Live(JS<Node>, Box<CollectionFilter>)
} }
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct HTMLCollection { pub struct HTMLCollection {
collection: CollectionTypeId, collection: CollectionTypeId,
reflector_: Reflector, reflector_: Reflector,

View file

@ -12,6 +12,7 @@ use dom::element::Element;
use dom::window::Window; use dom::window::Window;
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct NamedNodeMap { pub struct NamedNodeMap {
reflector_: Reflector, reflector_: Reflector,
owner: JS<Element>, owner: JS<Element>,

View file

@ -850,6 +850,7 @@ impl<'a> Iterator<JSRef<'a, Node>> for TreeIterator<'a> {
} }
} }
#[must_root]
pub struct NodeIterator { pub struct NodeIterator {
pub start_node: JS<Node>, pub start_node: JS<Node>,
pub current_node: Option<JS<Node>>, pub current_node: Option<JS<Node>>,

View file

@ -11,18 +11,21 @@ use dom::node::{Node, NodeHelpers};
use dom::window::Window; use dom::window::Window;
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub enum NodeListType { pub enum NodeListType {
Simple(Vec<JS<Node>>), Simple(Vec<JS<Node>>),
Children(JS<Node>) Children(JS<Node>)
} }
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct NodeList { pub struct NodeList {
list_type: NodeListType, list_type: NodeListType,
reflector_: Reflector, reflector_: Reflector,
} }
impl NodeList { impl NodeList {
#[allow(unrooted_must_root)]
pub fn new_inherited(list_type: NodeListType) -> NodeList { pub fn new_inherited(list_type: NodeListType) -> NodeList {
NodeList { NodeList {
list_type: list_type, list_type: list_type,
@ -30,6 +33,7 @@ impl NodeList {
} }
} }
#[allow(unrooted_must_root)]
pub fn new(window: &JSRef<Window>, pub fn new(window: &JSRef<Window>,
list_type: NodeListType) -> Temporary<NodeList> { list_type: NodeListType) -> Temporary<NodeList> {
reflect_dom_object(box NodeList::new_inherited(list_type), reflect_dom_object(box NodeList::new_inherited(list_type),

View file

@ -14,6 +14,7 @@ use time;
pub type DOMHighResTimeStamp = f64; pub type DOMHighResTimeStamp = f64;
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct Performance { pub struct Performance {
reflector_: Reflector, reflector_: Reflector,
timing: JS<PerformanceTiming>, timing: JS<PerformanceTiming>,

View file

@ -19,6 +19,7 @@ use js::jsapi::JSContext;
use js::jsval::{JSVal, NullValue}; use js::jsval::{JSVal, NullValue};
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct TestBinding { pub struct TestBinding {
reflector: Reflector, reflector: Reflector,
global: GlobalField, global: GlobalField,

View file

@ -30,6 +30,7 @@ use std::ptr;
pub struct TrustedWorkerAddress(pub *const c_void); pub struct TrustedWorkerAddress(pub *const c_void);
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct Worker { pub struct Worker {
eventtarget: EventTarget, eventtarget: EventTarget,
refcount: Cell<uint>, refcount: Cell<uint>,

View file

@ -98,6 +98,7 @@ enum SyncOrAsync<'a, 'b> {
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct XMLHttpRequest { pub struct XMLHttpRequest {
eventtarget: XMLHttpRequestEventTarget, eventtarget: XMLHttpRequestEventTarget,
ready_state: Traceable<Cell<XMLHttpRequestState>>, ready_state: Traceable<Cell<XMLHttpRequestState>>,

View file

@ -425,6 +425,7 @@ impl Page {
/// Information for one frame in the browsing context. /// Information for one frame in the browsing context.
#[deriving(Encodable)] #[deriving(Encodable)]
#[must_root]
pub struct Frame { pub struct Frame {
/// The document for this frame. /// The document for this frame.
pub document: JS<Document>, pub document: JS<Document>,