From edb8d1c0d4b2e32088f3c6a77437b091b10ddf7e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 13 Apr 2018 17:11:53 +0200 Subject: [PATCH 1/2] Conservatively assume dropping a generator touches its upvars, via locals' dtors. This is meant to address rust-lang/rust#49918. Review feedback: put back comment justifying skipping interior traversal. Review feedback: dropck generators like trait objects: all their upvars must outlive the generator itself, so just create a DtorckConstraint saying so. --- src/librustc_traits/dropck_outlives.rs | 40 ++++++++++++++++++++------ 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/librustc_traits/dropck_outlives.rs b/src/librustc_traits/dropck_outlives.rs index 5f4daf0d568f8..ba31ce2692fbc 100644 --- a/src/librustc_traits/dropck_outlives.rs +++ b/src/librustc_traits/dropck_outlives.rs @@ -193,14 +193,38 @@ fn dtorck_constraint_for_ty<'a, 'gcx, 'tcx>( .map(|ty| dtorck_constraint_for_ty(tcx, span, for_ty, depth + 1, ty)) .collect(), - ty::TyGenerator(def_id, substs, _) => { - // Note that the interior types are ignored here. - // Any type reachable inside the interior must also be reachable - // through the upvars. - substs - .upvar_tys(def_id, tcx) - .map(|ty| dtorck_constraint_for_ty(tcx, span, for_ty, depth + 1, ty)) - .collect() + ty::TyGenerator(def_id, substs, _interior) => { + // rust-lang/rust#49918: types can be constructed, stored + // in the interior, and sit idle when generator yields + // (and is subsequently dropped). + // + // It would be nice to descend into interior of a + // generator to determine what effects dropping it might + // have (by looking at any drop effects associated with + // its interior). + // + // However, the interior's representation uses things like + // TyGeneratorWitness that explicitly assume they are not + // traversed in such a manner. So instead, we will + // simplify things for now by treating all generators as + // if they were like trait objects, where its upvars must + // all be alive for the generator's (potential) + // destructor. + // + // In particular, skipping over `_interior` is safe + // because any side-effects from dropping `_interior` can + // only take place through references with lifetimes + // derived from lifetimes attached to the upvars, and we + // *do* incorporate the upvars here. + + let constraint = DtorckConstraint { + outlives: substs.upvar_tys(def_id, tcx).map(|t| t.into()).collect(), + dtorck_types: vec![], + overflows: vec![], + }; + debug!("dtorck_constraint: generator {:?} => {:?}", def_id, constraint); + + Ok(constraint) } ty::TyAdt(def, substs) => { From f12d7a55fca7215a08029c90634ab54f883a9f1c Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 16 Apr 2018 12:41:32 +0200 Subject: [PATCH 2/2] Update ui/generator tests to reflect changes from new generator drop rules. --- src/test/ui/generator/borrowing.nll.stderr | 38 +++++++++++++------ src/test/ui/generator/borrowing.rs | 4 +- src/test/ui/generator/dropck.nll.stderr | 26 ++++++++----- src/test/ui/generator/dropck.rs | 5 ++- src/test/ui/generator/dropck.stderr | 15 +++++++- .../ref-escapes-but-not-over-yield.nll.stderr | 18 ++++++--- 6 files changed, 74 insertions(+), 32 deletions(-) diff --git a/src/test/ui/generator/borrowing.nll.stderr b/src/test/ui/generator/borrowing.nll.stderr index 1801da6c8b2dd..243e901858513 100644 --- a/src/test/ui/generator/borrowing.nll.stderr +++ b/src/test/ui/generator/borrowing.nll.stderr @@ -1,14 +1,30 @@ -error: compilation successful - --> $DIR/borrowing.rs:15:1 +error[E0597]: `a` does not live long enough + --> $DIR/borrowing.rs:18:18 | -LL | / fn main() { #![rustc_error] // rust-lang/rust#49855 -LL | | let _b = { -LL | | let a = 3; -LL | | unsafe { (|| yield &a).resume() } -... | -LL | | }; -LL | | } - | |_^ +LL | unsafe { (|| yield &a).resume() } + | ^^^^^^^^^^^^^ + | | + | borrowed value does not live long enough + | borrow may end up in a temporary, created here +LL | //~^ ERROR: `a` does not live long enough +LL | }; + | -- temporary later dropped here, potentially using the reference + | | + | borrowed value only lives until here -error: aborting due to previous error +error[E0597]: `a` does not live long enough + --> $DIR/borrowing.rs:24:9 + | +LL | / || { +LL | | yield &a +LL | | //~^ ERROR: `a` does not live long enough +LL | | } + | |_________^ borrowed value does not live long enough +LL | }; + | - borrowed value only lives until here +LL | } + | - borrow later used here, when `_b` is dropped + +error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/generator/borrowing.rs b/src/test/ui/generator/borrowing.rs index f80aca9fb00e6..e56927d818231 100644 --- a/src/test/ui/generator/borrowing.rs +++ b/src/test/ui/generator/borrowing.rs @@ -8,11 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(generators, generator_trait, rustc_attrs)] +#![feature(generators, generator_trait)] use std::ops::Generator; -fn main() { #![rustc_error] // rust-lang/rust#49855 +fn main() { let _b = { let a = 3; unsafe { (|| yield &a).resume() } diff --git a/src/test/ui/generator/dropck.nll.stderr b/src/test/ui/generator/dropck.nll.stderr index 72ebaab327898..c352fd6a62f28 100644 --- a/src/test/ui/generator/dropck.nll.stderr +++ b/src/test/ui/generator/dropck.nll.stderr @@ -1,14 +1,20 @@ -error: compilation successful - --> $DIR/dropck.rs:16:1 +error[E0597]: `ref_` does not live long enough + --> $DIR/dropck.rs:22:11 | -LL | / fn main() { #![rustc_error] // rust-lang/rust#49855 -LL | | let (cell, mut gen); -LL | | cell = Box::new(RefCell::new(0)); -LL | | let ref_ = Box::leak(Box::new(Some(cell.borrow_mut()))); -... | -LL | | // drops the RefCell and then the Ref, leading to use-after-free -LL | | } - | |_^ +LL | gen = || { + | ___________^ +LL | | // but the generator can use it to drop a `Ref<'a, i32>`. +LL | | let _d = ref_.take(); //~ ERROR `ref_` does not live long enough +LL | | yield; +LL | | }; + | |_____^ borrowed value does not live long enough +... +LL | } + | - + | | + | borrowed value only lives until here + | borrow later used here, when `gen` is dropped error: aborting due to previous error +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/generator/dropck.rs b/src/test/ui/generator/dropck.rs index 8f4ba64fd5727..857288818c67c 100644 --- a/src/test/ui/generator/dropck.rs +++ b/src/test/ui/generator/dropck.rs @@ -8,15 +8,16 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(generators, generator_trait, box_leak, rustc_attrs)] +#![feature(generators, generator_trait, box_leak)] use std::cell::RefCell; use std::ops::Generator; -fn main() { #![rustc_error] // rust-lang/rust#49855 +fn main() { let (cell, mut gen); cell = Box::new(RefCell::new(0)); let ref_ = Box::leak(Box::new(Some(cell.borrow_mut()))); + //~^ ERROR `*cell` does not live long enough [E0597] // the upvar is the non-dropck `&mut Option>`. gen = || { // but the generator can use it to drop a `Ref<'a, i32>`. diff --git a/src/test/ui/generator/dropck.stderr b/src/test/ui/generator/dropck.stderr index 4a22d299701b4..1a6fed2dd3594 100644 --- a/src/test/ui/generator/dropck.stderr +++ b/src/test/ui/generator/dropck.stderr @@ -1,5 +1,16 @@ +error[E0597]: `*cell` does not live long enough + --> $DIR/dropck.rs:19:40 + | +LL | let ref_ = Box::leak(Box::new(Some(cell.borrow_mut()))); + | ^^^^ borrowed value does not live long enough +... +LL | } + | - `*cell` dropped here while still borrowed + | + = note: values in a scope are dropped in the opposite order they are created + error[E0597]: `ref_` does not live long enough - --> $DIR/dropck.rs:23:18 + --> $DIR/dropck.rs:24:18 | LL | gen = || { | -- capture occurs here @@ -12,6 +23,6 @@ LL | } | = note: values in a scope are dropped in the opposite order they are created -error: aborting due to previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/generator/ref-escapes-but-not-over-yield.nll.stderr b/src/test/ui/generator/ref-escapes-but-not-over-yield.nll.stderr index 08839c23c3762..70870b98365b9 100644 --- a/src/test/ui/generator/ref-escapes-but-not-over-yield.nll.stderr +++ b/src/test/ui/generator/ref-escapes-but-not-over-yield.nll.stderr @@ -1,11 +1,19 @@ error[E0597]: `b` does not live long enough --> $DIR/ref-escapes-but-not-over-yield.rs:24:13 | -LL | a = &b; - | ^^ borrowed value does not live long enough -LL | //~^ ERROR `b` does not live long enough -LL | }; - | - borrowed value only lives until here +LL | let mut b = move || { + | _________________- +LL | | yield(); +LL | | let b = 5; +LL | | a = &b; + | | ^^ borrowed value does not live long enough +LL | | //~^ ERROR `b` does not live long enough +LL | | }; + | | - + | | | + | | borrowed value only lives until here + | |_____temporary later dropped here, potentially using the reference + | borrow may end up in a temporary, created here error: aborting due to previous error