Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

closure field capturing: don't depend on alignment of packed fields #115315

Merged
merged 1 commit into from
Sep 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 10 additions & 36 deletions compiler/rustc_hir_typeck/src/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

assert_eq!(self.tcx.hir().body_owner_def_id(body.id()), closure_def_id);
let mut delegate = InferBorrowKind {
fcx: self,
closure_def_id,
capture_information: Default::default(),
fake_reads: Default::default(),
Expand Down Expand Up @@ -1607,34 +1606,20 @@ 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>,
mut place: Place<'tcx>,
mut curr_borrow_kind: ty::UpvarCapture,
) -> (Place<'tcx>, ty::UpvarCapture) {
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.
// Return true for fields of packed structs.
match p.kind {
ProjectionKind::Field(..) => match ty.kind() {
ty::Adt(def, _) if def.repr().packed() => {
// We erase regions here because they cannot be hashed
match tcx.layout_of(param_env.and(tcx.erase_regions(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
}
}
// We stop here regardless of field alignment. Field alignment can change as
// types change, including the types of private fields in other crates, and that
// shouldn't affect how we compute our captures.
true
}

_ => false,
Expand Down Expand Up @@ -1689,9 +1674,7 @@ fn drop_location_span(tcx: TyCtxt<'_>, hir_id: hir::HirId) -> Span {
tcx.sess.source_map().end_point(owner_span)
}

struct InferBorrowKind<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,

struct InferBorrowKind<'tcx> {
// The def-id of the closure whose kind and upvar accesses are being inferred.
closure_def_id: LocalDefId,

Expand Down Expand Up @@ -1725,7 +1708,7 @@ struct InferBorrowKind<'a, 'tcx> {
fake_reads: Vec<(Place<'tcx>, FakeReadCause, hir::HirId)>,
}

impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
fn fake_read(
&mut self,
place: &PlaceWithHirId<'tcx>,
Expand All @@ -1740,12 +1723,7 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {

let (place, _) = restrict_capture_precision(place.place.clone(), dummy_capture_kind);

let (place, _) = restrict_repr_packed_field_ref_capture(
self.fcx.tcx,
self.fcx.param_env,
place,
dummy_capture_kind,
);
let (place, _) = restrict_repr_packed_field_ref_capture(place, dummy_capture_kind);
self.fake_reads.push((place, cause, diag_expr_id));
}

Expand Down Expand Up @@ -1780,12 +1758,8 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
// We only want repr packed restriction to be applied to reading references into a packed
// struct, and not when the data is being moved. Therefore we call this method here instead
// of in `restrict_capture_precision`.
let (place, mut capture_kind) = restrict_repr_packed_field_ref_capture(
self.fcx.tcx,
self.fcx.param_env,
place_with_id.place.clone(),
capture_kind,
);
let (place, mut capture_kind) =
restrict_repr_packed_field_ref_capture(place_with_id.place.clone(), capture_kind);

// Raw pointers don't inherit mutability
if place_with_id.place.deref_tys().any(Ty::is_unsafe_ptr) {
Expand Down
10 changes: 5 additions & 5 deletions tests/ui/closures/2229_closure_analysis/repr_packed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
#![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.
// Therefore we *could* precisely (and safely) capture references to both the fields,
// but we don't, since we don't want capturing to change when field types change alignment.
fn test_alignment_not_affected() {
#[repr(packed)]
struct Foo { x: u8, y: u8 }
Expand All @@ -17,11 +18,10 @@ fn test_alignment_not_affected() {
//~^ 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
//~^ NOTE: Capturing foo[] -> ImmBorrow
let z2: &mut u8 = &mut foo.y;
//~^ NOTE: Capturing foo[(1, 0)] -> MutBorrow
//~| NOTE: Min Capture foo[(1, 0)] -> MutBorrow
//~^ NOTE: Capturing foo[] -> MutBorrow
//~| NOTE: Min Capture foo[] -> MutBorrow

*z2 = 42;

Expand Down
19 changes: 7 additions & 12 deletions tests/ui/closures/2229_closure_analysis/repr_packed.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0658]: attributes on expressions are experimental
--> $DIR/repr_packed.rs:13:17
--> $DIR/repr_packed.rs:14:17
|
LL | let mut c = #[rustc_capture_analysis]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -26,7 +26,7 @@ LL | let c = #[rustc_capture_analysis]
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable

error: First Pass analysis includes:
--> $DIR/repr_packed.rs:16:5
--> $DIR/repr_packed.rs:17:5
|
LL | / || {
LL | |
Expand All @@ -37,19 +37,19 @@ LL | | println!("({}, {})", z1, z2);
LL | | };
| |_____^
|
note: Capturing foo[(0, 0)] -> ImmBorrow
--> $DIR/repr_packed.rs:19:24
note: Capturing foo[] -> ImmBorrow
--> $DIR/repr_packed.rs:20:24
|
LL | let z1: &u8 = &foo.x;
| ^^^^^
note: Capturing foo[(1, 0)] -> MutBorrow
note: Capturing foo[] -> MutBorrow
--> $DIR/repr_packed.rs:22:32
|
LL | let z2: &mut u8 = &mut foo.y;
| ^^^^^

error: Min Capture analysis includes:
--> $DIR/repr_packed.rs:16:5
--> $DIR/repr_packed.rs:17:5
|
LL | / || {
LL | |
Expand All @@ -60,12 +60,7 @@ LL | | println!("({}, {})", z1, z2);
LL | | };
| |_____^
|
note: Min Capture foo[(0, 0)] -> ImmBorrow
--> $DIR/repr_packed.rs:19:24
|
LL | let z1: &u8 = &foo.x;
| ^^^^^
note: Min Capture foo[(1, 0)] -> MutBorrow
note: Min Capture foo[] -> MutBorrow
--> $DIR/repr_packed.rs:22:32
|
LL | let z2: &mut u8 = &mut foo.y;
Expand Down