From 0f91cb8012e92de3e540cbb0c24a99612bbfdeeb Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 12 Jun 2014 14:08:44 -0700 Subject: [PATCH] librustc: Forbid `transmute` from being called on types whose size is only known post-monomorphization, and report `transmute` errors before the code is generated for that `transmute`. This can break code that looked like: unsafe fn f(x: T) { let y: int = transmute(x); } Change such code to take a type parameter that has the same size as the type being transmuted to. Closes #12898. [breaking-change] --- src/libcore/intrinsics.rs | 14 ++ src/libcore/mem.rs | 25 +-- src/librustc/driver/driver.rs | 3 + src/librustc/lib.rs | 1 + src/librustc/middle/intrinsicck.rs | 155 ++++++++++++++++++ src/librustc/middle/trans/base.rs | 6 + src/librustc/middle/trans/intrinsic.rs | 61 ++++++- src/librustc/middle/trans/type_of.rs | 4 +- src/librustc/middle/ty.rs | 20 ++- src/librustc/plugin/load.rs | 6 +- src/librustdoc/plugins.rs | 7 +- src/libstd/dynamic_lib.rs | 2 +- .../compile-fail/transmute-different-sizes.rs | 27 +++ .../compile-fail/transmute-type-parameters.rs | 48 ++++++ 14 files changed, 340 insertions(+), 39 deletions(-) create mode 100644 src/librustc/middle/intrinsicck.rs create mode 100644 src/test/compile-fail/transmute-different-sizes.rs create mode 100644 src/test/compile-fail/transmute-type-parameters.rs diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index d61416a68e0d0..33f2a49d6d560 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -307,6 +307,20 @@ extern "rust-intrinsic" { /// `forget` is unsafe because the caller is responsible for /// ensuring the argument is deallocated already. pub fn forget(_: T) -> (); + + /// Unsafely transforms a value of one type into a value of another type. + /// + /// Both types must have the same size and alignment, and this guarantee + /// is enforced at compile-time. + /// + /// # Example + /// + /// ```rust + /// use std::mem; + /// + /// let v: &[u8] = unsafe { mem::transmute("L") }; + /// assert!(v == [76u8]); + /// ``` pub fn transmute(e: T) -> U; /// Returns `true` if a type requires drop glue. diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 8933c95350d59..237efcd0096d0 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -17,6 +17,8 @@ use ptr; use intrinsics; use intrinsics::{bswap16, bswap32, bswap64}; +pub use intrinsics::transmute; + /// Returns the size of a type in bytes. #[inline] #[stable] @@ -412,29 +414,6 @@ pub fn drop(_x: T) { } #[stable] pub unsafe fn forget(thing: T) { intrinsics::forget(thing) } -/// Unsafely transforms a value of one type into a value of another type. -/// -/// Both types must have the same size and alignment, and this guarantee is -/// enforced at compile-time. -/// -/// # Example -/// -/// ```rust -/// use std::mem; -/// -/// let v: &[u8] = unsafe { mem::transmute("L") }; -/// assert!(v == [76u8]); -/// ``` -#[inline] -#[unstable = "this function will be modified to reject invocations of it which \ - cannot statically prove that T and U are the same size. For \ - example, this function, as written today, will be rejected in \ - the future because the size of T and U cannot be statically \ - known to be the same"] -pub unsafe fn transmute(thing: T) -> U { - intrinsics::transmute(thing) -} - /// Interprets `src` as `&U`, and then reads `src` without moving the contained /// value. /// diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index d703ece307ffb..2a3472d761de6 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -332,6 +332,9 @@ pub fn phase_3_run_analysis_passes(sess: Session, time(time_passes, "privacy checking", maps, |(a, b)| middle::privacy::check_crate(&ty_cx, &exp_map2, a, b, krate)); + time(time_passes, "intrinsic checking", (), |_| + middle::intrinsicck::check_crate(&ty_cx, krate)); + time(time_passes, "effect checking", (), |_| middle::effect::check_crate(&ty_cx, krate)); diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index 297d55edec840..8a18a733ded9d 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -84,6 +84,7 @@ pub mod middle { pub mod expr_use_visitor; pub mod dependency_format; pub mod weak_lang_items; + pub mod intrinsicck; } pub mod front { diff --git a/src/librustc/middle/intrinsicck.rs b/src/librustc/middle/intrinsicck.rs new file mode 100644 index 0000000000000..93913f8427111 --- /dev/null +++ b/src/librustc/middle/intrinsicck.rs @@ -0,0 +1,155 @@ +// Copyright 2012-2014 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. + +use metadata::csearch; +use middle::def::DefFn; +use middle::subst::Subst; +use middle::ty::{TransmuteRestriction, ctxt, ty_bare_fn}; +use middle::ty; + +use syntax::abi::RustIntrinsic; +use syntax::ast::DefId; +use syntax::ast; +use syntax::ast_map::NodeForeignItem; +use syntax::codemap::Span; +use syntax::parse::token; +use syntax::visit::Visitor; +use syntax::visit; + +fn type_size_is_affected_by_type_parameters(tcx: &ty::ctxt, typ: ty::t) + -> bool { + let mut result = false; + ty::maybe_walk_ty(typ, |typ| { + match ty::get(typ).sty { + ty::ty_box(_) | ty::ty_uniq(_) | ty::ty_ptr(_) | + ty::ty_rptr(..) | ty::ty_bare_fn(..) | ty::ty_closure(..) => { + false + } + ty::ty_param(_) => { + result = true; + // No need to continue; we now know the result. + false + } + ty::ty_enum(did, ref substs) => { + for enum_variant in (*ty::enum_variants(tcx, did)).iter() { + for argument_type in enum_variant.args.iter() { + let argument_type = argument_type.subst(tcx, substs); + result = result || + type_size_is_affected_by_type_parameters( + tcx, + argument_type); + } + } + + // Don't traverse substitutions. + false + } + ty::ty_struct(did, ref substs) => { + for field in ty::struct_fields(tcx, did, substs).iter() { + result = result || + type_size_is_affected_by_type_parameters(tcx, + field.mt.ty); + } + + // Don't traverse substitutions. + false + } + _ => true, + } + }); + result +} + +struct IntrinsicCheckingVisitor<'a> { + tcx: &'a ctxt, +} + +impl<'a> IntrinsicCheckingVisitor<'a> { + fn def_id_is_transmute(&self, def_id: DefId) -> bool { + if def_id.krate == ast::LOCAL_CRATE { + match self.tcx.map.get(def_id.node) { + NodeForeignItem(ref item) => { + token::get_ident(item.ident) == + token::intern_and_get_ident("transmute") + } + _ => false, + } + } else { + match csearch::get_item_path(self.tcx, def_id).last() { + None => false, + Some(ref last) => { + token::get_name(last.name()) == + token::intern_and_get_ident("transmute") + } + } + } + } + + fn check_transmute(&self, span: Span, from: ty::t, to: ty::t) { + if type_size_is_affected_by_type_parameters(self.tcx, from) { + self.tcx.sess.span_err(span, + "cannot transmute from a type that \ + contains type parameters"); + } + if type_size_is_affected_by_type_parameters(self.tcx, to) { + self.tcx.sess.span_err(span, + "cannot transmute to a type that contains \ + type parameters"); + } + + let restriction = TransmuteRestriction { + span: span, + from: from, + to: to, + }; + self.tcx.transmute_restrictions.borrow_mut().push(restriction); + } +} + +impl<'a> Visitor<()> for IntrinsicCheckingVisitor<'a> { + fn visit_expr(&mut self, expr: &ast::Expr, (): ()) { + match expr.node { + ast::ExprPath(..) => { + match ty::resolve_expr(self.tcx, expr) { + DefFn(did, _) if self.def_id_is_transmute(did) => { + let typ = ty::node_id_to_type(self.tcx, expr.id); + match ty::get(typ).sty { + ty_bare_fn(ref bare_fn_ty) + if bare_fn_ty.abi == RustIntrinsic => { + let from = *bare_fn_ty.sig.inputs.get(0); + let to = bare_fn_ty.sig.output; + self.check_transmute(expr.span, from, to); + } + _ => { + self.tcx + .sess + .span_bug(expr.span, + "transmute wasn't a bare fn?!"); + } + } + } + _ => {} + } + } + _ => {} + } + + visit::walk_expr(self, expr, ()); + } +} + +pub fn check_crate(tcx: &ctxt, krate: &ast::Crate) { + let mut visitor = IntrinsicCheckingVisitor { + tcx: tcx, + }; + + visit::walk_crate(&mut visitor, krate, ()); +} + diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 9cebc79171f5f..2a51f1bc96e2a 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -59,6 +59,7 @@ use middle::trans::expr; use middle::trans::foreign; use middle::trans::glue; use middle::trans::inline; +use middle::trans::intrinsic; use middle::trans::machine; use middle::trans::machine::{llalign_of_min, llsize_of, llsize_of_real}; use middle::trans::meth; @@ -2329,6 +2330,11 @@ pub fn trans_crate(krate: ast::Crate, let ccx = CrateContext::new(llmod_id.as_slice(), tcx, exp_map2, Sha256::new(), link_meta, reachable); + + // First, verify intrinsics. + intrinsic::check_intrinsics(&ccx); + + // Next, translate the module. { let _icx = push_ctxt("text"); trans_mod(&ccx, &krate.module); diff --git a/src/librustc/middle/trans/intrinsic.rs b/src/librustc/middle/trans/intrinsic.rs index 0719288bb0285..f77b41764d584 100644 --- a/src/librustc/middle/trans/intrinsic.rs +++ b/src/librustc/middle/trans/intrinsic.rs @@ -389,14 +389,23 @@ pub fn trans_intrinsic(ccx: &CrateContext, ast_map::NodeExpr(e) => e.span, _ => fail!("transmute has non-expr arg"), }; - ccx.sess().span_fatal(sp, + ccx.sess().span_bug(sp, format!("transmute called on types with different sizes: \ - {intype} ({insize, plural, =1{# bit} other{# bits}}) to \ - {outtype} ({outsize, plural, =1{# bit} other{# bits}})", - intype = ty_to_str(ccx.tcx(), in_type), - insize = in_type_size as uint, - outtype = ty_to_str(ccx.tcx(), out_type), - outsize = out_type_size as uint).as_slice()); + {} ({} bit{}) to {} ({} bit{})", + ty_to_str(ccx.tcx(), in_type), + in_type_size as uint, + if in_type_size == 1 { + "" + } else { + "s" + }, + ty_to_str(ccx.tcx(), out_type), + out_type_size as uint, + if out_type_size == 1 { + "" + } else { + "s" + }).as_slice()); } if !return_type_is_void(ccx, out_type) { @@ -554,3 +563,41 @@ pub fn trans_intrinsic(ccx: &CrateContext, } fcx.cleanup(); } + +/// Performs late verification that intrinsics are used correctly. At present, +/// the only intrinsic that needs such verification is `transmute`. +pub fn check_intrinsics(ccx: &CrateContext) { + for transmute_restriction in ccx.tcx + .transmute_restrictions + .borrow() + .iter() { + let llfromtype = type_of::sizing_type_of(ccx, + transmute_restriction.from); + let lltotype = type_of::sizing_type_of(ccx, + transmute_restriction.to); + let from_type_size = machine::llbitsize_of_real(ccx, llfromtype); + let to_type_size = machine::llbitsize_of_real(ccx, lltotype); + if from_type_size != to_type_size { + ccx.sess() + .span_err(transmute_restriction.span, + format!("transmute called on types with different sizes: \ + {} ({} bit{}) to {} ({} bit{})", + ty_to_str(ccx.tcx(), transmute_restriction.from), + from_type_size as uint, + if from_type_size == 1 { + "" + } else { + "s" + }, + ty_to_str(ccx.tcx(), transmute_restriction.to), + to_type_size as uint, + if to_type_size == 1 { + "" + } else { + "s" + }).as_slice()); + } + } + ccx.sess().abort_if_errors(); +} + diff --git a/src/librustc/middle/trans/type_of.rs b/src/librustc/middle/trans/type_of.rs index bf5bedd98e884..2229a68509024 100644 --- a/src/librustc/middle/trans/type_of.rs +++ b/src/librustc/middle/trans/type_of.rs @@ -152,8 +152,8 @@ pub fn sizing_type_of(cx: &CrateContext, t: ty::t) -> Type { } } - ty::ty_self(_) | ty::ty_infer(..) | ty::ty_param(..) | - ty::ty_err(..) | ty::ty_vec(_, None) | ty::ty_str => { + ty::ty_self(_) | ty::ty_infer(..) | ty::ty_err(..) | + ty::ty_param(..) | ty::ty_vec(_, None) | ty::ty_str => { cx.sess().bug(format!("fictitious type {:?} in sizing_type_of()", ty::get(t).sty).as_slice()) } diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 99c337946ae3b..a3dfcc171fe34 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -237,6 +237,17 @@ pub enum AutoRef { AutoBorrowObj(Region, ast::Mutability), } +/// A restriction that certain types must be the same size. The use of +/// `transmute` gives rise to these restrictions. +pub struct TransmuteRestriction { + /// The span from whence the restriction comes. + pub span: Span, + /// The type being transmuted from. + pub from: t, + /// The type being transmuted to. + pub to: t, +} + /// The data structure to keep track of all the information that typechecker /// generates so that so that it can be reused and doesn't have to be redone /// later on. @@ -359,6 +370,11 @@ pub struct ctxt { pub node_lint_levels: RefCell>, + + /// The types that must be asserted to be the same size for `transmute` + /// to be valid. We gather up these restrictions in the intrinsicck pass + /// and check them in trans. + pub transmute_restrictions: RefCell>, } pub enum tbox_flag { @@ -1108,6 +1124,7 @@ pub fn mk_ctxt(s: Session, vtable_map: RefCell::new(FnvHashMap::new()), dependency_formats: RefCell::new(HashMap::new()), node_lint_levels: RefCell::new(HashMap::new()), + transmute_restrictions: RefCell::new(Vec::new()), } } @@ -2689,8 +2706,7 @@ pub fn pat_ty(cx: &ctxt, pat: &ast::Pat) -> t { // // NB (2): This type doesn't provide type parameter substitutions; e.g. if you // ask for the type of "id" in "id(3)", it will return "fn(&int) -> int" -// instead of "fn(t) -> T with T = int". If this isn't what you want, see -// expr_ty_params_and_ty() below. +// instead of "fn(t) -> T with T = int". pub fn expr_ty(cx: &ctxt, expr: &ast::Expr) -> t { return node_id_to_type(cx, expr.id); } diff --git a/src/librustc/plugin/load.rs b/src/librustc/plugin/load.rs index 97ffcf279ae73..7d74b8c729652 100644 --- a/src/librustc/plugin/load.rs +++ b/src/librustc/plugin/load.rs @@ -126,9 +126,11 @@ impl<'a> PluginLoader<'a> { }; unsafe { - let registrar: PluginRegistrarFun = + let registrar = match lib.symbol(symbol.as_slice()) { - Ok(registrar) => registrar, + Ok(registrar) => { + mem::transmute::<*u8,PluginRegistrarFun>(registrar) + } // again fatal if we can't register macros Err(err) => self.sess.span_fatal(vi.span, err.as_slice()) }; diff --git a/src/librustdoc/plugins.rs b/src/librustdoc/plugins.rs index fee1d63a274d5..e5bced8038baf 100644 --- a/src/librustdoc/plugins.rs +++ b/src/librustdoc/plugins.rs @@ -12,6 +12,7 @@ use clean; use dl = std::dynamic_lib; use serialize::json; +use std::mem; use std::string::String; pub type PluginJson = Option<(String, json::Json)>; @@ -45,9 +46,11 @@ impl PluginManager { let x = self.prefix.join(libname(name)); let lib_result = dl::DynamicLibrary::open(Some(&x)); let lib = lib_result.unwrap(); - let plugin = unsafe { lib.symbol("rustdoc_plugin_entrypoint") }.unwrap(); + unsafe { + let plugin = lib.symbol("rustdoc_plugin_entrypoint").unwrap(); + self.callbacks.push(mem::transmute::<*u8,PluginCallback>(plugin)); + } self.dylibs.push(lib); - self.callbacks.push(plugin); } /// Load a normal Rust function as a plugin. diff --git a/src/libstd/dynamic_lib.rs b/src/libstd/dynamic_lib.rs index fa6efc8a4b18a..61f7997071e8b 100644 --- a/src/libstd/dynamic_lib.rs +++ b/src/libstd/dynamic_lib.rs @@ -134,7 +134,7 @@ impl DynamicLibrary { } /// Access the value at the symbol of the dynamic library - pub unsafe fn symbol(&self, symbol: &str) -> Result { + pub unsafe fn symbol(&self, symbol: &str) -> Result<*T, String> { // This function should have a lifetime constraint of 'a on // T but that feature is still unimplemented diff --git a/src/test/compile-fail/transmute-different-sizes.rs b/src/test/compile-fail/transmute-different-sizes.rs new file mode 100644 index 0000000000000..5ea1c496c769f --- /dev/null +++ b/src/test/compile-fail/transmute-different-sizes.rs @@ -0,0 +1,27 @@ +// Copyright 2012 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. + +// Tests that `transmute` cannot be called on types of different size. + +use std::mem::transmute; + +unsafe fn f() { + let _: i8 = transmute(16i16); + //~^ ERROR transmute called on types with different sizes +} + +unsafe fn g(x: &T) { + let _: i8 = transmute(x); + //~^ ERROR transmute called on types with different sizes +} + +fn main() {} + + diff --git a/src/test/compile-fail/transmute-type-parameters.rs b/src/test/compile-fail/transmute-type-parameters.rs new file mode 100644 index 0000000000000..53391a0e8947b --- /dev/null +++ b/src/test/compile-fail/transmute-type-parameters.rs @@ -0,0 +1,48 @@ +// Copyright 2012 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. + +// Tests that `transmute` cannot be called on type parameters. + +use std::mem::transmute; + +unsafe fn f(x: T) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +unsafe fn g(x: (T, int)) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +unsafe fn h(x: [T, ..10]) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +struct Bad { + f: T, +} + +unsafe fn i(x: Bad) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +enum Worse { + A(T), + B, +} + +unsafe fn j(x: Worse) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +unsafe fn k(x: Option) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +fn main() {}