OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #25212 - pnkfelix:dropck-box-trait, r=nikomatsakis

Browse Source 
dropck: must assume `Box<Trait + 'a>` has a destructor of interest.

Fix #25199.

This detail was documented in [RFC 769]; the implementation was just missing.

[breaking-change]

The breakage here falls into both obvious and non-obvious cases.

The obvious case: if you were relying on the unsoundness this exposes (namely being able to reference dead storage from a destructor, by doing it via a boxed trait object bounded by the lifetime of the dead storage), then this change disallows that.

The non-obvious cases: The way dropck works, it causes lifetimes to be extended to longer extents than they covered before. I.e.  lifetimes that are attached as trait-bounds may become longer than they were previously.

* This includes lifetimes that are only *implicitly* attached as trait-bounds (due to [RFC 599]). So you may have code that was e.g. taking a parameter of type `&'a Box<Trait>` (which expands to `&'a Box<Trait+'a>`), that now may need to be assigned type `&'a Box<Trait+'static>` to ensure that `'a` is not inadvertantly inferred to a region that is actually too long.  (See commit ee06263 for an example of this.)

[RFC 769]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#the-drop-check-rule

[RFC 599]: https://github.com/rust-lang/rfcs/blob/master/text/0599-default-object-bound.md
This commit is contained in:
bors 2015-05-09 09:20:43 +00:00
commit 3906edf41e
8 changed files with 393 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

259
src/librustc_typeck/check/dropck.rs
View File

