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

Handle generic functions #3

Open
Munksgaard opened this issue Mar 19, 2015 · 6 comments
Open

Handle generic functions #3

Munksgaard opened this issue Mar 19, 2015 · 6 comments
Assignees

Comments

@Munksgaard
Copy link
Collaborator

One thing we don't handle is functions like this:

fn dropit<T>(x: T) { }

Since we just check all functions, but not instantiations of them, we can call dropit on a drop protected var. Any suggestions?

@Manishearth writes:

But we do; we still have allow_drop on the functions, right? If one of the arguments transitively resolves to a protected type (unless behind &, we probably want a utility function for that), and the function isn't annotated with #[allow_drop(Foo)], then it's a case of a generic function being fed a protected type, and we can error.

However, stuff like let mut v = Vec::new(); let c = Chan(); v.push(c); let c_ = v.pop(); c_.close() should be allowed.

@Manishearth
Copy link
Owner

(Ignore what I wrote, I forgot we had changed that bit of the code 😛

@Munksgaard Munksgaard self-assigned this Mar 19, 2015
Munksgaard added a commit that referenced this issue Apr 1, 2015
This shows how we can partially get around the problem with types
defined in foreign crates etc.

In rust-sesssions, we could imagine creating a trait and impl like this:

```
unsafe trait Closeable {
    fn close(self);
}

unsafe impl Closeable for Vec<Chan<Eps>> {
  #[allow_drop="collections::vec::Vec<Chan<Eps>>"]
  fn close(self) { }
}
```

Of course, there's a problem with the environment, as the Chan actually
looks like `Chan<E, Eps>`, where E is something we don't know, and don't
really care about.

TODO: Some kind of pattern matching could perhaps solve this?

cc #3
@Munksgaard
Copy link
Collaborator Author

The code in the fix-generics branch is an attempt at this. However, it still needs to error on stuff like drop.

@Manishearth
Copy link
Owner

\o/

@Munksgaard
Copy link
Collaborator Author

Actually there are several problems here:

  1. We wish to prohibit stuff like let c = channel(); drop(c);
  2. We wish to be able to correctly track stuff in generic containers such as vectors.
  3. We wish to prohibit calling functions such as the above dropit with protected values.

1 and 3 are closely related of course, but the added problem of 1 is that, since drop is in an external crate, we can't actually walk through it.

@Munksgaard
Copy link
Collaborator Author

After some discussion about this, I'm not sure we can currently handle 1 and 2 from above in an exhausstive manner. For example, consider the following function (defined in an external crate from the one humpty is currently checking):

fn foo<T>(v: Vec<T>) -> Vec<T> {
    let mut new_v = Vec::new();
    if let Some(x) = v.into_iter().last() {
        new_v.push(x);
    }
    x
}

Since humpty can only access the meta-data for the function, ie. the argument types and the return type, we have no way of knowing whether or not it is safe to call with a protected value.

My current thinking is that we could simply warn on calls to external functions (that don't have allow_drop), perhaps with the exception of calls to a specific list of functions that we white-list (vec.push, vec.pop comes to mind). Of course, we'd need to manually make sure that the white-listed functions are actually linear with respect to their arguments.

I still think that 3 from above can be addressed, but we'd probably have to invoke check_fn recursively (with some kind of state tracking to avoid overflow) with the type parameters instantialized to the protected types we call it with.

cc @laumann

@Manishearth
Copy link
Owner

My current thinking is that we could simply warn on calls to external functions

This makes sense.

We can just change the ExprCall/ExprMethodCall handling to worry about situations where:

  • The function is externally defined
  • The function has no annotation
  • One of the arguments has a protected type, found via expr_ty followed by walk_ty

modulo whitelist

We wish to be able to correctly track stuff in generic containers such as vectors.

Note that we already do this. walk_ty walks the type name (not the actual type fields), so Vec<Foo<Bar, Baz>> would be walked as Vec<Foo<Bar, Baz>>, Foo<Bar, Baz>, Bar, Baz. This might just solve the problem in its entirety (assuming we fix the function issue above):

  • If our type name is Foo<NoDrop>, i.e. has NoDrop somewhere within the type params, walk_ty will find out and error. Easy peasy.
  • If our type name is Foo, and the contained NoDrop is hardcoded in the definition of Foo (as opposed to a type param); since Foo needs to be defined in a crate after (or the same crate as) the crate which NoDrop is defined in, it should be running the plugin too, and we can ensure that drop_protect is propagated via a simple lint.

The only way to work around this is:

  • Use a type alias to circumvent walk_ty (type MyVec=Vec<NoDrop>). This is easily caught by a lint.
  • Define a struct containing the protected type in a crate that doesn't run the plugin: We are assuming that all crates using the crate with drop protected variables will also run the plugin (not much we can do if people forget)
  • Trait objects. Not entirely sure of this, but I think the creation of an owning trait object of a protected type can be caught by a lint. Or we can ignore it; this seems like an edge case.

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

No branches or pull requests

2 participants