forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Allow types containing references to have finalizers
Previously FSA prevented `Gc<T>`s from having a finalizer if `T`
(directly or indirectly) contained a reference. This made retro-fitting
GC to large libraries difficult. It also, somewhat surprisingly,
completely prevents finalizable nested `Gc<dyn Trait>`s (due to the
underlying vptr being a `&'static` reference).
This commit loosens this restriction, allowing FSA to interrogate drop
method bodies (much the same as for `Send + Sync + FinalizerSafe`)
looking for unsound uses of references.
This change is based on the following observation: a reference inside of
`T`'s drop method is safe to dereference provided it never originates
from `T` or one of `T`'s fields. For example:
```rust
fn drop(&mut self) {
let a = 123;
let r1 = &1; // const with 'static lifetime
let r2 = &a; // stack-local
let r3 = &*(unsafe { ... }); // unsafe code, all bets are off.
let r4: &u64 = self.a;
let r5: &u64 = self.b.a;
let r6: &u64 = self.c[0]; // c: [&u64; 2]
}
```
In this example, `r1`, `r2`, and `r3` are allowed by FSA: `r1` and `r2`
are sound because their referent will always be valid when the finalizer
is called. `r3` uses unsafe code, so FSA gets out of the way here, as it
is up to the user to ensure that this is sound themselves.
`r4`, `r5`, and `r6` on the other hand are not allowed by FSA. This is
because they are references which are obtained through field or array
index projections into `T`. Such projections are nearly always unsound,
so we always prevent these.
This change means that we can remove the `ReferenceFree` trait, since we
now always check for references inside a drop method. As with our
relaxed FSA rules for `FinalizerSafe` and friends -- if for any reason
we can't see into a type, we err on the side of caution and emit an
error.- Loading branch information
1 parent
e292b7e
commit be985b8
Showing
14 changed files
with
380 additions
and
71 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -284,7 +284,6 @@ symbols! { | |
| RefCell, | ||
| RefCellRef, | ||
| RefCellRefMut, | ||
| ReferenceFree, | ||
| Relaxed, | ||
| Release, | ||
| Result, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| #[inline(never)] | ||
| fn use_val<T: std::fmt::Debug>(x: T) { | ||
| dbg!("{:?}", x); | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| struct HasNestedRef<'a> { | ||
| a: &'a u64, | ||
| b: u64, | ||
| c: HasRef<'a>, | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| struct HasRef<'a> { | ||
| a: &'a u64, | ||
| b: u64, | ||
| c: [&'a u64; 2] | ||
| } | ||
|
|
||
| impl<'a> HasRef<'a> { | ||
| #[inline(never)] | ||
| fn new(a: &'a u64, b: u64, c: [&'a u64; 2]) -> Self { | ||
| Self { a, b, c } | ||
| } | ||
|
|
||
| #[inline(always)] | ||
| fn new_inlined(a: &'a u64, b: u64, c: [&'a u64; 2]) -> Self { | ||
| Self { a, b, c } | ||
| } | ||
| } | ||
|
|
||
| impl<'a> std::default::Default for HasRef<'a> { | ||
| #[inline(never)] | ||
| fn default() -> Self { | ||
| Self { a: &1, b: 1, c: [&1, &2] } | ||
| } | ||
| } | ||
|
|
||
| impl<'a> std::default::Default for HasNestedRef<'a> { | ||
| #[inline(never)] | ||
| fn default() -> Self { | ||
| Self { a: &1, b: 1, c: HasRef::default() } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #![feature(gc)] | ||
| #![feature(negative_impls)] | ||
| #![allow(dead_code)] | ||
| #![allow(unused_variables)] | ||
| include!{"./auxiliary/types.rs"} | ||
|
|
||
| impl<'a> Drop for HasRef<'a> { | ||
| fn drop(&mut self) { | ||
| use_val(self.a); // should fail | ||
| use_val(self.b); // should pass | ||
| use_val(self.c[0]); // should fail | ||
|
|
||
| let a = self.a; // should fail | ||
| let b = self.b; // should pass | ||
| let c = self.c; | ||
| use_val(c[1]); // should fail | ||
|
|
||
| // should pass, as not a field projection | ||
| let c = &1; | ||
| use_val(c); | ||
| } | ||
| } | ||
|
|
||
| fn main() { | ||
| std::gc::Gc::new(HasRef::default()); | ||
| //~^ ERROR: `HasRef::default()` has a drop method which cannot be safely finalized. | ||
| //~^^ ERROR: `HasRef::default()` has a drop method which cannot be safely finalized. | ||
| //~^^^ ERROR: `HasRef::default()` has a drop method which cannot be safely finalized. | ||
| //~^^^^ ERROR: `HasRef::default()` has a drop method which cannot be safely finalized. | ||
| //~^^^^^ ERROR: `HasRef::default()` has a drop method which cannot be safely finalized. | ||
| } |
Oops, something went wrong.