mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-02 07:22:42 +00:00
2229: Handle capturing a reference into a repr packed struct
RFC 1240 states that it is unsafe to capture references into a packed-struct. This PR ensures that when a closure captures a precise path, we aren't violating this safety constraint. To acheive so we restrict the capture precision to the struct itself. An interesting edge case: ```rust struct Foo(String); let foo: Foo; let c = || { println!("{}", foo.0); let x = foo.0; } ``` Given how closures get desugared today, foo.0 will be moved into the closure, making the `println!`, safe. However this can be very subtle and also will be unsafe if the closure gets inline. Closes: https://github.com/rust-lang/project-rfc-2229/issues/33
This commit is contained in:
parent
dfe519b344
commit
612a9b2f95
@ -1053,6 +1053,52 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Truncate the capture so that the place being borrowed is in accordance with RFC 1240,
|
||||
/// which states that it's unsafe to take a reference into a struct marked `repr(packed)`.
|
||||
fn restrict_repr_packed_field_ref_capture<'tcx>(
|
||||
tcx: TyCtxt<'tcx>,
|
||||
param_env: ty::ParamEnv<'tcx>,
|
||||
place: &Place<'tcx>,
|
||||
) -> Place<'tcx> {
|
||||
let pos = place.projections.iter().enumerate().position(|(i, p)| {
|
||||
let ty = place.ty_before_projection(i);
|
||||
|
||||
// Return true for fields of packed structs, unless those fields have alignment 1.
|
||||
match p.kind {
|
||||
ProjectionKind::Field(..) => match ty.kind() {
|
||||
ty::Adt(def, _) if def.repr.packed() => {
|
||||
match tcx.layout_raw(param_env.and(p.ty)) {
|
||||
Ok(layout) if layout.align.abi.bytes() == 1 => {
|
||||
// if the alignment is 1, the type can't be further
|
||||
// disaligned.
|
||||
debug!(
|
||||
"restrict_repr_packed_field_ref_capture: ({:?}) - align = 1",
|
||||
place
|
||||
);
|
||||
false
|
||||
}
|
||||
_ => {
|
||||
debug!("restrict_repr_packed_field_ref_capture: ({:?}) - true", place);
|
||||
true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
_ => false,
|
||||
},
|
||||
_ => false,
|
||||
}
|
||||
});
|
||||
|
||||
let mut place = place.clone();
|
||||
|
||||
if let Some(pos) = pos {
|
||||
place.projections.truncate(pos);
|
||||
}
|
||||
|
||||
place
|
||||
}
|
||||
|
||||
struct InferBorrowKind<'a, 'tcx> {
|
||||
fcx: &'a FnCtxt<'a, 'tcx>,
|
||||
|
||||
@ -1391,8 +1437,15 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
|
||||
place_with_id, diag_expr_id, bk
|
||||
);
|
||||
|
||||
let place = restrict_repr_packed_field_ref_capture(
|
||||
self.fcx.tcx,
|
||||
self.fcx.param_env,
|
||||
&place_with_id.place,
|
||||
);
|
||||
let place_with_id = PlaceWithHirId { place, ..*place_with_id };
|
||||
|
||||
if !self.capture_information.contains_key(&place_with_id.place) {
|
||||
self.init_capture_info_for_place(place_with_id, diag_expr_id);
|
||||
self.init_capture_info_for_place(&place_with_id, diag_expr_id);
|
||||
}
|
||||
|
||||
match bk {
|
||||
@ -1409,11 +1462,7 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
|
||||
fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) {
|
||||
debug!("mutate(assignee_place={:?}, diag_expr_id={:?})", assignee_place, diag_expr_id);
|
||||
|
||||
if !self.capture_information.contains_key(&assignee_place.place) {
|
||||
self.init_capture_info_for_place(assignee_place, diag_expr_id);
|
||||
}
|
||||
|
||||
self.adjust_upvar_borrow_kind_for_mut(assignee_place, diag_expr_id);
|
||||
self.borrow(assignee_place, diag_expr_id, ty::BorrowKind::MutBorrow);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -0,0 +1,36 @@
|
||||
// check-pass
|
||||
|
||||
#![feature(capture_disjoint_fields)]
|
||||
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
|
||||
|
||||
// Given how the closure desugaring is implemented (at least at the time of writing this test),
|
||||
// we don't need to truncate the captured path to a reference into a packed-struct if the field
|
||||
// being referenced will be moved into the closure, since it's safe to move out a field from a
|
||||
// packed-struct.
|
||||
//
|
||||
// However to avoid surprises for the user, or issues when the closure is
|
||||
// inlined we will truncate the capture to access just the struct regardless of if the field
|
||||
// might get moved into the closure.
|
||||
//
|
||||
// It is possible for someone to try writing the code that relies on the desugaring to access a ref
|
||||
// into a packed-struct without explicity using unsafe. Here we test that the compiler warns the
|
||||
// user that such an access is still unsafe.
|
||||
fn test_missing_unsafe_warning_on_repr_packed() {
|
||||
#[repr(packed)]
|
||||
struct Foo { x: String }
|
||||
|
||||
let foo = Foo { x: String::new() };
|
||||
|
||||
let c = || {
|
||||
println!("{}", foo.x);
|
||||
//~^ WARNING: borrow of packed field is unsafe and requires unsafe function or block
|
||||
//~| WARNING: this was previously accepted by the compiler but is being phased out
|
||||
let _z = foo.x;
|
||||
};
|
||||
|
||||
c();
|
||||
}
|
||||
|
||||
fn main() {
|
||||
test_missing_unsafe_warning_on_repr_packed();
|
||||
}
|
@ -0,0 +1,22 @@
|
||||
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
|
||||
--> $DIR/repr_packed.rs:3:12
|
||||
|
|
||||
LL | #![feature(capture_disjoint_fields)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `#[warn(incomplete_features)]` on by default
|
||||
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
|
||||
|
||||
warning: borrow of packed field is unsafe and requires unsafe function or block (error E0133)
|
||||
--> $DIR/repr_packed.rs:25:24
|
||||
|
|
||||
LL | println!("{}", foo.x);
|
||||
| ^^^^^
|
||||
|
|
||||
= note: `#[warn(safe_packed_borrows)]` on by default
|
||||
= 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>
|
||||
= note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior
|
||||
|
||||
warning: 2 warnings emitted
|
||||
|
103
src/test/ui/closures/2229_closure_analysis/repr_packed.rs
Normal file
103
src/test/ui/closures/2229_closure_analysis/repr_packed.rs
Normal file
@ -0,0 +1,103 @@
|
||||
#![feature(capture_disjoint_fields)]
|
||||
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
|
||||
//~| `#[warn(incomplete_features)]` on by default
|
||||
//~| see issue #53488 <https://github.com/rust-lang/rust/issues/53488>
|
||||
|
||||
#![feature(rustc_attrs)]
|
||||
|
||||
// `u8` aligned at a byte and are unaffected by repr(packed).
|
||||
// Therefore we can precisely (and safely) capture references to both the fields.
|
||||
fn test_alignment_not_affected() {
|
||||
#[repr(packed)]
|
||||
struct Foo { x: u8, y: u8 }
|
||||
|
||||
let mut foo = Foo { x: 0, y: 0 };
|
||||
|
||||
let mut c = #[rustc_capture_analysis]
|
||||
//~^ ERROR: attributes on expressions are experimental
|
||||
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|
||||
|| {
|
||||
//~^ ERROR: First Pass analysis includes:
|
||||
//~| ERROR: Min Capture analysis includes:
|
||||
let z1: &u8 = &foo.x;
|
||||
//~^ NOTE: Capturing foo[(0, 0)] -> ImmBorrow
|
||||
//~| NOTE: Min Capture foo[(0, 0)] -> ImmBorrow
|
||||
let z2: &mut u8 = &mut foo.y;
|
||||
//~^ NOTE: Capturing foo[(1, 0)] -> MutBorrow
|
||||
//~| NOTE: Min Capture foo[(1, 0)] -> MutBorrow
|
||||
|
||||
*z2 = 42;
|
||||
|
||||
println!("({}, {})", z1, z2);
|
||||
};
|
||||
|
||||
c();
|
||||
}
|
||||
|
||||
// `String`, `u16` are not aligned at a one byte boundry and are thus affected by repr(packed).
|
||||
//
|
||||
// Here we test that the closure doesn't capture a reference point to `foo.x` but
|
||||
// rather capture `foo` entirely.
|
||||
fn test_alignment_affected() {
|
||||
#[repr(packed)]
|
||||
struct Foo { x: String, y: u16 }
|
||||
|
||||
let mut foo = Foo { x: String::new(), y: 0 };
|
||||
|
||||
let mut c = #[rustc_capture_analysis]
|
||||
//~^ ERROR: attributes on expressions are experimental
|
||||
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|
||||
|| {
|
||||
//~^ ERROR: First Pass analysis includes:
|
||||
//~| ERROR: Min Capture analysis includes:
|
||||
let z1: &String = &foo.x;
|
||||
let z2: &mut u16 = &mut foo.y;
|
||||
//~^ NOTE: Capturing foo[] -> MutBorrow
|
||||
//~| NOTE: Min Capture foo[] -> MutBorrow
|
||||
|
||||
|
||||
*z2 = 42;
|
||||
|
||||
println!("({}, {})", z1, z2);
|
||||
};
|
||||
|
||||
c();
|
||||
}
|
||||
|
||||
// Given how the closure desugaring is implemented (at least at the time of writing this test),
|
||||
// we don't need to truncate the captured path to a reference into a packed-struct if the field
|
||||
// being referenced will be moved into the closure, since it's safe to move out a field from a
|
||||
// packed-struct.
|
||||
//
|
||||
// However to avoid surprises for the user, or issues when the closure is
|
||||
// inlined we will truncate the capture to access just the struct regardless of if the field
|
||||
// might get moved into the closure.
|
||||
fn test_truncation_when_ref_and_move() {
|
||||
#[repr(packed)]
|
||||
struct Foo { x: String }
|
||||
|
||||
let mut foo = Foo { x: String::new() };
|
||||
|
||||
let c = #[rustc_capture_analysis]
|
||||
//~^ ERROR: attributes on expressions are experimental
|
||||
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|
||||
|| {
|
||||
//~^ ERROR: First Pass analysis includes:
|
||||
//~| ERROR: Min Capture analysis includes:
|
||||
println!("{}", foo.x);
|
||||
//~^ NOTE: Capturing foo[] -> ImmBorrow
|
||||
//~| NOTE: Min Capture foo[] -> ByValue
|
||||
//~| NOTE: foo[] used here
|
||||
let _z = foo.x;
|
||||
//~^ NOTE: Capturing foo[(0, 0)] -> ByValue
|
||||
//~| NOTE: foo[] captured as ByValue here
|
||||
};
|
||||
|
||||
c();
|
||||
}
|
||||
|
||||
fn main() {
|
||||
test_truncation_when_ref_and_move();
|
||||
test_alignment_affected();
|
||||
test_alignment_not_affected();
|
||||
}
|
165
src/test/ui/closures/2229_closure_analysis/repr_packed.stderr
Normal file
165
src/test/ui/closures/2229_closure_analysis/repr_packed.stderr
Normal file
@ -0,0 +1,165 @@
|
||||
error[E0658]: attributes on expressions are experimental
|
||||
--> $DIR/repr_packed.rs:16:17
|
||||
|
|
||||
LL | let mut c = #[rustc_capture_analysis]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
|
||||
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
|
||||
|
||||
error[E0658]: attributes on expressions are experimental
|
||||
--> $DIR/repr_packed.rs:47:17
|
||||
|
|
||||
LL | let mut c = #[rustc_capture_analysis]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
|
||||
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
|
||||
|
||||
error[E0658]: attributes on expressions are experimental
|
||||
--> $DIR/repr_packed.rs:81:13
|
||||
|
|
||||
LL | let c = #[rustc_capture_analysis]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
|
||||
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
|
||||
|
||||
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
|
||||
--> $DIR/repr_packed.rs:1:12
|
||||
|
|
||||
LL | #![feature(capture_disjoint_fields)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `#[warn(incomplete_features)]` on by default
|
||||
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
|
||||
|
||||
error: First Pass analysis includes:
|
||||
--> $DIR/repr_packed.rs:19:5
|
||||
|
|
||||
LL | / || {
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | let z1: &u8 = &foo.x;
|
||||
... |
|
||||
LL | | println!("({}, {})", z1, z2);
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
|
||||
note: Capturing foo[(0, 0)] -> ImmBorrow
|
||||
--> $DIR/repr_packed.rs:22:24
|
||||
|
|
||||
LL | let z1: &u8 = &foo.x;
|
||||
| ^^^^^
|
||||
note: Capturing foo[(1, 0)] -> MutBorrow
|
||||
--> $DIR/repr_packed.rs:25:32
|
||||
|
|
||||
LL | let z2: &mut u8 = &mut foo.y;
|
||||
| ^^^^^
|
||||
|
||||
error: Min Capture analysis includes:
|
||||
--> $DIR/repr_packed.rs:19:5
|
||||
|
|
||||
LL | / || {
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | let z1: &u8 = &foo.x;
|
||||
... |
|
||||
LL | | println!("({}, {})", z1, z2);
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
|
||||
note: Min Capture foo[(0, 0)] -> ImmBorrow
|
||||
--> $DIR/repr_packed.rs:22:24
|
||||
|
|
||||
LL | let z1: &u8 = &foo.x;
|
||||
| ^^^^^
|
||||
note: Min Capture foo[(1, 0)] -> MutBorrow
|
||||
--> $DIR/repr_packed.rs:25:32
|
||||
|
|
||||
LL | let z2: &mut u8 = &mut foo.y;
|
||||
| ^^^^^
|
||||
|
||||
error: First Pass analysis includes:
|
||||
--> $DIR/repr_packed.rs:50:5
|
||||
|
|
||||
LL | / || {
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | let z1: &String = &foo.x;
|
||||
... |
|
||||
LL | | println!("({}, {})", z1, z2);
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
|
||||
note: Capturing foo[] -> MutBorrow
|
||||
--> $DIR/repr_packed.rs:54:33
|
||||
|
|
||||
LL | let z2: &mut u16 = &mut foo.y;
|
||||
| ^^^^^
|
||||
|
||||
error: Min Capture analysis includes:
|
||||
--> $DIR/repr_packed.rs:50:5
|
||||
|
|
||||
LL | / || {
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | let z1: &String = &foo.x;
|
||||
... |
|
||||
LL | | println!("({}, {})", z1, z2);
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
|
||||
note: Min Capture foo[] -> MutBorrow
|
||||
--> $DIR/repr_packed.rs:54:33
|
||||
|
|
||||
LL | let z2: &mut u16 = &mut foo.y;
|
||||
| ^^^^^
|
||||
|
||||
error: First Pass analysis includes:
|
||||
--> $DIR/repr_packed.rs:84:5
|
||||
|
|
||||
LL | / || {
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | println!("{}", foo.x);
|
||||
... |
|
||||
LL | |
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
|
||||
note: Capturing foo[] -> ImmBorrow
|
||||
--> $DIR/repr_packed.rs:87:24
|
||||
|
|
||||
LL | println!("{}", foo.x);
|
||||
| ^^^^^
|
||||
note: Capturing foo[(0, 0)] -> ByValue
|
||||
--> $DIR/repr_packed.rs:91:18
|
||||
|
|
||||
LL | let _z = foo.x;
|
||||
| ^^^^^
|
||||
|
||||
error: Min Capture analysis includes:
|
||||
--> $DIR/repr_packed.rs:84:5
|
||||
|
|
||||
LL | / || {
|
||||
LL | |
|
||||
LL | |
|
||||
LL | | println!("{}", foo.x);
|
||||
... |
|
||||
LL | |
|
||||
LL | | };
|
||||
| |_____^
|
||||
|
|
||||
note: Min Capture foo[] -> ByValue
|
||||
--> $DIR/repr_packed.rs:87:24
|
||||
|
|
||||
LL | println!("{}", foo.x);
|
||||
| ^^^^^ foo[] used here
|
||||
...
|
||||
LL | let _z = foo.x;
|
||||
| ^^^^^ foo[] captured as ByValue here
|
||||
|
||||
error: aborting due to 9 previous errors; 1 warning emitted
|
||||
|
||||
For more information about this error, try `rustc --explain E0658`.
|
Loading…
Reference in New Issue
Block a user