Skip to content

Commit

Permalink
Rollup merge of #118540 - RalfJung:unsized-packed-offset, r=TaKO8Ki
Browse files Browse the repository at this point in the history
codegen, miri: fix computing the offset of an unsized field in a packed struct

`#[repr(packed)]`  strikes again.

Fixes #118537
Fixes rust-lang/miri#3200

`@bjorn3` I assume cranelift needs the same fix.
  • Loading branch information
TaKO8Ki authored Dec 4, 2023
2 parents da30882 + ef15a81 commit 87625db
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 11 deletions.
22 changes: 12 additions & 10 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
// Simple cases, which don't need DST adjustment:
// * no metadata available - just log the case
// * known alignment - sized types, `[T]`, `str` or a foreign type
// * packed struct - there is no alignment padding
// Note that looking at `field.align` is incorrect since that is not necessarily equal
// to the dynamic alignment of the type.
match field.ty.kind() {
_ if self.llextra.is_none() => {
debug!(
Expand All @@ -154,14 +155,6 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
}
_ if field.is_sized() => return simple(),
ty::Slice(..) | ty::Str | ty::Foreign(..) => return simple(),
ty::Adt(def, _) => {
if def.repr().packed() {
// FIXME(eddyb) generalize the adjustment when we
// start supporting packing to larger alignments.
assert_eq!(self.layout.align.abi.bytes(), 1);
return simple();
}
}
_ => {}
}

Expand All @@ -186,7 +179,16 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
let unaligned_offset = bx.cx().const_usize(offset.bytes());

// Get the alignment of the field
let (_, unsized_align) = glue::size_and_align_of_dst(bx, field.ty, meta);
let (_, mut unsized_align) = glue::size_and_align_of_dst(bx, field.ty, meta);

// For packed types, we need to cap alignment.
if let ty::Adt(def, _) = self.layout.ty.kind()
&& let Some(packed) = def.repr().pack
{
let packed = bx.const_usize(packed.bytes());
let cmp = bx.icmp(IntPredicate::IntULT, unsized_align, packed);
unsized_align = bx.select(cmp, unsized_align, packed)
}

// Bump the unaligned offset up to the appropriate alignment
let offset = round_up_const_value_to_alignment(bx, unaligned_offset, unsized_align);
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,17 @@ where
// With custom DSTS, this *will* execute user-defined code, but the same
// happens at run-time so that's okay.
match self.size_and_align_of(&base_meta, &field_layout)? {
Some((_, align)) => (base_meta, offset.align_to(align)),
Some((_, align)) => {
// For packed types, we need to cap alignment.
let align = if let ty::Adt(def, _) = base.layout().ty.kind()
&& let Some(packed) = def.repr().pack
{
align.min(packed)
} else {
align
};
(base_meta, offset.align_to(align))
}
None => {
// For unsized types with an extern type tail we perform no adjustments.
// NOTE: keep this in sync with `PlaceRef::project_field` in the codegen backend.
Expand Down
35 changes: 35 additions & 0 deletions src/tools/miri/tests/pass/issues/issue-3200-packed-field-offset.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#![feature(layout_for_ptr)]
use std::mem;

#[repr(packed, C)]
struct PackedSized {
f: u8,
d: [u32; 4],
}

#[repr(packed, C)]
struct PackedUnsized {
f: u8,
d: [u32],
}

impl PackedSized {
fn unsize(&self) -> &PackedUnsized {
// We can't unsize via a generic type since then we get the error
// that packed structs with unsized tail don't work if the tail
// might need dropping.
let len = 4usize;
unsafe { mem::transmute((self, len)) }
}
}

fn main() { unsafe {
let p = PackedSized { f: 0, d: [1, 2, 3, 4] };
let p = p.unsize() as *const PackedUnsized;
// Make sure the size computation does *not* think there is
// any padding in front of the `d` field.
assert_eq!(mem::size_of_val_raw(p), 1 + 4*4);
// And likewise for the offset computation.
let d = std::ptr::addr_of!((*p).d);
assert_eq!(d.cast::<u32>().read_unaligned(), 1);
} }
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#![feature(layout_for_ptr)]
use std::mem;

#[repr(packed(4))]
struct Slice([u32]);

#[repr(packed(2), C)]
struct PackedSized {
f: u8,
d: [u32; 4],
}

#[repr(packed(2), C)]
struct PackedUnsized {
f: u8,
d: Slice,
}

impl PackedSized {
fn unsize(&self) -> &PackedUnsized {
// We can't unsize via a generic type since then we get the error
// that packed structs with unsized tail don't work if the tail
// might need dropping.
let len = 4usize;
unsafe { mem::transmute((self, len)) }
}
}

fn main() { unsafe {
let p = PackedSized { f: 0, d: [1, 2, 3, 4] };
let p = p.unsize() as *const PackedUnsized;
// Make sure the size computation correctly adds exact 1 byte of padding
// in front of the `d` field.
assert_eq!(mem::size_of_val_raw(p), 1 + 1 + 4*4);
// And likewise for the offset computation.
let d = std::ptr::addr_of!((*p).d);
assert_eq!(d.cast::<u32>().read_unaligned(), 1);
} }
39 changes: 39 additions & 0 deletions tests/ui/packed/issue-118537-field-offset-ice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// run-pass
#![feature(layout_for_ptr)]
use std::mem;

#[repr(packed(4))]
struct Slice([u32]);

#[repr(packed(2), C)]
struct PackedSized {
f: u8,
d: [u32; 4],
}

#[repr(packed(2), C)]
struct PackedUnsized {
f: u8,
d: Slice,
}

impl PackedSized {
fn unsize(&self) -> &PackedUnsized {
// We can't unsize via a generic type since then we get the error
// that packed structs with unsized tail don't work if the tail
// might need dropping.
let len = 4usize;
unsafe { mem::transmute((self, len)) }
}
}

fn main() { unsafe {
let p = PackedSized { f: 0, d: [1, 2, 3, 4] };
let p = p.unsize() as *const PackedUnsized;
// Make sure the size computation correctly adds exact 1 byte of padding
// in front of the `d` field.
assert_eq!(mem::size_of_val_raw(p), 1 + 1 + 4*4);
// And likewise for the offset computation.
let d = std::ptr::addr_of!((*p).d);
assert_eq!(d.cast::<u32>().read_unaligned(), 1);
} }
36 changes: 36 additions & 0 deletions tests/ui/packed/issue-118537-field-offset.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// run-pass
#![feature(layout_for_ptr)]
use std::mem;

#[repr(packed, C)]
struct PackedSized {
f: u8,
d: [u32; 4],
}

#[repr(packed, C)]
struct PackedUnsized {
f: u8,
d: [u32],
}

impl PackedSized {
fn unsize(&self) -> &PackedUnsized {
// We can't unsize via a generic type since then we get the error
// that packed structs with unsized tail don't work if the tail
// might need dropping.
let len = 4usize;
unsafe { mem::transmute((self, len)) }
}
}

fn main() { unsafe {
let p = PackedSized { f: 0, d: [1, 2, 3, 4] };
let p = p.unsize() as *const PackedUnsized;
// Make sure the size computation does *not* think there is
// any padding in front of the `d` field.
assert_eq!(mem::size_of_val_raw(p), 1 + 4*4);
// And likewise for the offset computation.
let d = std::ptr::addr_of!((*p).d);
assert_eq!(d.cast::<u32>().read_unaligned(), 1);
} }

0 comments on commit 87625db

Please sign in to comment.