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

Auto merge of #34198 - eddyb:you're-a-bad-transmute-and-you-should-feel-bad, r=nikomatsakis

Browse Source 
Make transmuting from fn item types to pointer-sized types a hard error.

Closes #19925 by removing the future compatibility lint and the associated workarounds.
This is a `[breaking-change]` if you `transmute` from a function item without casting first.
For more information on how to fix your code, see https://github.com/rust-lang/rust/issues/19925.
This commit is contained in:
bors 2017-03-01 10:03:44 +00:00
commit 691eba1358
8 changed files with 153 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
src/librustc/diagnostics.rs
View File

@ -1725,6 +1725,68 @@ If you want to get command-line arguments, use `std::env::args`. To exit with a
specified exit code, use `std::process::exit`.
"##,
E0591: r##"
Per [RFC 401][rfc401], if you have a function declaration `foo`:
```rust,ignore
// For the purposes of this explanation, all of these
// different kinds of `fn` declarations are equivalent:
fn foo(x: i32) { ... }
extern "C" fn foo(x: i32);
impl i32 { fn foo(x: self) { ... } }
```
the type of `foo` is **not** `fn(i32)`, as one might expect.
Rather, it is a unique, zero-sized marker type written here as `typeof(foo)`.
However, `typeof(foo)` can be _coerced_ to a function pointer `fn(i32)`,
so you rarely notice this:
```rust,ignore
let x: fn(i32) = foo; // OK, coerces
```
The reason that this matter is that the type `fn(i32)` is not specific to
any particular function: it's a function _pointer_. So calling `x()` results
in a virtual call, whereas `foo()` is statically dispatched, because the type
of `foo` tells us precisely what function is being called.
As noted above, coercions mean that most code doesn't have to be
concerned with this distinction. However, you can tell the difference
when using **transmute** to convert a fn item into a fn pointer.
This is sometimes done as part of an FFI:
```rust,ignore
extern "C" fn foo(userdata: Box<i32>) {
...
}
let f: extern "C" fn(*mut i32) = transmute(foo);
callback(f);
```
Here, transmute is being used to convert the types of the fn arguments.
This pattern is incorrect because, because the type of `foo` is a function
**item** (`typeof(foo)`), which is zero-sized, and the target type (`fn()`)
is a function pointer, which is not zero-sized.
This pattern should be rewritten. There are a few possible ways to do this:
- change the original fn declaration to match the expected signature,
and do the cast in the fn body (the prefered option)
- cast the fn item fo a fn pointer before calling transmute, as shown here:
- `let f: extern "C" fn(*mut i32) = transmute(foo as extern "C" fn(_))`
- `let f: extern "C" fn(*mut i32) = transmute(foo as usize) /* works too */`
The same applies to transmutes to `*mut fn()`, which were observedin practice.
Note though that use of this type is generally incorrect.
The intention is typically to describe a function pointer, but just `fn()`
alone suffices for that. `*mut fn()` is a pointer to a fn pointer.
(Since these values are typically just passed to C code, however, this rarely
makes a difference in practice.)
[rfc401]: https://github.com/rust-lang/rfcs/blob/master/text/0401-coercions.md
"##,
}

7
src/librustc/lint/builtin.rs
View File

