Auto merge of #44884 - arielb1:pack-safe, r=nikomatsakis,eddyb

Make accesses to fields of packed structs unsafe

To handle packed structs with destructors (which you'll think are a rare
case, but the `#[repr(packed)] struct Packed<T>(T);` pattern is
ever-popular, which requires handling packed structs with destructors to
avoid monomorphization-time errors), drops of subfields of packed
structs should drop a local move of the field instead of the original
one.

That's it, I think I'll use a strategy suggested by @Zoxc, where this mir
```
drop(packed_struct.field)
```

is replaced by
```
tmp0 = packed_struct.field;
drop tmp0
```

cc #27060 - this should deal with that issue after codegen of drop glue
is updated.

The new errors need to be changed to future-compatibility warnings, but
I'll rather do a crater run first with them as errors to assess the
impact.

cc @eddyb

Things which still need to be done for this:
 - [ ] - handle `repr(packed)` structs in `derive` the same way I did in `Span`, and use derive there again
 - [ ] - implement the "fix packed drops" pass and call it in both the MIR shim and validated MIR pipelines
 - [ ] - do a crater run
 - [ ] - convert the errors to compatibility warnings
This commit is contained in:
bors 2017-11-27 14:23:02 +00:00
commit 58e1234cdd
32 changed files with 1035 additions and 241 deletions

View file

@ -0,0 +1,16 @@
// Copyright 2017 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.
#[repr(packed)]
pub struct Bad<T: ?Sized> {
data: T, //~ ERROR `T: std::marker::Sized` is not satisfied
}
fn main() {}

View file

