Auto merge of #118932 - matthiaskrgr:rollup-fi7xzqa, r=matthiaskrgr

Rollup of 3 pull requests

Successful merges:

 - #118375 (Add -Zunpretty=stable-mir output test)
 - #118538 (fix dynamic size/align computation logic for packed types with dyn trait tail)
 - #118789 (fix --dry-run when the change-id warning is printed)

r? `@ghost`
`@rustbot` modify labels: rollup

compiler/rustc_codegen_ssa/src/size_of_val.rs

@@ -84,37 +84,40 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             debug!("DST {} layout: {:?}", t, layout);
 
             let i = layout.fields.count() - 1;
-            let sized_size = layout.fields.offset(i).bytes();
+            let unsized_offset_unadjusted = layout.fields.offset(i).bytes();
             let sized_align = layout.align.abi.bytes();
-            debug!("DST {} statically sized prefix size: {} align: {}", t, sized_size, sized_align);
-            let sized_size = bx.const_usize(sized_size);
+            debug!(
+                "DST {} offset of dyn field: {}, statically sized align: {}",
+                t, unsized_offset_unadjusted, sized_align
+            );
+            let unsized_offset_unadjusted = bx.const_usize(unsized_offset_unadjusted);
             let sized_align = bx.const_usize(sized_align);
 
             // Recurse to get the size of the dynamically sized field (must be
             // the last field).
             let field_ty = layout.field(bx, i).ty;
             let (unsized_size, mut unsized_align) = size_and_align_of_dst(bx, field_ty, info);
 
-            // FIXME (#26403, #27023): We should be adding padding
-            // to `sized_size` (to accommodate the `unsized_align`
-            // required of the unsized field that follows) before
-            // summing it with `sized_size`. (Note that since #26403
-            // is unfixed, we do not yet add the necessary padding
-            // here. But this is where the add would go.)
-
-            // Return the sum of sizes and max of aligns.
-            let size = bx.add(sized_size, unsized_size);
-
-            // Packed types ignore the alignment of their fields.
-            if let ty::Adt(def, _) = t.kind() {
-                if def.repr().packed() {
-                    unsized_align = sized_align;
+            // # First compute the dynamic alignment
+
+            // For packed types, we need to cap the alignment.
+            if let ty::Adt(def, _) = t.kind()
+                && let Some(packed) = def.repr().pack
+            {
+                if packed.bytes() == 1 {
+                    // We know this will be capped to 1.
+                    unsized_align = bx.const_usize(1);
+                } else {
+                    // We have to dynamically compute `min(unsized_align, packed)`.
+                    let packed = bx.const_usize(packed.bytes());
+                    let cmp = bx.icmp(IntPredicate::IntULT, unsized_align, packed);
+                    unsized_align = bx.select(cmp, unsized_align, packed);
                 }
             }
 
             // Choose max of two known alignments (combined value must
             // be aligned according to more restrictive of the two).
-            let align = match (
+            let full_align = match (
                 bx.const_to_opt_u128(sized_align, false),
                 bx.const_to_opt_u128(unsized_align, false),
             ) {
@@ -129,6 +132,19 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
                 }
             };
 
+            // # Then compute the dynamic size
+
+            // The full formula for the size would be:
+            // let unsized_offset_adjusted = unsized_offset_unadjusted.align_to(unsized_align);
+            // let full_size = (unsized_offset_adjusted + unsized_size).align_to(full_align);
+            // However, `unsized_size` is a multiple of `unsized_align`.
+            // Therefore, we can equivalently do the `align_to(unsized_align)` *after* adding `unsized_size`:
+            // let full_size = (unsized_offset_unadjusted + unsized_size).align_to(unsized_align).align_to(full_align);
+            // Furthermore, `align >= unsized_align`, and therefore we only need to do:
+            // let full_size = (unsized_offset_unadjusted + unsized_size).align_to(full_align);
+
+            let full_size = bx.add(unsized_offset_unadjusted, unsized_size);
+
             // Issue #27023: must add any necessary padding to `size`
             // (to make it a multiple of `align`) before returning it.
             //
@@ -140,12 +156,12 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             //
             //   `(size + (align-1)) & -align`
             let one = bx.const_usize(1);
-            let addend = bx.sub(align, one);
-            let add = bx.add(size, addend);
-            let neg = bx.neg(align);
-            let size = bx.and(add, neg);
+            let addend = bx.sub(full_align, one);
+            let add = bx.add(full_size, addend);
+            let neg = bx.neg(full_align);
+            let full_size = bx.and(add, neg);
 
-            (size, align)
+            (full_size, full_align)
         }
         _ => bug!("size_and_align_of_dst: {t} not supported"),
     }

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -686,14 +686,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 assert!(layout.fields.count() > 0);
                 trace!("DST layout: {:?}", layout);
 
-                let sized_size = layout.fields.offset(layout.fields.count() - 1);
+                let unsized_offset_unadjusted = layout.fields.offset(layout.fields.count() - 1);
                 let sized_align = layout.align.abi;
