#14902 Write a MIR lint for rooting analysis

components/script_plugins/unrooted_must_root.rs

@@ -8,7 +8,7 @@ use rustc::hir::map as ast_map;
 use rustc::lint::{LateContext, LintPass, LintArray, LateLintPass, LintContext};
 use rustc::ty;
 use syntax::{ast, codemap};
-use utils::{match_def_path, in_derive_expn};
+use utils::match_def_path;
 
 declare_lint!(UNROOTED_MUST_ROOT, Deny,
               "Warn and report usage of unrooted jsmanaged objects");
@@ -123,13 +123,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnrootedPass {
             }
         }
     }
+
     /// Function arguments that are #[must_root] types are not allowed
     fn check_fn(&mut self,
                 cx: &LateContext<'a, 'tcx>,
                 kind: visit::FnKind,
-                decl: &'tcx hir::FnDecl,
-                body: &'tcx hir::Body,
-                span: codemap::Span,
+                _decl: &'tcx hir::FnDecl,
+                _body: &'tcx hir::Body,
+                _span: codemap::Span,
                 id: ast::NodeId) {
         let in_new_function = match kind {
             visit::FnKind::ItemFn(n, _, _, _, _, _, _) |
@@ -139,97 +140,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnrootedPass {
             visit::FnKind::Closure(_) => return,
         };
 
-        if !in_derive_expn(span) {
-            let def_id = cx.tcx.hir.local_def_id(id);
-            let sig = cx.tcx.type_of(def_id).fn_sig(cx.tcx);
-
-            for (arg, ty) in decl.inputs.iter().zip(sig.inputs().0.iter()) {
-                if is_unrooted_ty(cx, ty, false) {
-                    cx.span_lint(UNROOTED_MUST_ROOT, arg.span, "Type must be rooted")
+        let def_id = cx.tcx.hir.local_def_id(id);
+        let mir = cx.tcx.optimized_mir(def_id);
+        for (i, decl) in mir.local_decls.iter().enumerate() {
+            match i {
+                0 => if !in_new_function && is_unrooted_ty(cx, decl.ty, false) {
+                    cx.span_lint(UNROOTED_MUST_ROOT, decl.source_info.span, "Function return type must be rooted.")
                 }
-            }
-
-            if !in_new_function {
-                if is_unrooted_ty(cx, sig.output().0, false) {
-                    cx.span_lint(UNROOTED_MUST_ROOT, decl.output.span(), "Type must be rooted")
+                _ if i <= mir.arg_count => if is_unrooted_ty(cx, decl.ty, false) {
+                    cx.span_lint(UNROOTED_MUST_ROOT, decl.source_info.span, "Function argument type must be rooted.")
                 }
-            }
-        }
-
-        let mut visitor = FnDefVisitor {
-            cx: cx,
-            in_new_function: in_new_function,
-        };
-        visit::walk_expr(&mut visitor, &body.value);
-    }
-}
-
-struct FnDefVisitor<'a, 'b: 'a, 'tcx: 'a + 'b> {
-    cx: &'a LateContext<'b, 'tcx>,
-    in_new_function: bool,
-}
-
-impl<'a, 'b, 'tcx> visit::Visitor<'tcx> for FnDefVisitor<'a, 'b, 'tcx> {
-    fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
-        let cx = self.cx;
-
-        fn require_rooted(cx: &LateContext, in_new_function: bool, subexpr: &hir::Expr) {
-            let ty = cx.tables.expr_ty(&subexpr);
-            if is_unrooted_ty(cx, ty, in_new_function) {
-                cx.span_lint(UNROOTED_MUST_ROOT,
-                             subexpr.span,
-                             &format!("Expression of type {:?} must be rooted", ty))
-            }
-        }
-
-        match expr.node {
-            // Trait casts from #[must_root] types are not allowed
-            hir::ExprCast(ref subexpr, _) => require_rooted(cx, self.in_new_function, &*subexpr),
-            // This catches assignments... the main point of this would be to catch mutable
-            // references to `JS<T>`.
-            // FIXME: Enable this? Triggers on certain kinds of uses of DomRefCell.
-            // hir::ExprAssign(_, ref rhs) => require_rooted(cx, self.in_new_function, &*rhs),
-            // This catches calls; basically, this enforces the constraint that only constructors
-            // can call other constructors.
-            // FIXME: Enable this? Currently triggers with constructs involving DomRefCell, and
-            // constructs like Vec<JS<T>> and RootedVec<JS<T>>.
-            // hir::ExprCall(..) if !self.in_new_function => {
-            //     require_rooted(cx, self.in_new_function, expr);
-            // }
-            _ => {
-                // TODO(pcwalton): Check generics with a whitelist of allowed generics.
-            }
-        }
-
-        visit::walk_expr(self, expr);
-    }
-
-    fn visit_pat(&mut self, pat: &'tcx hir::Pat) {
-        let cx = self.cx;
-
-        // We want to detect pattern bindings that move a value onto the stack.
-        // When "default binding modes" https://github.com/rust-lang/rust/issues/42640
-        // are implemented, the `Unannotated` case could cause false-positives.
-        // These should be fixable by adding an explicit `ref`.
-        match pat.node {
-            hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, _, _) |
-            hir::PatKind::Binding(hir::BindingAnnotation::Mutable, _, _, _) => {
-                let ty = cx.tables.pat_ty(pat);
-                if is_unrooted_ty(cx, ty, self.in_new_function) {
-                    cx.span_lint(UNROOTED_MUST_ROOT,
-                                pat.span,
-                                &format!("Expression of type {:?} must be rooted", ty))
+                _ => if is_unrooted_ty(cx, decl.ty, in_new_function) {
+                    cx.span_lint(UNROOTED_MUST_ROOT, decl.source_info.span, "Type of binding/expression must be rooted.")
                 }
             }
-            _ => {}
         }
-
-        visit::walk_pat(self, pat);
-    }
-
-    fn visit_ty(&mut self, _: &'tcx hir::Ty) {}
-
-    fn nested_visit_map<'this>(&'this mut self) -> hir::intravisit::NestedVisitorMap<'this, 'tcx> {
-        hir::intravisit::NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir)
     }
 }

components/script_plugins/utils.rs

@@ -4,7 +4,6 @@
 
 use rustc::hir::def_id::DefId;
 use rustc::lint::LateContext;
-use syntax::codemap::{ExpnFormat, Span};
 
 /// check if a DefId's path matches the given absolute type path
 /// usage e.g. with
@@ -27,15 +26,3 @@ pub fn match_def_path(cx: &LateContext, def_id: DefId, path: &[&str]) -> bool {
          .zip(path)
          .all(|(nm, p)| &*nm.as_interned_str() == *p)
 }
-
-pub fn in_derive_expn(span: Span) -> bool {
-    if let Some(i) = span.ctxt().outer().expn_info() {
-        if let ExpnFormat::MacroAttribute(n) = i.callee.format {
-            n.as_str().contains("derive")
-        } else {
-            false
-        }
-    } else {
-        false
-    }
-}

tests/unit/script_plugins/lib.rs

@@ -11,8 +11,29 @@ pub mod unrooted_must_root {
     #[must_root] struct Foo(i32);
     #[must_root] struct Bar(Foo);
 
+    impl Foo {
+        fn new() -> Box<Foo> {
+            Box::new(Foo(0))
+        }
+
+        fn new_with(x: i32) -> Foo {
+            Foo(x)
+        }
+    }
+
+    // MIR check gives this errors:
+    // ---- lib.rs - unrooted_must_root::ok (line 6) stdout ----
+    //         error: Type of binding/expression must be rooted.
+    // --> lib.rs:15:18
+    //    |
+    // 10 |         Box::new(Foo(0))
+    //    |                  ^^^^^^
+    //    |
+    //    = note: #[deny(unrooted_must_root)] on by default
+
     fn foo1(_: &Foo) {}
     fn foo2(_: &()) -> &Foo { unimplemented!() }
+    fn new_hack() -> Foo { Foo::new_with(0) }
 
     fn main() {}
     ```
@@ -60,4 +81,48 @@ pub mod unrooted_must_root {
     */
     pub fn return_type() {}
 
+    /**
+    ```compile_fail
+    #![feature(plugin)]
+    #![plugin(script_plugins)]
+
+    #[must_root] struct Foo(i32);
+
+    fn foo(x: &Foo) -> i32 {
+        let y = Foo(x.0 + 3);
+        y.0
+    }
+
+    fn main() {}
+    ```
+    */
+    pub fn expression() {}
+
+    /**
+    ```compile_fail
+    #![feature(plugin)]
+    #![plugin(script_plugins)]
+
+    #[must_root] struct Foo(i32);
+    #[allow(unrooted_must_root)] struct Bar(Foo);
+
+    fn foo(bar: Bar) -> bool {
+        match bar {
+            Bar(f @ Foo(_)) => f.0 == 4,
+            _ => false,
+        }
+    }
+
+    fn main() {}
+    ```
+    */
+    pub fn pattern_binding() {}
+
+    // TODO how to test against:
+    // match expr.node {
+    //     // Trait casts from #[must_root] types are not allowed
+    //     hir::ExprCast(ref subexpr, _) => require_rooted(cx, self.in_new_function, &*subexpr),
+    // 'as' is for casting between primitive types
+
+    // TODO do we want and how to test against Boxing types that are #[must_root]
 }