@ -396,19 +396,24 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
}
};
let opt_type_did = match typ.sty {
ty::ty_struct(struct_did, _) => Some(struct_did),
ty::ty_enum(enum_did, _) => Some(enum_did),
_ => None,
let dtor_kind = match typ.sty {
ty::ty_enum(def_id, _) |
ty::ty_struct(def_id, _) => {
match destructor_for_type.get(&def_id) {
Some(def_id) => DtorKind::KnownDropMethod(*def_id),
None => DtorKind::PureRecur,
}
}
ty::ty_trait(ref ty_trait) => {
DtorKind::Unknown(ty_trait.bounds.clone())
}
_ => DtorKind::PureRecur,
};
let opt_dtor =
opt_type_did.and_then(|did| destructor_for_type.get(&did));
debug!("iterate_over_potentially_unsafe_regions_in_type \
{}typ: {} scope: {:?} opt_dtor: {:?} xref: {}",
{}typ: {} scope: {:?} xref: {}",
(0..depth).map(|_| ' ').collect::<String>(),
typ.repr(rcx.tcx()), scope, opt_dtor, xref_depth);
typ.repr(rcx.tcx()), scope, xref_depth);
// If `typ` has a destructor, then we must ensure that all
// borrowed data reachable via `typ` must outlive the parent
@ -439,102 +444,7 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
// simply skip the `type_must_outlive` call entirely (but
// resume the recursive checking of the type-substructure).
let has_dtor_of_interest;
if let Some(&dtor_method_did) = opt_dtor {
let impl_did = ty::impl_of_method(rcx.tcx(), dtor_method_did)
.unwrap_or_else(|| {
rcx.tcx().sess.span_bug(
span, "no Drop impl found for drop method")
});
let dtor_typescheme = ty::lookup_item_type(rcx.tcx(), impl_did);
let dtor_generics = dtor_typescheme.generics;
let mut has_pred_of_interest = false;
let mut seen_items = Vec::new();
let mut items_to_inspect = vec![impl_did];
'items: while let Some(item_def_id) = items_to_inspect.pop() {
if seen_items.contains(&item_def_id) {
continue;
}
for pred in ty::lookup_predicates(rcx.tcx(), item_def_id).predicates {
let result = match pred {
ty::Predicate::Equate(..) |
ty::Predicate::RegionOutlives(..) |
ty::Predicate::TypeOutlives(..) |
ty::Predicate::Projection(..) => {
// For now, assume all these where-clauses
// may give drop implementation capabilty
// to access borrowed data.
true
}
ty::Predicate::Trait(ty::Binder(ref t_pred)) => {
let def_id = t_pred.trait_ref.def_id;
if ty::trait_items(rcx.tcx(), def_id).len() != 0 {
// If trait has items, assume it adds
// capability to access borrowed data.
true
} else {
// Trait without items is itself
// uninteresting from POV of dropck.
//
// However, may have parent w/ items;
// so schedule checking of predicates,
items_to_inspect.push(def_id);
// and say "no capability found" for now.
false
}
}
};
if result {
has_pred_of_interest = true;
debug!("typ: {} has interesting dtor due to generic preds, e.g. {}",
typ.repr(rcx.tcx()), pred.repr(rcx.tcx()));
break 'items;
}
}
seen_items.push(item_def_id);
}
// In `impl<'a> Drop ...`, we automatically assume
// `'a` is meaningful and thus represents a bound
// through which we could reach borrowed data.
//
// FIXME (pnkfelix): In the future it would be good to
// extend the language to allow the user to express,
// in the impl signature, that a lifetime is not
// actually used (something like `where 'a: ?Live`).
let has_region_param_of_interest =
dtor_generics.has_region_params(subst::TypeSpace);
has_dtor_of_interest =
has_region_param_of_interest ||
has_pred_of_interest;
if has_dtor_of_interest {
debug!("typ: {} has interesting dtor, due to \
region params: {} or pred: {}",
typ.repr(rcx.tcx()),
has_region_param_of_interest,
has_pred_of_interest);
} else {
debug!("typ: {} has dtor, but it is uninteresting",
typ.repr(rcx.tcx()));
}
} else {
debug!("typ: {} has no dtor, and thus is uninteresting",
typ.repr(rcx.tcx()));
has_dtor_of_interest = false;
}
if has_dtor_of_interest {
if has_dtor_of_interest(rcx.tcx(), dtor_kind, typ, span) {
// If `typ` has a destructor, then we must ensure that all
// borrowed data reachable via `typ` must outlive the
// parent of `scope`. (It does not suffice for it to
@ -620,7 +530,7 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
ty::ty_rptr(..) | ty::ty_ptr(_) | ty::ty_bare_fn(..) => {
// Don't recurse, since references, pointers,
// boxes, and bare functions don't own instances
// and bare functions don't own instances
// of the types appearing within them.
walker.skip_current_subtree();
}
@ -639,3 +549,140 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
return Ok(());
}
enum DtorKind<'tcx> {
// Type has an associated drop method with this def id
KnownDropMethod(ast::DefId),
// Type has no destructor (or its dtor is known to be pure
// with respect to lifetimes), though its *substructure*
// may carry a destructor.
PureRecur,
// Type may have impure destructor that is unknown;
// e.g. `Box<Trait+'a>`
Unknown(ty::ExistentialBounds<'tcx>),
}
fn has_dtor_of_interest<'tcx>(tcx: &ty::ctxt<'tcx>,
dtor_kind: DtorKind,
typ: ty::Ty<'tcx>,
span: Span) -> bool {
let has_dtor_of_interest: bool;
match dtor_kind {
DtorKind::PureRecur => {
has_dtor_of_interest = false;
debug!("typ: {} has no dtor, and thus is uninteresting",
typ.repr(tcx));
}
DtorKind::Unknown(bounds) => {
match bounds.region_bound {
ty::ReStatic => {
debug!("trait: {} has 'static bound, and thus is uninteresting",
typ.repr(tcx));
has_dtor_of_interest = false;
}
ty::ReEmpty => {
debug!("trait: {} has empty region bound, and thus is uninteresting",
typ.repr(tcx));
has_dtor_of_interest = false;
}
r => {
debug!("trait: {} has non-static bound: {}; assumed interesting",
typ.repr(tcx), r.repr(tcx));
has_dtor_of_interest = true;
}
}
}
DtorKind::KnownDropMethod(dtor_method_did) => {
let impl_did = ty::impl_of_method(tcx, dtor_method_did)
.unwrap_or_else(|| {
tcx.sess.span_bug(
span, "no Drop impl found for drop method")
});
let dtor_typescheme = ty::lookup_item_type(tcx, impl_did);
let dtor_generics = dtor_typescheme.generics;
let mut has_pred_of_interest = false;
let mut seen_items = Vec::new();
let mut items_to_inspect = vec![impl_did];
'items: while let Some(item_def_id) = items_to_inspect.pop() {
if seen_items.contains(&item_def_id) {
continue;
}
for pred in ty::lookup_predicates(tcx, item_def_id).predicates {
let result = match pred {
ty::Predicate::Equate(..) |
ty::Predicate::RegionOutlives(..) |
ty::Predicate::TypeOutlives(..) |
ty::Predicate::Projection(..) => {
// For now, assume all these where-clauses
// may give drop implementation capabilty
// to access borrowed data.
true
}
ty::Predicate::Trait(ty::Binder(ref t_pred)) => {
let def_id = t_pred.trait_ref.def_id;
if ty::trait_items(tcx, def_id).len() != 0 {
// If trait has items, assume it adds
// capability to access borrowed data.
true
} else {
// Trait without items is itself
// uninteresting from POV of dropck.
//
// However, may have parent w/ items;
// so schedule checking of predicates,
items_to_inspect.push(def_id);
// and say "no capability found" for now.
false
}
}
};
if result {
has_pred_of_interest = true;
debug!("typ: {} has interesting dtor due to generic preds, e.g. {}",
typ.repr(tcx), pred.repr(tcx));
break 'items;
}
}
seen_items.push(item_def_id);
}
// In `impl<'a> Drop ...`, we automatically assume
// `'a` is meaningful and thus represents a bound
// through which we could reach borrowed data.
//
// FIXME (pnkfelix): In the future it would be good to
// extend the language to allow the user to express,
// in the impl signature, that a lifetime is not
// actually used (something like `where 'a: ?Live`).
let has_region_param_of_interest =
dtor_generics.has_region_params(subst::TypeSpace);
has_dtor_of_interest =
has_region_param_of_interest ||
has_pred_of_interest;
if has_dtor_of_interest {
debug!("typ: {} has interesting dtor, due to \
region params: {} or pred: {}",
typ.repr(tcx),
has_region_param_of_interest,
has_pred_of_interest);
} else {
debug!("typ: {} has dtor, but it is uninteresting",
typ.repr(tcx));
}
}
}
return has_dtor_of_interest;
}

