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

Write a MIR lint for rooting analysis #14902

Open
jdm opened this issue Jan 6, 2017 · 14 comments
Open

Write a MIR lint for rooting analysis #14902

jdm opened this issue Jan 6, 2017 · 14 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Jan 6, 2017

From the work week discussion about rooting safety, one problem that was discussed was dealing with generic types. It was proposed that a MIR lint would be safer than the current lints, because MIR lints run on monomorphized types. My understanding was that @Manishearth and @nikomatsakis were interested in making this happen.

@jdm
Copy link
Member Author

@jdm jdm commented Jan 6, 2017

@Manishearth tells me that he is too busy to work on this. I would settle for any information about how to write a MIR lint so that someone else could theoretically do this work.

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Jan 6, 2017

I've never done it before. You need to register a MIR pass. You create MIR passes by implementing MirPass (MirMapPass is auto magically implemented)

You probably want a MIR visitor (http://manishearth.github.io/rust-internals-docs/rustc/mir/visit/trait.Visitor.html) for walking the MIR.

@jdm jdm added the A-plugins label Jan 6, 2017
@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Jan 9, 2017

I started doing it and didn't get very far. It doesn't seem very hard to do though. @jdm do you plan to pursue it?

@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Jan 9, 2017

Maybe we can chat about it on IRC :)

@nox
Copy link
Member

@nox nox commented Oct 7, 2017

@nikomatsakis Think you could push what you did somewhere?

@mrowqa
Copy link

@mrowqa mrowqa commented Mar 1, 2018

Hi. As we talked before, I will work on it as a part of Bachelor's thesis. I plan to start working on it quite soon (probably tomorrow), so I'd like to know if there are some good resources to learn about rust compiler plugins. Furthermore, is there anything else I should know? I know the general idea of handles and rooting.
CC @jdm

@jdm
Copy link
Member Author

@jdm jdm commented Mar 2, 2018

You can see the existing HIR plugin at https://github.com/servo/servo/blob/master/components/script_plugins/unrooted_must_root.rs. There are a few unit tests and it might be good to add some more as part of this work.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 2, 2018

You can find documentation for the compiler internals at http://manishearth.github.io/rust-internals-docs/rustc_plugin/, and https://doc.rust-lang.org/beta/unstable-book/language-features/plugin.html may have some useful information.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 2, 2018

https://rust-lang-nursery.github.io/rustc-guide/ may also have information about the compiler internals that the plugin uses.

@mrowqa
Copy link

@mrowqa mrowqa commented Mar 6, 2018

I read those materials and some other stuff I have found, so I understand the general idea of MIR. I tried to write some minimal plugin, but actually I don't know how exactly. rustc_plugin::registry::Registry doesn't have register_mir_pass anymore. I tried to hunt down some type that implements MirPass and found out that only internal optimized_mir method run these passes. The problem is, it runs hardcoded list of passes and it seems like they don't want anything else to be plugged in.

Also, the docs linked here (at manishearth.github.io) are a little bit out of date. I mean they show that rustc::mir has submodule transform, but it seems to be moved to rustc_mir crate.

Here is something what I tried:

diff --git a/components/script_plugins/lib.rs b/components/script_plugins/lib.rs
index 3f256fd..e10ab16 100644
--- a/components/script_plugins/lib.rs
+++ b/components/script_plugins/lib.rs
@@ -26,6 +26,7 @@
 extern crate rustc;

 extern crate rustc_plugin;
+extern crate rustc_mir;
 extern crate syntax;

 use rustc_plugin::Registry;
@@ -42,6 +43,7 @@ mod utils;
 pub fn plugin_registrar(reg: &mut Registry) {
     #[cfg(feature = "unrooted_must_root_lint")]
     reg.register_late_lint_pass(Box::new(unrooted_must_root::UnrootedPass::new()));
+    //reg.register_mir_pass(Box::new(unrooted_must_root::UnrootedPassX::new()));

     reg.register_attribute("allow_unrooted_interior".to_string(), Whitelisted);
     reg.register_attribute("must_root".to_string(), Whitelisted);
diff --git a/components/script_plugins/unrooted_must_root.rs b/components/script_plugins/unrooted_must_root.rs
index 502ae03..e20f4fb 100644
--- a/components/script_plugins/unrooted_must_root.rs
+++ b/components/script_plugins/unrooted_must_root.rs
@@ -6,13 +6,18 @@ use rustc::hir;
 use rustc::hir::intravisit as visit;
 use rustc::hir::map as ast_map;
 use rustc::lint::{LateContext, LintPass, LintArray, LateLintPass, LintContext};
+use rustc::mir::Mir;
 use rustc::ty;
+use rustc_mir::transform::{MirPass, MirSource};
 use syntax::{ast, codemap};
 use utils::{match_def_path, in_derive_expn};

 declare_lint!(UNROOTED_MUST_ROOT, Deny,
               "Warn and report usage of unrooted jsmanaged objects");

+declare_lint!(UNROOTED_MUST_ROOT_X, Deny,
+              "Warn and report usage of unrooted jsmanaged objects");
+
 /// Lint for ensuring safe usage of unrooted pointers
 ///
 /// This lint (disable with `-A unrooted-must-root`/`#[allow(unrooted_must_root)]`) ensures that `#[must_root]`
@@ -31,6 +36,7 @@ declare_lint!(UNROOTED_MUST_ROOT, Deny,
 /// can be marked as `#[allow(unrooted_must_root)]`. Smart pointers which root their interior type
 /// can be marked as `#[allow_unrooted_interior]`
 pub struct UnrootedPass;
+pub struct UnrootedPassX;

 impl UnrootedPass {
     pub fn new() -> UnrootedPass {
@@ -38,6 +44,30 @@ impl UnrootedPass {
     }
 }

+impl UnrootedPassX {
+    pub fn new() -> UnrootedPassX {
+        UnrootedPassX
+    }
+}
+
+impl MirPass for UnrootedPassX {
+    fn run_pass<'a, 'tcx>(
+        &self,
+        tcx: ty::TyCtxt<'a, 'tcx, 'tcx>,
+        source: MirSource,
+        mir: &mut Mir<'tcx>
+    ) {
+
+    }
+}
+
+impl LintPass for UnrootedPassX {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(UNROOTED_MUST_ROOT_X)
+    }
+}
+
+
 /// Checks if a type is unrooted or contains any owned unrooted types
 fn is_unrooted_ty(cx: &LateContext, ty: &ty::TyS, in_new_function: bool) -> bool {
     let mut ret = false;
@jdm
Copy link
Member Author

@jdm jdm commented Mar 6, 2018

@nikomatsakis Where are the most up to date docs for rustc?

@jdm
Copy link
Member Author

@jdm jdm commented Mar 6, 2018

rust-lang/rust#40239 may have made this impossible.

mrowqa added a commit to mrowqa/servo that referenced this issue Mar 10, 2018
@mrowqa mrowqa mentioned this issue Mar 10, 2018
0 of 3 tasks complete
@mrowqa mrowqa mentioned this issue Apr 8, 2018
3 of 3 tasks complete
@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Apr 9, 2018

#20474 has landed and includes an update to nightly-2018-04-08.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants
You can’t perform that action at this time.