@ -155,12 +155,6 @@ declare_lint! {
"uses of #[derive] with raw pointers are rarely correct"
}
declare_lint! {
pub TRANSMUTE_FROM_FN_ITEM_TYPES,
Deny,
"transmute from function item type to pointer-sized type erroneously allowed"
}
declare_lint! {
pub HR_LIFETIME_IN_ASSOC_TYPE,
Deny,
@ -279,7 +273,6 @@ impl LintPass for HardwiredLints {
ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN,
CONST_ERR,
RAW_POINTER_DERIVE,
TRANSMUTE_FROM_FN_ITEM_TYPES,
OVERLAPPING_INHERENT_IMPLS,
RENAMED_AND_REMOVED_LINTS,
SUPER_OR_SELF_IN_GLOBAL_PATH,

48
src/librustc/middle/intrinsicck.rs
View File

@ -17,7 +17,6 @@ use ty::{self, Ty, TyCtxt};
use ty::layout::{LayoutError, Pointer, SizeSkeleton};
use syntax::abi::Abi::RustIntrinsic;
use syntax::ast;
use syntax_pos::Span;
use hir::intravisit::{self, Visitor, NestedVisitorMap};
use hir;
@ -37,6 +36,35 @@ struct ExprVisitor<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>
}
/// If the type is `Option<T>`, it will return `T`, otherwise
/// the type itself. Works on most `Option`-like types.
fn unpack_option_like<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
ty: Ty<'tcx>)
-> Ty<'tcx> {
let (def, substs) = match ty.sty {
ty::TyAdt(def, substs) => (def, substs),
_ => return ty
};
if def.variants.len() == 2 && !def.repr.c && def.repr.int.is_none() {
let data_idx;
if def.variants[0].fields.is_empty() {
data_idx = 1;
} else if def.variants[1].fields.is_empty() {
data_idx = 0;
} else {
return ty;
}
if def.variants[data_idx].fields.len() == 1 {
return def.variants[data_idx].fields[0].ty(tcx, substs);
}
}
ty
}
impl<'a, 'gcx, 'tcx> ExprVisitor<'a, 'gcx, 'tcx> {
fn def_id_is_transmute(&self, def_id: DefId) -> bool {
let intrinsic = match self.infcx.tcx.item_type(def_id).sty {
@ -46,7 +74,7 @@ impl<'a, 'gcx, 'tcx> ExprVisitor<'a, 'gcx, 'tcx> {
intrinsic && self.infcx.tcx.item_name(def_id) == "transmute"
}
fn check_transmute(&self, span: Span, from: Ty<'gcx>, to: Ty<'gcx>, id: ast::NodeId) {
fn check_transmute(&self, span: Span, from: Ty<'gcx>, to: Ty<'gcx>) {
let sk_from = SizeSkeleton::compute(from, self.infcx);
let sk_to = SizeSkeleton::compute(to, self.infcx);
@ -56,15 +84,17 @@ impl<'a, 'gcx, 'tcx> ExprVisitor<'a, 'gcx, 'tcx> {
return;
}
// Special-case transmutting from `typeof(function)` and
// `Option<typeof(function)>` to present a clearer error.
let from = unpack_option_like(self.infcx.tcx.global_tcx(), from);
match (&from.sty, sk_to) {
(&ty::TyFnDef(..), SizeSkeleton::Known(size_to))
if size_to == Pointer.size(&self.infcx.tcx.data_layout) => {
// FIXME #19925 Remove this warning after a release cycle.
let msg = format!("`{}` is now zero-sized and has to be cast \
to a pointer before transmuting to `{}`",
from, to);
self.infcx.tcx.sess.add_lint(
::lint::builtin::TRANSMUTE_FROM_FN_ITEM_TYPES, id, span, msg);
struct_span_err!(self.infcx.tcx.sess, span, E0591,
"`{}` is zero-sized and can't be transmuted to `{}`",
from, to)
.span_note(span, &format!("cast with `as` to a pointer instead"))
.emit();
return;
}
_ => {}
@ -140,7 +170,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for ExprVisitor<'a, 'gcx, 'tcx> {
ty::TyFnDef(.., sig) if sig.abi() == RustIntrinsic => {
let from = sig.inputs().skip_binder()[0];
let to = *sig.output().skip_binder();
self.check_transmute(expr.span, from, to, expr.id);
self.check_transmute(expr.span, from, to);
}
_ => {
span_bug!(expr.span, "transmute wasn't a bare fn?!");

6
src/librustc_lint/lib.rs
View File

@ -195,10 +195,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
id: LintId::of(SUPER_OR_SELF_IN_GLOBAL_PATH),
reference: "issue #36888 <https://github.com/rust-lang/rust/issues/36888>",
},
FutureIncompatibleInfo {
id: LintId::of(TRANSMUTE_FROM_FN_ITEM_TYPES),
reference: "issue #19925 <https://github.com/rust-lang/rust/issues/19925>",
},
FutureIncompatibleInfo {
id: LintId::of(OVERLAPPING_INHERENT_IMPLS),
reference: "issue #36889 <https://github.com/rust-lang/rust/issues/36889>",
@ -264,4 +260,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
store.register_removed("raw_pointer_deriving",
"using derive with raw pointers is ok");
store.register_removed("drop_with_repr_extern", "drop flags have been removed");
store.register_removed("transmute_from_fn_item_types",
"always cast functions before transmuting them");
}

21
src/librustc_trans/mir/block.rs
View File

@ -21,7 +21,7 @@ use builder::Builder;
use common::{self, Funclet};
use common::{C_bool, C_str_slice, C_struct, C_u32, C_undef};
use consts;
use machine::{llalign_of_min, llbitsize_of_real};
use machine::llalign_of_min;
use meth;
use type_of::{self, align_of};
use glue;
@ -869,24 +869,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
fn trans_transmute_into(&mut self, bcx: &Builder<'a, 'tcx>,
src: &mir::Operand<'tcx>,
dst: &LvalueRef<'tcx>) {
let mut val = self.trans_operand(bcx, src);
if let ty::TyFnDef(def_id, substs, _) = val.ty.sty {
let llouttype = type_of::type_of(bcx.ccx, dst.ty.to_ty(bcx.tcx()));
let out_type_size = llbitsize_of_real(bcx.ccx, llouttype);
if out_type_size != 0 {
// FIXME #19925 Remove this hack after a release cycle.
let f = Callee::def(bcx.ccx, def_id, substs);
let ty = match f.ty.sty {
ty::TyFnDef(.., f) => bcx.tcx().mk_fn_ptr(f),
_ => f.ty
};
val = OperandRef {
val: Immediate(f.reify(bcx.ccx)),
ty: ty
};
}
}
let val = self.trans_operand(bcx, src);
let llty = type_of::type_of(bcx.ccx, val.ty);
let cast_ptr = bcx.pointercast(dst.llval, llty.ptr_to());
let in_type = val.ty;

49
src/test/compile-fail/transmute-from-fn-item-types-error.rs
View File

@ -10,14 +10,61 @@
use std::mem;
unsafe fn foo() -> (isize, *const (), Option<fn()>) {
let i = mem::transmute(bar);
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
let p = mem::transmute(foo);
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
let of = mem::transmute(main);
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
(i, p, of)
}
unsafe fn bar() {
// Error, still, if the resulting type is not pointer-sized.
// Error as usual if the resulting type is not pointer-sized.
mem::transmute::<_, u8>(main);
//~^ ERROR transmute called with differently sized types
//~^^ NOTE transmuting between 0 bits and 8 bits
mem::transmute::<_, *mut ()>(foo);
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
mem::transmute::<_, fn()>(bar);
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
// No error if a coercion would otherwise occur.
mem::transmute::<fn(), usize>(main);
}
unsafe fn baz() {
mem::transmute::<_, *mut ()>(Some(foo));
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
mem::transmute::<_, fn()>(Some(bar));
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
mem::transmute::<_, Option<fn()>>(Some(baz));
//~^ ERROR is zero-sized and can't be transmuted
//~^^ NOTE cast with `as` to a pointer instead
// No error if a coercion would otherwise occur.
mem::transmute::<Option<fn()>, usize>(Some(main));
}
fn main() {
unsafe {
foo();
bar();
baz();
}
}

49
src/test/compile-fail/transmute-from-fn-item-types-lint.rs
View File

@ -1,49 +0,0 @@
// Copyright 2016 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.
#![deny(transmute_from_fn_item_types)]
use std::mem;
unsafe fn foo() -> (isize, *const (), Option<fn()>) {
let i = mem::transmute(bar);
//~^ ERROR is now zero-sized and has to be cast to a pointer before transmuting
//~^^ WARNING was previously accepted
let p = mem::transmute(foo);
//~^ ERROR is now zero-sized and has to be cast to a pointer before transmuting
//~^^ WARNING was previously accepted
let of = mem::transmute(main);
//~^ ERROR is now zero-sized and has to be cast to a pointer before transmuting
//~^^ WARNING was previously accepted
(i, p, of)
}
unsafe fn bar() {
mem::transmute::<_, *mut ()>(foo);
//~^ ERROR is now zero-sized and has to be cast to a pointer before transmuting
//~^^ WARNING was previously accepted
mem::transmute::<_, fn()>(bar);
//~^ ERROR is now zero-sized and has to be cast to a pointer before transmuting
//~^^ WARNING was previously accepted
// No error if a coercion would otherwise occur.
mem::transmute::<fn(), usize>(main);
}
fn main() {
unsafe {
foo();
bar();
}
}

27
src/test/run-pass/transmute-from-fn-item-types.rs
View File

@ -1,27 +0,0 @@
// Copyright 2016 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.
#![allow(transmute_from_fn_item_types)]
use std::mem;
fn main() {
unsafe {
let u = mem::transmute(main);
let p = mem::transmute(main);
let f = mem::transmute(main);
let tuple: (usize, *mut (), fn()) = (u, p, f);
assert_eq!(mem::transmute::<_, [usize; 3]>(tuple), [main as usize; 3]);
mem::transmute::<_, usize>(main);
mem::transmute::<_, *mut ()>(main);
mem::transmute::<_, fn()>(main);
}
}