From 603a75c8eaa8ee168e4333e4fba5eb782ed7192b Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 26 Sep 2015 01:27:39 +0300 Subject: [PATCH] ensure that the types of methods are well-formed By RFC1214: Before calling a fn, we check that its argument and return types are WF. This check takes place after all higher-ranked lifetimes have been instantiated. Checking the argument types ensures that the implied bounds due to argument types are correct. Checking the return type ensures that the resulting type of the call is WF. The previous code only checked the trait-ref, which was not enough in several cases. As this is a soundness fix, it is a [breaking-change]. Fixes #28609 --- src/librustc_typeck/check/method/confirm.rs | 22 +++-- src/librustc_typeck/check/method/mod.rs | 3 + .../wf-method-late-bound-regions.rs | 33 ++++++++ .../wf-misc-methods-issue-28609.rs | 84 +++++++++++++++++++ src/test/compile-fail/wf-static-method.rs | 37 ++++++++ 5 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 src/test/compile-fail/wf-method-late-bound-regions.rs create mode 100644 src/test/compile-fail/wf-misc-methods-issue-28609.rs create mode 100644 src/test/compile-fail/wf-static-method.rs diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs index b0e81803ba7..72131627aa5 100644 --- a/src/librustc_typeck/check/method/confirm.rs +++ b/src/librustc_typeck/check/method/confirm.rs @@ -103,22 +103,23 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> { // Unify the (adjusted) self type with what the method expects. self.unify_receivers(self_ty, method_self_ty); - // Add any trait/regions obligations specified on the method's type parameters. - self.add_obligations(&pick, &all_substs, &method_predicates); - - // Create the final `MethodCallee`. + // Create the method type let method_ty = pick.item.as_opt_method().unwrap(); let fty = self.tcx().mk_fn(None, self.tcx().mk_bare_fn(ty::BareFnTy { sig: ty::Binder(method_sig), unsafety: method_ty.fty.unsafety, abi: method_ty.fty.abi.clone(), })); + + // Add any trait/regions obligations specified on the method's type parameters. + self.add_obligations(fty, &all_substs, &method_predicates); + + // Create the final `MethodCallee`. let callee = ty::MethodCallee { def_id: pick.item.def_id(), ty: fty, substs: self.tcx().mk_substs(all_substs) }; - // If this is an `&mut self` method, bias the receiver // expression towards mutability (this will switch // e.g. `Deref` to `DerefMut` in overloaded derefs and so on). @@ -422,11 +423,11 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> { } fn add_obligations(&mut self, - pick: &probe::Pick<'tcx>, + fty: Ty<'tcx>, all_substs: &subst::Substs<'tcx>, method_predicates: &ty::InstantiatedPredicates<'tcx>) { - debug!("add_obligations: pick={:?} all_substs={:?} method_predicates={:?}", - pick, + debug!("add_obligations: fty={:?} all_substs={:?} method_predicates={:?}", + fty, all_substs, method_predicates); @@ -439,6 +440,11 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> { self.fcx.add_wf_bounds( all_substs, self.call_expr); + + // the function type must also be well-formed (this is not + // implied by the substs being well-formed because of inherent + // impls and late-bound regions - see issue #28609). + self.fcx.register_wf_obligation(fty, self.span, traits::MiscObligation); } /////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index edf1cc9b7ef..e0ad51b4ea1 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -255,6 +255,9 @@ pub fn lookup_in_trait_adjusted<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, traits::ObligationCause::misc(span, fcx.body_id), &method_bounds); + // Also register an obligation for the method type being well-formed. + fcx.register_wf_obligation(fty, span, traits::MiscObligation); + // FIXME(#18653) -- Try to resolve obligations, giving us more // typing information, which can sometimes be needed to avoid // pathological region inference failures. diff --git a/src/test/compile-fail/wf-method-late-bound-regions.rs b/src/test/compile-fail/wf-method-late-bound-regions.rs new file mode 100644 index 00000000000..b9d292fd156 --- /dev/null +++ b/src/test/compile-fail/wf-method-late-bound-regions.rs @@ -0,0 +1,33 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// A method's receiver must be well-formed, even if it has late-bound regions. +// Because of this, a method's substs being well-formed does not imply that +// the method's implied bounds are met. + +struct Foo<'b>(Option<&'b ()>); + +trait Bar<'b> { + fn xmute<'a>(&'a self, u: &'b u32) -> &'a u32; +} + +impl<'b> Bar<'b> for Foo<'b> { + fn xmute<'a>(&'a self, u: &'b u32) -> &'a u32 { u } +} + +fn main() { + let f = Foo(None); + let f2 = f; + let dangling = { + let pointer = Box::new(42); + f2.xmute(&pointer) //~ ERROR `pointer` does not live long enough + }; + println!("{}", dangling); +} diff --git a/src/test/compile-fail/wf-misc-methods-issue-28609.rs b/src/test/compile-fail/wf-misc-methods-issue-28609.rs new file mode 100644 index 00000000000..055c86a03a7 --- /dev/null +++ b/src/test/compile-fail/wf-misc-methods-issue-28609.rs @@ -0,0 +1,84 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// check that misc. method calls are well-formed + +use std::marker::PhantomData; +use std::ops::{Deref, Shl}; + +#[derive(Copy, Clone)] +struct S<'a, 'b: 'a> { + marker: PhantomData<&'a &'b ()>, + bomb: Option<&'b u32> +} + +type S2<'a> = S<'a, 'a>; + +impl<'a, 'b> S<'a, 'b> { + fn transmute_inherent(&self, a: &'b u32) -> &'a u32 { + a + } +} + +fn return_dangling_pointer_inherent(s: S2) -> &u32 { + let s = s; + s.transmute_inherent(&mut 42) //~ ERROR does not live long enough +} + +impl<'a, 'b> Deref for S<'a, 'b> { + type Target = &'a u32; + fn deref(&self) -> &&'a u32 { + self.bomb.as_ref().unwrap() + } +} + +fn return_dangling_pointer_coerce(s: S2) -> &u32 { + let four = 4; + let mut s = s; + s.bomb = Some(&four); //~ ERROR does not live long enough + &s +} + +fn return_dangling_pointer_unary_op(s: S2) -> &u32 { + let four = 4; + let mut s = s; + s.bomb = Some(&four); //~ ERROR does not live long enough + &*s +} + +impl<'a, 'b> Shl<&'b u32> for S<'a, 'b> { + type Output = &'a u32; + fn shl(self, t: &'b u32) -> &'a u32 { t } +} + +fn return_dangling_pointer_binary_op(s: S2) -> &u32 { + let s = s; + s << &mut 3 //~ ERROR does not live long enough +} + +fn return_dangling_pointer_method(s: S2) -> &u32 { + let s = s; + s.shl(&mut 3) //~ ERROR does not live long enough +} + +fn return_dangling_pointer_ufcs(s: S2) -> &u32 { + let s = s; + S2::shl(s, &mut 3) //~ ERROR does not live long enough +} + +fn main() { + let s = S { marker: PhantomData, bomb: None }; + let _inherent_dp = return_dangling_pointer_inherent(s); + let _coerce_dp = return_dangling_pointer_coerce(s); + let _unary_dp = return_dangling_pointer_unary_op(s); + let _binary_dp = return_dangling_pointer_binary_op(s); + let _method_dp = return_dangling_pointer_method(s); + let _ufcs_dp = return_dangling_pointer_ufcs(s); +} diff --git a/src/test/compile-fail/wf-static-method.rs b/src/test/compile-fail/wf-static-method.rs new file mode 100644 index 00000000000..8f9b0cb80ba --- /dev/null +++ b/src/test/compile-fail/wf-static-method.rs @@ -0,0 +1,37 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// check that static methods don't get to assume `Self` is well-formed + +trait Foo<'a, 'b>: Sized { + fn make_me() -> Self { loop {} } + fn static_evil(u: &'a u32) -> &'b u32; +} + +struct Evil<'a, 'b: 'a>(Option<&'a &'b ()>); + +impl<'a, 'b> Foo<'a, 'b> for Evil<'a, 'b> { + fn make_me() -> Self { Evil(None) } + fn static_evil(u: &'a u32) -> &'b u32 { + u //~ ERROR cannot infer an appropriate lifetime + } +} + +struct IndirectEvil<'a, 'b: 'a>(Option<&'a &'b ()>); + +impl<'a, 'b> Foo<'a, 'b> for IndirectEvil<'a, 'b> { + fn make_me() -> Self { IndirectEvil(None) } + fn static_evil(u: &'a u32) -> &'b u32 { + let me = Self::make_me(); //~ ERROR lifetime bound not satisfied + loop {} // (`me` could be used for the lifetime transmute). + } +} + +fn main() {}