codegen, miri: fix computing the offset of an unsized field in a packed struct

Author: RalfJung

compiler/rustc_codegen_ssa/src/mir/place.rs

@@ -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!(
@@ -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();
-                }
-            }
             _ => {}
         }
 
@@ -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);

compiler/rustc_const_eval/src/interpret/projection.rs

@@ -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.

src/tools/miri/tests/pass/issues/issue-3200-packed-field-offset.rs

@@ -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);
+} }

src/tools/miri/tests/pass/issues/issue-3200-packed2-field-offset.rs

@@ -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);
+} }

tests/ui/packed/issue-118537-field-offset-ice.rs

@@ -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);
+} }

tests/ui/packed/issue-118537-field-offset.rs

@@ -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);
+} }