Skip to content

Commit

Permalink
closure field capturing: don't depend on alignment of packed fields
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Aug 28, 2023
1 parent 7e02fd8 commit a671127
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 53 deletions.
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

0 comments on commit a671127

Please sign in to comment.