Skip to content

Commit 8767e97

Browse files
borsbors
committed
Auto merge of #24392 - seanmonstar:lint-transmute-mut, r=alexcrichton
The [UnsafeCell documentation says it is undefined behavior](http://doc.rust-lang.org/nightly/std/cell/struct.UnsafeCell.html), so people shouldn't do it. This happened to catch one case in libstd that was doing this, and I switched that to use an UnsafeCell internally. Closes #13146
2 parents 6c9f840 + 5624cfb commit 8767e97

File tree

6 files changed

+110
-15
lines changed

6 files changed

+110
-15
lines changed

‎src/librustc_lint/builtin.rs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2121,6 +2121,72 @@ impl LintPass for InvalidNoMangleItems {
21212121
}
21222122
}
21232123

2124+
#[derive(Clone, Copy)]
2125+
pub struct MutableTransmutes;
2126+
2127+
declare_lint! {
2128+
MUTABLE_TRANSMUTES,
2129+
Deny,
2130+
"mutating transmuted &mut T from &T may cause undefined behavior"
2131+
}
2132+
2133+
impl LintPass for MutableTransmutes {
2134+
fn get_lints(&self) -> LintArray {
2135+
lint_array!(MUTABLE_TRANSMUTES)
2136+
}
2137+
2138+
fn check_expr(&mut self, cx: &Context, expr: &ast::Expr) {
2139+
use syntax::ast::DefId;
2140+
use syntax::abi::RustIntrinsic;
2141+
let msg = "mutating transmuted &mut T from &T may cause undefined behavior,\
2142+
consider instead using an UnsafeCell";
2143+
match get_transmute_from_to(cx, expr) {
2144+
Some((&ty::ty_rptr(_, from_mt), &ty::ty_rptr(_, to_mt))) => {
2145+
if to_mt.mutbl == ast::Mutability::MutMutable
2146+
&& from_mt.mutbl == ast::Mutability::MutImmutable {
2147+
cx.span_lint(MUTABLE_TRANSMUTES, expr.span, msg);
2148+
}
2149+
}
2150+
_ => ()
2151+
}
2152+
2153+
fn get_transmute_from_to<'a, 'tcx>(cx: &Context<'a, 'tcx>, expr: &ast::Expr)
2154+
-> Option<(&'tcx ty::sty<'tcx>, &'tcx ty::sty<'tcx>)> {
2155+
match expr.node {
2156+
ast::ExprPath(..) => (),
2157+
_ => return None
2158+
}
2159+
if let DefFn(did, _) = ty::resolve_expr(cx.tcx, expr) {
2160+
if !def_id_is_transmute(cx, did) {
2161+
return None;
2162+
}
2163+
let typ = ty::node_id_to_type(cx.tcx, expr.id);
2164+
match typ.sty {
2165+
ty::ty_bare_fn(_, ref bare_fn) if bare_fn.abi == RustIntrinsic => {
2166+
if let ty::FnConverging(to) = bare_fn.sig.0.output {
2167+
let from = bare_fn.sig.0.inputs[0];
2168+
return Some((&from.sty, &to.sty));
2169+
}
2170+
},
2171+
_ => ()
2172+
}
2173+
}
2174+
None
2175+
}
2176+
2177+
fn def_id_is_transmute(cx: &Context, def_id: DefId) -> bool {
2178+
match ty::lookup_item_type(cx.tcx, def_id).ty.sty {
2179+
ty::ty_bare_fn(_, ref bfty) if bfty.abi == RustIntrinsic => (),
2180+
_ => return false
2181+
}
2182+
ty::with_path(cx.tcx, def_id, |path| match path.last() {
2183+
Some(ref last) => last.name().as_str() == "transmute",
2184+
_ => false
2185+
})
2186+
}
2187+
}
2188+
}
2189+
21242190
/// Forbids using the `#[feature(...)]` attribute
21252191
#[derive(Copy, Clone)]
21262192
pub struct UnstableFeatures;

‎src/librustc_lint/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
109109
InvalidNoMangleItems,
110110
PluginAsLibrary,
111111
DropWithReprExtern,
112+
MutableTransmutes,
112113
);
113114

