From cd9f5ff2a1c50a5af94ad1dd1c976f631b9f19a6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 13 Feb 2020 20:29:03 +0000 Subject: [PATCH 1/2] Check `Copy` lifetimes bounds when copying from a projection --- .../borrow_check/type_check/mod.rs | 52 +++++++++---------- ...not-ignore-lifetime-bounds-in-copy-proj.rs | 12 +++++ ...ignore-lifetime-bounds-in-copy-proj.stderr | 14 +++++ 3 files changed, 51 insertions(+), 27 deletions(-) create mode 100644 src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.rs create mode 100644 src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.stderr diff --git a/src/librustc_mir/borrow_check/type_check/mod.rs b/src/librustc_mir/borrow_check/type_check/mod.rs index 5dab064c7b7..232e7e6b83d 100644 --- a/src/librustc_mir/borrow_check/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/mod.rs @@ -466,33 +466,6 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { let mut place_ty = PlaceTy::from_ty(self.body.local_decls[place.local].ty); - if place.projection.is_empty() { - if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) = context { - let tcx = self.tcx(); - let trait_ref = ty::TraitRef { - def_id: tcx.lang_items().copy_trait().unwrap(), - substs: tcx.mk_substs_trait(place_ty.ty, &[]), - }; - - // To have a `Copy` operand, the type `T` of the - // value must be `Copy`. Note that we prove that `T: Copy`, - // rather than using the `is_copy_modulo_regions` - // test. This is important because - // `is_copy_modulo_regions` ignores the resulting region - // obligations and assumes they pass. This can result in - // bounds from `Copy` impls being unsoundly ignored (e.g., - // #29149). Note that we decide to use `Copy` before knowing - // whether the bounds fully apply: in effect, the rule is - // that if a value of some type could implement `Copy`, then - // it must. - self.cx.prove_trait_ref( - trait_ref, - location.to_locations(), - ConstraintCategory::CopyBound, - ); - } - } - for elem in place.projection.iter() { if place_ty.variant_index.is_none() { if place_ty.ty.references_error() { @@ -503,6 +476,31 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { place_ty = self.sanitize_projection(place_ty, elem, place, location) } + if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) = context { + let tcx = self.tcx(); + let trait_ref = ty::TraitRef { + def_id: tcx.lang_items().copy_trait().unwrap(), + substs: tcx.mk_substs_trait(place_ty.ty, &[]), + }; + + // To have a `Copy` operand, the type `T` of the + // value must be `Copy`. Note that we prove that `T: Copy`, + // rather than using the `is_copy_modulo_regions` + // test. This is important because + // `is_copy_modulo_regions` ignores the resulting region + // obligations and assumes they pass. This can result in + // bounds from `Copy` impls being unsoundly ignored (e.g., + // #29149). Note that we decide to use `Copy` before knowing + // whether the bounds fully apply: in effect, the rule is + // that if a value of some type could implement `Copy`, then + // it must. + self.cx.prove_trait_ref( + trait_ref, + location.to_locations(), + ConstraintCategory::CopyBound, + ); + } + place_ty } diff --git a/src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.rs b/src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.rs new file mode 100644 index 00000000000..96c8719468f --- /dev/null +++ b/src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.rs @@ -0,0 +1,12 @@ +// Test that the 'static bound from the Copy impl is respected. Regression test for #29149. + +#[derive(Clone)] +struct Foo<'a>(&'a u32); +impl Copy for Foo<'static> {} + +fn main() { + let s = 2; + let a = (Foo(&s),); //~ ERROR `s` does not live long enough [E0597] + drop(a.0); + drop(a.0); +} diff --git a/src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.stderr b/src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.stderr new file mode 100644 index 00000000000..65be3b37e0e --- /dev/null +++ b/src/test/ui/nll/do-not-ignore-lifetime-bounds-in-copy-proj.stderr @@ -0,0 +1,14 @@ +error[E0597]: `s` does not live long enough + --> $DIR/do-not-ignore-lifetime-bounds-in-copy-proj.rs:9:18 + | +LL | let a = (Foo(&s),); + | ^^ borrowed value does not live long enough +LL | drop(a.0); + | --- copying this value requires that `s` is borrowed for `'static` +LL | drop(a.0); +LL | } + | - `s` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. From ddc25456c5945457ba86cb60994ce9872bd98edd Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 13 Feb 2020 20:29:30 +0000 Subject: [PATCH 2/2] Check types of statics in MIR typeck --- .../borrow_check/type_check/mod.rs | 20 +++++++++++-- src/test/ui/nll/issue-69114-static-mut-ty.rs | 30 +++++++++++++++++++ .../ui/nll/issue-69114-static-mut-ty.stderr | 27 +++++++++++++++++ src/test/ui/nll/issue-69114-static-ty.rs | 9 ++++++ src/test/ui/nll/issue-69114-static-ty.stderr | 15 ++++++++++ 5 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/nll/issue-69114-static-mut-ty.rs create mode 100644 src/test/ui/nll/issue-69114-static-mut-ty.stderr create mode 100644 src/test/ui/nll/issue-69114-static-ty.rs create mode 100644 src/test/ui/nll/issue-69114-static-ty.stderr diff --git a/src/librustc_mir/borrow_check/type_check/mod.rs b/src/librustc_mir/borrow_check/type_check/mod.rs index 232e7e6b83d..78f708b9a74 100644 --- a/src/librustc_mir/borrow_check/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/mod.rs @@ -309,6 +309,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> { ); } } else { + let tcx = self.tcx(); if let ty::ConstKind::Unevaluated(def_id, substs, promoted) = constant.literal.val { if let Some(promoted) = promoted { let check_err = |verifier: &mut TypeVerifier<'a, 'b, 'tcx>, @@ -358,10 +359,23 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> { ); } } - } - if let ty::FnDef(def_id, substs) = constant.literal.ty.kind { - let tcx = self.tcx(); + } else if let Some(static_def_id) = constant.check_static_ptr(tcx) { + let unnormalized_ty = tcx.type_of(static_def_id); + let locations = location.to_locations(); + let normalized_ty = self.cx.normalize(unnormalized_ty, locations); + let literal_ty = constant.literal.ty.builtin_deref(true).unwrap().ty; + if let Err(terr) = self.cx.eq_types( + normalized_ty, + literal_ty, + locations, + ConstraintCategory::Boring, + ) { + span_mirbug!(self, constant, "bad static type {:?} ({:?})", constant, terr); + } + } + + if let ty::FnDef(def_id, substs) = constant.literal.ty.kind { let instantiated_predicates = tcx.predicates_of(def_id).instantiate(tcx, substs); self.cx.normalize_and_prove_instantiated_predicates( instantiated_predicates, diff --git a/src/test/ui/nll/issue-69114-static-mut-ty.rs b/src/test/ui/nll/issue-69114-static-mut-ty.rs new file mode 100644 index 00000000000..ce37da053e3 --- /dev/null +++ b/src/test/ui/nll/issue-69114-static-mut-ty.rs @@ -0,0 +1,30 @@ +// Check that borrowck ensures that `static mut` items have the expected type. + +static FOO: u8 = 42; +static mut BAR: &'static u8 = &FOO; +static mut BAR_ELIDED: &u8 = &FOO; + +fn main() { + unsafe { + println!("{} {}", BAR, BAR_ELIDED); + set_bar(); + set_bar_elided(); + println!("{} {}", BAR, BAR_ELIDED); + } +} + +fn set_bar() { + let n = 42; + unsafe { + BAR = &n; + //~^ ERROR does not live long enough + } +} + +fn set_bar_elided() { + let n = 42; + unsafe { + BAR_ELIDED = &n; + //~^ ERROR does not live long enough + } +} diff --git a/src/test/ui/nll/issue-69114-static-mut-ty.stderr b/src/test/ui/nll/issue-69114-static-mut-ty.stderr new file mode 100644 index 00000000000..5e55cb502ca --- /dev/null +++ b/src/test/ui/nll/issue-69114-static-mut-ty.stderr @@ -0,0 +1,27 @@ +error[E0597]: `n` does not live long enough + --> $DIR/issue-69114-static-mut-ty.rs:19:15 + | +LL | BAR = &n; + | ------^^ + | | | + | | borrowed value does not live long enough + | assignment requires that `n` is borrowed for `'static` +... +LL | } + | - `n` dropped here while still borrowed + +error[E0597]: `n` does not live long enough + --> $DIR/issue-69114-static-mut-ty.rs:27:22 + | +LL | BAR_ELIDED = &n; + | -------------^^ + | | | + | | borrowed value does not live long enough + | assignment requires that `n` is borrowed for `'static` +... +LL | } + | - `n` dropped here while still borrowed + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-69114-static-ty.rs b/src/test/ui/nll/issue-69114-static-ty.rs new file mode 100644 index 00000000000..3318433a1c5 --- /dev/null +++ b/src/test/ui/nll/issue-69114-static-ty.rs @@ -0,0 +1,9 @@ +// Check that borrowck ensures that `static` items have the expected type. + +static FOO: &'static (dyn Fn(&'static u8) + Send + Sync) = &drop; + +fn main() { + let n = 42; + FOO(&n); + //~^ ERROR does not live long enough +} diff --git a/src/test/ui/nll/issue-69114-static-ty.stderr b/src/test/ui/nll/issue-69114-static-ty.stderr new file mode 100644 index 00000000000..0815e74b553 --- /dev/null +++ b/src/test/ui/nll/issue-69114-static-ty.stderr @@ -0,0 +1,15 @@ +error[E0597]: `n` does not live long enough + --> $DIR/issue-69114-static-ty.rs:7:9 + | +LL | FOO(&n); + | ----^^- + | | | + | | borrowed value does not live long enough + | argument requires that `n` is borrowed for `'static` +LL | +LL | } + | - `n` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`.