2
src/libstd/net/parser.rs
View File

@ -61,7 +61,7 @@ impl<'a> Parser<'a> {
}
// Return result of first successful parser
fn read_or<T>(&mut self, parsers: &mut [Box<FnMut(&mut Parser) -> Option<T>>])
fn read_or<T>(&mut self, parsers: &mut [Box<FnMut(&mut Parser) -> Option<T> + 'static>])
-> Option<T> {
for pf in parsers.iter_mut() {
match self.read_atomically(|p: &mut Parser| pf(p)) {

7
src/libsyntax/ext/expand.rs
View File

@ -986,9 +986,10 @@ fn expand_pat(p: P<ast::Pat>, fld: &mut MacroExpander) -> P<ast::Pat> {
let fm = fresh_mark();
let marked_before = mark_tts(&tts[..], fm);
let mac_span = fld.cx.original_span();
let expanded = match expander.expand(fld.cx,
mac_span,
&marked_before[..]).make_pat() {
let pat = expander.expand(fld.cx,
mac_span,
&marked_before[..]).make_pat();
let expanded = match pat {
Some(e) => e,
None => {
fld.cx.span_err(

6
src/libsyntax/util/parser_testing.rs
View File

@ -73,7 +73,11 @@ pub fn string_to_stmt(source_str : String) -> P<ast::Stmt> {
/// Parse a string, return a pat. Uses "irrefutable"... which doesn't
/// (currently) affect parsing.
pub fn string_to_pat(source_str: String) -> P<ast::Pat> {
string_to_parser(&new_parse_sess(), source_str).parse_pat()
// Binding `sess` and `parser` works around dropck-injected
// region-inference issues; see #25212, #22323, #22321.
let sess = new_parse_sess();
let mut parser = string_to_parser(&sess, source_str);
parser.parse_pat()
}
/// Convert a vector of strings to a vector of ast::Ident's

131
src/test/compile-fail/dropck_trait_cycle_checked.rs Normal file
View File

@ -0,0 +1,131 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Reject mixing cyclic structure and Drop when using trait
// objects to hide the the cross-references.
//
// (Compare against compile-fail/dropck_vec_cycle_checked.rs)
use std::cell::Cell;
use id::Id;
mod s {
use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering};
static S_COUNT: AtomicUsize = ATOMIC_USIZE_INIT;
pub fn next_count() -> usize {
S_COUNT.fetch_add(1, Ordering::SeqCst) + 1
}
}
mod id {
use s;
#[derive(Debug)]
pub struct Id {
orig_count: usize,
count: usize,
}
impl Id {
pub fn new() -> Id {
let c = s::next_count();
println!("building Id {}", c);
Id { orig_count: c, count: c }
}
pub fn count(&self) -> usize {
println!("Id::count on {} returns {}", self.orig_count, self.count);
self.count
}
}
impl Drop for Id {
fn drop(&mut self) {
println!("dropping Id {}", self.count);
self.count = 0;
}
}
}
trait HasId {
fn count(&self) -> usize;
}
#[derive(Debug)]
struct CheckId<T:HasId> {
v: T
}
#[allow(non_snake_case)]
fn CheckId<T:HasId>(t: T) -> CheckId<T> { CheckId{ v: t } }
impl<T:HasId> Drop for CheckId<T> {
fn drop(&mut self) {
assert!(self.v.count() > 0);
}
}
trait Obj<'a> : HasId {
fn set0(&self, b: &'a Box<Obj<'a>>);
fn set1(&self, b: &'a Box<Obj<'a>>);
}
struct O<'a> {
id: Id,
obj0: CheckId<Cell<Option<&'a Box<Obj<'a>>>>>,
obj1: CheckId<Cell<Option<&'a Box<Obj<'a>>>>>,
}
impl<'a> HasId for O<'a> {
fn count(&self) -> usize { self.id.count() }
}
impl<'a> O<'a> {
fn new() -> Box<O<'a>> {
Box::new(O {
id: Id::new(),
obj0: CheckId(Cell::new(None)),
obj1: CheckId(Cell::new(None)),
})
}
}
impl<'a> HasId for Cell<Option<&'a Box<Obj<'a>>>> {
fn count(&self) -> usize {
match self.get() {
None => 1,
Some(c) => c.count(),
}
}
}
impl<'a> Obj<'a> for O<'a> {
fn set0(&self, b: &'a Box<Obj<'a>>) {
self.obj0.v.set(Some(b))
}
fn set1(&self, b: &'a Box<Obj<'a>>) {
self.obj1.v.set(Some(b))
}
}
fn f() {
let (o1, o2, o3): (Box<Obj>, Box<Obj>, Box<Obj>) = (O::new(), O::new(), O::new());
o1.set0(&o2); //~ ERROR `o2` does not live long enough
o1.set1(&o3); //~ ERROR `o3` does not live long enough
o2.set0(&o2); //~ ERROR `o2` does not live long enough
o2.set1(&o3); //~ ERROR `o3` does not live long enough
o3.set0(&o1); //~ ERROR `o1` does not live long enough
o3.set1(&o2); //~ ERROR `o2` does not live long enough
}
fn main() {
f();
}

83
src/test/compile-fail/issue-25199.rs Normal file
View File

@ -0,0 +1,83 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Regression test for Issue 25199: Check that one cannot hide a
// destructor's access to borrowed data behind a boxed trait object.
//
// Prior to fixing Issue 25199, this example was able to be compiled
// with rustc, and thus when you ran it, you would see the `Drop` impl
// for `Test` accessing state that had already been dropping (which is
// marked explicitly here with checking code within the `Drop` impl
// for `VecHolder`, but in the general case could just do unsound
// things like accessing memory that has been freed).
//
// Note that I would have liked to encode my go-to example of cyclic
// structure that accesses its neighbors in drop (and thus is
// fundamentally unsound) via this trick, but the closest I was able
// to come was dropck_trait_cycle_checked.rs, which is not quite as
// "good" as this regression test because the encoding of that example
// was forced to attach a lifetime to the trait definition itself
// (`trait Obj<'a>`) while *this* example is solely
use std::cell::RefCell;
trait Obj { }
struct VecHolder {
v: Vec<(bool, &'static str)>,
}
impl Drop for VecHolder {
fn drop(&mut self) {
println!("Dropping Vec");
self.v[30].0 = false;
self.v[30].1 = "invalid access: VecHolder dropped already";
}
}
struct Container<'a> {
v: VecHolder,
d: RefCell<Vec<Box<Obj+'a>>>,
}
impl<'a> Container<'a> {
fn new() -> Container<'a> {
Container {
d: RefCell::new(Vec::new()),
v: VecHolder {
v: vec![(true, "valid"); 100]
}
}
}
fn store<T: Obj+'a>(&'a self, val: T) {
self.d.borrow_mut().push(Box::new(val));
}
}
struct Test<'a> {
test: &'a Container<'a>,
}
impl<'a> Obj for Test<'a> { }
impl<'a> Drop for Test<'a> {
fn drop(&mut self) {
for e in &self.test.v.v {
assert!(e.0, e.1);
}
}
}
fn main() {
let container = Container::new();
let test = Test{test: &container}; //~ ERROR `container` does not live long enough
println!("container.v[30]: {:?}", container.v.v[30]);
container.store(test); //~ ERROR `container` does not live long enough
}

14
src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs
View File

@ -22,12 +22,24 @@ fn a() {
let mut factorial: Option<Box<Fn(u32) -> u32>> = None;
let f = |x: u32| -> u32 {
//~^ ERROR `factorial` does not live long enough
let g = factorial.as_ref().unwrap();
if x == 0 {1} else {x * g(x-1)}
};
factorial = Some(Box::new(f));
}
fn b() {
let mut factorial: Option<Box<Fn(u32) -> u32 + 'static>> = None;
let f = |x: u32| -> u32 {
//~^ ERROR closure may outlive the current function, but it borrows `factorial`
let g = factorial.as_ref().unwrap();
if x == 0 {1} else {x * g(x-1)}
};
factorial = Some(Box::new(f));
//~^ ERROR cannot assign to `factorial` because it is borrowed
}
fn main() { }

6
src/test/run-pass/issue-11205.rs
View File

@ -21,7 +21,7 @@ fn foos(_: &[&Foo]) {}
fn foog<T>(_: &[T], _: &[T]) {}
fn bar(_: [Box<Foo>; 2]) {}
fn bars(_: &[Box<Foo>]) {}
fn bars(_: &[Box<Foo+'static>]) {}
fn main() {
let x: [&Foo; 2] = [&1, &2];
@ -45,11 +45,11 @@ fn main() {
bar(x);
bar([Box::new(1), Box::new(2)]);
let x: &[Box<Foo>] = &[Box::new(1), Box::new(2)];
let x: &[Box<Foo+'static>] = &[Box::new(1), Box::new(2)];
bars(x);
bars(&[Box::new(1), Box::new(2)]);
let x: &[Box<Foo>] = &[Box::new(1), Box::new(2)];
let x: &[Box<Foo+'static>] = &[Box::new(1), Box::new(2)];
foog(x, &[Box::new(1)]);
struct T<'a> {