114115
add_builtin_with_new!(sess,

‎src/libstd/sync/mpsc/select.rs‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858

5959
use core::prelude::*;
6060

61-
use core::cell::Cell;
61+
use core::cell::{Cell, UnsafeCell};
6262
use core::marker;
6363
use core::mem;
6464
use core::ptr;
@@ -70,9 +70,13 @@ use sync::mpsc::blocking::{self, SignalToken};
7070
/// The "receiver set" of the select interface. This structure is used to manage
7171
/// a set of receivers which are being selected over.
7272
pub struct Select {
73+
inner: UnsafeCell<SelectInner>,
74+
next_id: Cell<usize>,
75+
}
76+
77+
struct SelectInner {
7378
head: *mut Handle<'static, ()>,
7479
tail: *mut Handle<'static, ()>,
75-
next_id: Cell<usize>,
7680
}
7781

7882
impl !marker::Send for Select {}
@@ -84,7 +88,7 @@ pub struct Handle<'rx, T:Send+'rx> {
8488
/// The ID of this handle, used to compare against the return value of
8589
/// `Select::wait()`
8690
id: usize,
87-
selector: &'rx Select,
91+
selector: *mut SelectInner,
8892
next: *mut Handle<'static, ()>,
8993
prev: *mut Handle<'static, ()>,
9094
added: bool,
@@ -127,8 +131,10 @@ impl Select {
127131
/// ```
128132
pub fn new() -> Select {
129133
Select {
130-
head: ptr::null_mut(),
131-
tail: ptr::null_mut(),
134+
inner: UnsafeCell::new(SelectInner {
135+
head: ptr::null_mut(),
136+
tail: ptr::null_mut(),
137+
}),
132138
next_id: Cell::new(1),
133139
}
134140
}
@@ -141,7 +147,7 @@ impl Select {
141147
self.next_id.set(id + 1);
142148
Handle {
143149
id: id,
144-
selector: self,
150+
selector: self.inner.get(),
145151
next: ptr::null_mut(),
146152
prev: ptr::null_mut(),
147153
added: false,
@@ -250,7 +256,7 @@ impl Select {
250256
}
251257
}
252258

253-
fn iter(&self) -> Packets { Packets { cur: self.head } }
259+
fn iter(&self) -> Packets { Packets { cur: unsafe { &*self.inner.get() }.head } }
254260
}
255261

256262
impl<'rx, T: Send> Handle<'rx, T> {
@@ -271,7 +277,7 @@ impl<'rx, T: Send> Handle<'rx, T> {
271277
/// while it is added to the `Select` set.
272278
pub unsafe fn add(&mut self) {
273279
if self.added { return }
274-
let selector: &mut Select = mem::transmute(&*self.selector);
280+
let selector = &mut *self.selector;
275281
let me: *mut Handle<'static, ()> = mem::transmute(&*self);
276282

277283
if selector.head.is_null() {
@@ -292,7 +298,7 @@ impl<'rx, T: Send> Handle<'rx, T> {
292298
pub unsafe fn remove(&mut self) {
293299
if !self.added { return }
294300

295-
let selector: &mut Select = mem::transmute(&*self.selector);
301+
let selector = &mut *self.selector;
296302
let me: *mut Handle<'static, ()> = mem::transmute(&*self);
297303

298304
if self.prev.is_null() {
@@ -317,8 +323,10 @@ impl<'rx, T: Send> Handle<'rx, T> {
317323

318324
impl Drop for Select {
319325
fn drop(&mut self) {
320-
assert!(self.head.is_null());
321-
assert!(self.tail.is_null());
326+
unsafe {
327+
assert!((&*self.inner.get()).head.is_null());
328+
assert!((&*self.inner.get()).tail.is_null());
329+
}
322330
}
323331
}
324332

‎src/test/auxiliary/issue_8401.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,6 @@ impl A for B {}
2121
fn bar<T>(_: &mut A, _: &T) {}
2222

2323
fn foo<T>(t: &T) {
24-
let b = B;
25-
bar(unsafe { mem::transmute(&b as &A) }, t)
24+
let mut b = B;
25+
bar(&mut b as &mut A, t)
2626
}

‎src/test/compile-fail/transmute-imut-to-mut.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Tests that transmuting from &T to &mut T is Undefined Behavior.
12+
13+
use std::mem::transmute;
14+
15+
fn main() {
16+
let _a: &mut u8 = unsafe { transmute(&1u8) };
17+
//~^ ERROR mutating transmuted &mut T from &T may cause undefined behavior
18+
}
19+
20+

‎src/test/run-pass/issue-2718.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ pub mod pipes {
170170
unsafe {
171171
if self.p != None {
172172
let self_p: &mut Option<*const packet<T>> =
173-
mem::transmute(&self.p);
173+
mem::transmute(&mut self.p);
174174
let p = replace(self_p, None);
175175
sender_terminate(p.unwrap())
176176
}
@@ -199,7 +199,7 @@ pub mod pipes {
199199
unsafe {
200200
if self.p != None {
201201
let self_p: &mut Option<*const packet<T>> =
202-
mem::transmute(&self.p);
202+
mem::transmute(&mut self.p);
203203
let p = replace(self_p, None);
204204
receiver_terminate(p.unwrap())
205205
}

0 commit comments

Comments
 (0)