-                trace!(
-                    "DST {} statically sized prefix size: {:?} align: {:?}",
-                    layout.ty,
-                    sized_size,
-                    sized_align
-                );
 
                 // Recurse to get the size of the dynamically sized field (must be
                 // the last field). Can't have foreign types here, how would we
@@ -707,36 +701,35 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     return Ok(None);
                 };
 
-                // FIXME (#26403, #27023): We should be adding padding
-                // to `sized_size` (to accommodate the `unsized_align`
-                // required of the unsized field that follows) before
-                // summing it with `sized_size`. (Note that since #26403
-                // is unfixed, we do not yet add the necessary padding
-                // here. But this is where the add would go.)
-
-                // Return the sum of sizes and max of aligns.
-                let size = sized_size + unsized_size; // `Size` addition
+                // # First compute the dynamic alignment
 
-                // Packed types ignore the alignment of their fields.
+                // Packed type alignment needs to be capped.
                 if let ty::Adt(def, _) = layout.ty.kind() {
-                    if def.repr().packed() {
-                        unsized_align = sized_align;
+                    if let Some(packed) = def.repr().pack {
+                        unsized_align = unsized_align.min(packed);
                     }
                 }
 
                 // Choose max of two known alignments (combined value must
                 // be aligned according to more restrictive of the two).
-                let align = sized_align.max(unsized_align);
+                let full_align = sized_align.max(unsized_align);
+
+                // # Then compute the dynamic size
 
-                // Issue #27023: must add any necessary padding to `size`
-                // (to make it a multiple of `align`) before returning it.
-                let size = size.align_to(align);
+                let unsized_offset_adjusted = unsized_offset_unadjusted.align_to(unsized_align);
+                let full_size = (unsized_offset_adjusted + unsized_size).align_to(full_align);
+
+                // Just for our sanitiy's sake, assert that this is equal to what codegen would compute.
+                assert_eq!(
+                    full_size,
+                    (unsized_offset_unadjusted + unsized_size).align_to(full_align)
+                );
 
                 // Check if this brought us over the size limit.
-                if size > self.max_size_of_val() {
+                if full_size > self.max_size_of_val() {
                     throw_ub!(InvalidMeta(InvalidMetaKind::TooBig));
                 }
-                Ok(Some((size, align)))
+                Ok(Some((full_size, full_align)))
             }
             ty::Dynamic(_, _, ty::Dyn) => {
                 let vtable = metadata.unwrap_meta().to_pointer(self)?;

compiler/stable_mir/src/mir/pretty.rs

@@ -58,18 +58,31 @@ pub fn pretty_statement(statement: &StatementKind) -> String {
             pretty.push_str(format!("        _{} = ", place.local).as_str());
             pretty.push_str(format!("{}", &pretty_rvalue(rval)).as_str());
         }
-        StatementKind::FakeRead(_, _) => todo!(),
-        StatementKind::SetDiscriminant { .. } => todo!(),
-        StatementKind::Deinit(_) => todo!(),
-        StatementKind::StorageLive(_) => todo!(),
-        StatementKind::StorageDead(_) => todo!(),
-        StatementKind::Retag(_, _) => todo!(),
-        StatementKind::PlaceMention(_) => todo!(),
-        StatementKind::AscribeUserType { .. } => todo!(),
-        StatementKind::Coverage(_) => todo!(),
-        StatementKind::Intrinsic(_) => todo!(),
-        StatementKind::ConstEvalCounter => (),
-        StatementKind::Nop => (),
+        // FIXME: Add rest of the statements
+        StatementKind::FakeRead(_, _) => return format!("StatementKind::FakeRead:Unimplemented"),
+        StatementKind::SetDiscriminant { .. } => {
+            return format!("StatementKind::SetDiscriminant:Unimplemented");
+        }
+        StatementKind::Deinit(_) => return format!("StatementKind::Deinit:Unimplemented"),
+        StatementKind::StorageLive(_) => {
+            return format!("StatementKind::StorageLive:Unimplemented");
+        }
+        StatementKind::StorageDead(_) => {
+            return format!("StatementKind::StorageDead:Unimplemented");
+        }
+        StatementKind::Retag(_, _) => return format!("StatementKind::Retag:Unimplemented"),
+        StatementKind::PlaceMention(_) => {
+            return format!("StatementKind::PlaceMention:Unimplemented");
+        }
+        StatementKind::AscribeUserType { .. } => {
+            return format!("StatementKind::AscribeUserType:Unimplemented");
+        }
+        StatementKind::Coverage(_) => return format!("StatementKind::Coverage:Unimplemented"),
+        StatementKind::Intrinsic(_) => return format!("StatementKind::Intrinsic:Unimplemented"),
+        StatementKind::ConstEvalCounter => {
+            return format!("StatementKind::ConstEvalCounter:Unimplemented");
+        }
+        StatementKind::Nop => return format!("StatementKind::Nop:Unimplemented"),
     }
     pretty
 }