@ -0,0 +1,43 @@
// Copyright 2017 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.
#[repr(packed)]
pub struct Good {
data: &'static u32,
data2: [&'static u32; 2],
aligned: [u8; 32],
}
#[repr(packed)]
pub struct JustArray {
array: [u32]
}
#[deny(safe_packed_borrows)]
fn main() {
let good = Good {
data: &0,
data2: [&0, &0],
aligned: [0; 32]
};
unsafe {
let _ = &good.data; // ok
let _ = &good.data2[0]; // ok
}
let _ = &good.data; //~ ERROR borrow of packed field requires unsafe
//~| hard error
let _ = &good.data2[0]; //~ ERROR borrow of packed field requires unsafe
//~| hard error
let _ = &*good.data; // ok, behind a pointer
let _ = &good.aligned; // ok, has align 1
let _ = &good.aligned[2]; // ok, has align 1
}

View file

@ -0,0 +1,68 @@
// Copyright 2017 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.
fn main() {
let mut x = Packed(Aligned(Droppy(0)));
x.0 = Aligned(Droppy(0));
}
struct Aligned(Droppy);
#[repr(packed)]
struct Packed(Aligned);
struct Droppy(usize);
impl Drop for Droppy {
fn drop(&mut self) {}
}
// END RUST SOURCE
// START rustc.main.EraseRegions.before.mir
// fn main() -> () {
// let mut _0: ();
// scope 1 {
// let mut _1: Packed;
// }
// scope 2 {
// }
// let mut _2: Aligned;
// let mut _3: Droppy;
// let mut _4: Aligned;
// let mut _5: Droppy;
// let mut _6: Aligned;
//
// bb0: {
// StorageLive(_1);
// ...
// _1 = Packed::{{constructor}}(_2,);
// ...
// StorageLive(_6);
// _6 = (_1.0: Aligned);
// drop(_6) -> [return: bb4, unwind: bb3];
// }
// bb1: {
// resume;
// }
// bb2: {
// StorageDead(_1);
// return;
// }
// bb3: {
// (_1.0: Aligned) = _4;
// drop(_1) -> bb1;
// }
// bb4: {
// StorageDead(_6);
// (_1.0: Aligned) = _4;
// StorageDead(_4);
// _0 = ();
// drop(_1) -> bb2;
// }
// }
// END rustc.main.EraseRegions.before.mir

View file

@ -0,0 +1,45 @@
// Copyright 2017 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.
// check that derive on a packed struct does not call field
// methods with a misaligned field.
use std::mem;
#[derive(Copy, Clone)]
struct Aligned(usize);
#[inline(never)]
fn check_align(ptr: *const Aligned) {
assert_eq!(ptr as usize % mem::align_of::<Aligned>(),
0);
}
impl PartialEq for Aligned {
fn eq(&self, other: &Self) -> bool {
check_align(self);
check_align(other);
self.0 == other.0
}
}
#[repr(packed)]
#[derive(Copy, Clone, PartialEq)]
struct Packed(Aligned, Aligned);
#[derive(PartialEq)]
#[repr(C)]
struct Dealigned<T>(u8, T);
fn main() {
let d1 = Dealigned(0, Packed(Aligned(1), Aligned(2)));
let ck = d1 == d1;
assert!(ck);
}

View file

@ -0,0 +1,42 @@
// Copyright 2017 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.
#[repr(packed)]
pub struct Good {
data: &'static u32,
data2: [&'static u32; 2],
aligned: [u8; 32],
}
#[repr(packed)]
pub struct JustArray {
array: [u32]
}
// kill this test when that turns to a hard error
#[allow(safe_packed_borrows)]
fn main() {
let good = Good {
data: &0,
data2: [&0, &0],
aligned: [0; 32]
};
unsafe {
let _ = &good.data; // ok
let _ = &good.data2[0]; // ok
}
let _ = &good.data;
let _ = &good.data2[0];
let _ = &*good.data; // ok, behind a pointer
let _ = &good.aligned; // ok, has align 1
let _ = &good.aligned[2]; // ok, has align 1
}

View file

@ -17,7 +17,7 @@ struct Foo {
pub fn main() {
let foo = Foo { bar: 1, baz: 2 };
let brw = &foo.baz;
let brw = unsafe { &foo.baz };
assert_eq!(*brw, 2);
}

View file

@ -0,0 +1,42 @@
// Copyright 2017 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.
use std::cell::Cell;
use std::mem;
struct Aligned<'a> {
drop_count: &'a Cell<usize>
}
#[inline(never)]
fn check_align(ptr: *const Aligned) {
assert_eq!(ptr as usize % mem::align_of::<Aligned>(),
0);
}
impl<'a> Drop for Aligned<'a> {
fn drop(&mut self) {
check_align(self);
self.drop_count.set(self.drop_count.get() + 1);
}
}
#[repr(packed)]
struct Packed<'a>(u8, Aligned<'a>);
fn main() {
let drop_count = &Cell::new(0);
{
let mut p = Packed(0, Aligned { drop_count });
p.1 = Aligned { drop_count };
assert_eq!(drop_count.get(), 1);
}
assert_eq!(drop_count.get(), 2);
}

View file

@ -9,8 +9,12 @@
// except according to those terms.
#[repr(packed)]
#[derive(Copy, Clone)]
struct Packed<T>(T);
struct Packed<T: Copy>(T);
impl<T: Copy> Copy for Packed<T> {}
impl<T: Copy> Clone for Packed<T> {
fn clone(&self) -> Self { *self }
}
fn main() {
let one = (Some(Packed((&(), 0))), true);

View file

@ -0,0 +1,41 @@
// Copyright 2017 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(safe_packed_borrows)]
// check that derive on a packed struct with non-Copy fields
// correctly. This can't be made to work perfectly because
// we can't just use the field from the struct as it might
// not be aligned.
#[derive(Copy, Clone, PartialEq, Eq)]
//~^ ERROR #[derive] can't be used
//~| hard error
//~^^^ ERROR #[derive] can't be used
//~| hard error
#[repr(packed)]
pub struct Foo<T>(T, T, T);
#[derive(PartialEq, Eq)]
//~^ ERROR #[derive] can't be used
//~| hard error
#[repr(packed)]
pub struct Bar(u32, u32, u32);
#[derive(PartialEq)]
struct Y(usize);
#[derive(PartialEq)]
//~^ ERROR #[derive] can't be used on a non-Copy #[repr(packed)]
//~| hard error
#[repr(packed)]
struct X(Y);
fn main() {}

View file

@ -0,0 +1,43 @@
error: #[derive] can't be used on a #[repr(packed)] struct with type parameters (error E0133)
--> $DIR/deriving-with-repr-packed.rs:18:16
|
18 | #[derive(Copy, Clone, PartialEq, Eq)]
| ^^^^^
|
note: lint level defined here
--> $DIR/deriving-with-repr-packed.rs:11:9
|
11 | #![deny(safe_packed_borrows)]
| ^^^^^^^^^^^^^^^^^^^
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
error: #[derive] can't be used on a #[repr(packed)] struct with type parameters (error E0133)
--> $DIR/deriving-with-repr-packed.rs:18:23
|
18 | #[derive(Copy, Clone, PartialEq, Eq)]
| ^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
error: #[derive] can't be used on a non-Copy #[repr(packed)] struct (error E0133)
--> $DIR/deriving-with-repr-packed.rs:26:10
|
26 | #[derive(PartialEq, Eq)]
| ^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
error: #[derive] can't be used on a non-Copy #[repr(packed)] struct (error E0133)
--> $DIR/deriving-with-repr-packed.rs:35:10
|
35 | #[derive(PartialEq)]
| ^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
error: aborting due to 4 previous errors