@@ -355,7 +368,7 @@ pub fn pretty_rvalue(rval: &Rvalue) -> String {
             pretty.push_str(" ");
             pretty.push_str(&pretty_ty(cnst.ty().kind()));
         }
-        Rvalue::ShallowInitBox(_, _) => todo!(),
+        Rvalue::ShallowInitBox(_, _) => (),
         Rvalue::ThreadLocalRef(item) => {
             pretty.push_str("thread_local_ref");
             pretty.push_str(format!("{:#?}", item).as_str());

src/bootstrap/src/bin/main.rs

@@ -174,7 +174,7 @@ fn check_version(config: &Config) -> Option<String> {
                 "update `config.toml` to use `change-id = {latest_change_id}` instead"
             ));
 
-            if io::stdout().is_terminal() {
+            if io::stdout().is_terminal() && !config.dry_run() {
                 t!(fs::write(warned_id_path, latest_change_id.to_string()));
             }
         }

src/bootstrap/src/core/config/config.rs

@@ -1969,7 +1969,7 @@ impl Config {
         config
     }
 
-    pub(crate) fn dry_run(&self) -> bool {
+    pub fn dry_run(&self) -> bool {
         match self.dry_run {
             DryRun::Disabled => false,
             DryRun::SelfCheck | DryRun::UserSelected => true,

src/tools/miri/tests/pass/packed-struct-dyn-trait.rs

@@ -0,0 +1,21 @@
+// run-pass
+use std::ptr::addr_of;
+
+// When the unsized tail is a `dyn Trait`, its alignments is only dynamically known. This means the
+// packed(2) needs to be applied at runtime: the actual alignment of the field is `min(2,
+// usual_alignment)`. Here we check that we do this right by comparing size, alignment, and field
+// offset before and after unsizing.
+fn main() {
+    #[repr(C, packed(2))]
+    struct Packed<T: ?Sized>(u8, core::mem::ManuallyDrop<T>);
+
+    let p = Packed(0, core::mem::ManuallyDrop::new(1));
+    let p: &Packed<usize> = &p;
+    let sized = (core::mem::size_of_val(p), core::mem::align_of_val(p));
+    let sized_offset = unsafe { addr_of!(p.1).cast::<u8>().offset_from(addr_of!(p.0)) };
+    let p: &Packed<dyn Send> = p;
+    let un_sized = (core::mem::size_of_val(p), core::mem::align_of_val(p));
+    let un_sized_offset = unsafe { addr_of!(p.1).cast::<u8>().offset_from(addr_of!(p.0)) };
+    assert_eq!(sized, un_sized);
+    assert_eq!(sized_offset, un_sized_offset);
+}

src/tools/tidy/src/ui_tests.rs

@@ -11,7 +11,7 @@ use std::path::{Path, PathBuf};
 const ENTRY_LIMIT: usize = 900;
 // FIXME: The following limits should be reduced eventually.
 const ISSUES_ENTRY_LIMIT: usize = 1852;
-const ROOT_ENTRY_LIMIT: usize = 866;
+const ROOT_ENTRY_LIMIT: usize = 867;
 
 const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
     "rs",     // test source files

tests/ui/packed/dyn-trait.rs

@@ -0,0 +1,21 @@
+// run-pass
+use std::ptr::addr_of;
+
+// When the unsized tail is a `dyn Trait`, its alignments is only dynamically known. This means the
+// packed(2) needs to be applied at runtime: the actual alignment of the field is `min(2,
+// usual_alignment)`. Here we check that we do this right by comparing size, alignment, and field
+// offset before and after unsizing.
+fn main() {
+    #[repr(C, packed(2))]
+    struct Packed<T: ?Sized>(u8, core::mem::ManuallyDrop<T>);
+
+    let p = Packed(0, core::mem::ManuallyDrop::new(1));
+    let p: &Packed<usize> = &p;
+    let sized = (core::mem::size_of_val(p), core::mem::align_of_val(p));
+    let sized_offset = unsafe { addr_of!(p.1).cast::<u8>().offset_from(addr_of!(p.0)) };
+    let p: &Packed<dyn Send> = p;
+    let un_sized = (core::mem::size_of_val(p), core::mem::align_of_val(p));
+    let un_sized_offset = unsafe { addr_of!(p.1).cast::<u8>().offset_from(addr_of!(p.0)) };
+    assert_eq!(sized, un_sized);
+    assert_eq!(sized_offset, un_sized_offset);
+}

tests/ui/stable-mir-print/basic_function.rs

@@ -0,0 +1,14 @@
+// compile-flags: -Z unpretty=stable-mir -Z mir-opt-level=3
+// check-pass
+
+fn foo(i:i32) -> i32 {
+    i + 1
+}
+
+fn bar(vec: &mut Vec<i32>) -> Vec<i32> {
+    let mut new_vec = vec.clone();
+    new_vec.push(1);
+    new_vec
+}
+
+fn main(){}