From 520662eb2ac157bf4bfcda83838c9dac89c9daf4 Mon Sep 17 00:00:00 2001 From: jyn Date: Sat, 9 Dec 2023 15:19:08 -0500 Subject: [PATCH 1/3] fix --dry-run when the change-id warning is printed --- src/bootstrap/src/bin/main.rs | 2 +- src/bootstrap/src/core/config/config.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/src/bin/main.rs b/src/bootstrap/src/bin/main.rs index 952ef40ed424d..3d593392e79f4 100644 --- a/src/bootstrap/src/bin/main.rs +++ b/src/bootstrap/src/bin/main.rs @@ -143,7 +143,7 @@ fn check_version(config: &Config) -> Option { "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())); } } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 22e8ce8365b1f..02b596f2b5aab 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1741,7 +1741,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, From 0a0231cf63f59fb4d7a87c3647eaa164a247bb75 Mon Sep 17 00:00:00 2001 From: ouz-a Date: Mon, 27 Nov 2023 18:40:00 +0300 Subject: [PATCH 2/3] add stable_mir output test --- compiler/stable_mir/src/mir/pretty.rs | 39 ++- src/tools/tidy/src/ui_tests.rs | 2 +- tests/ui/stable-mir-print/basic_function.rs | 14 ++ .../ui/stable-mir-print/basic_function.stdout | 234 ++++++++++++++++++ 4 files changed, 275 insertions(+), 14 deletions(-) create mode 100644 tests/ui/stable-mir-print/basic_function.rs create mode 100644 tests/ui/stable-mir-print/basic_function.stdout diff --git a/compiler/stable_mir/src/mir/pretty.rs b/compiler/stable_mir/src/mir/pretty.rs index 3a0eed521dc6e..f37b768b7e37c 100644 --- a/compiler/stable_mir/src/mir/pretty.rs +++ b/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()); diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 40149f8f1c3b6..dfa386b49de7c 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/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 diff --git a/tests/ui/stable-mir-print/basic_function.rs b/tests/ui/stable-mir-print/basic_function.rs new file mode 100644 index 0000000000000..4df0967a38da7 --- /dev/null +++ b/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) -> Vec { + let mut new_vec = vec.clone(); + new_vec.push(1); + new_vec +} + +fn main(){} diff --git a/tests/ui/stable-mir-print/basic_function.stdout b/tests/ui/stable-mir-print/basic_function.stdout new file mode 100644 index 0000000000000..d9b33a4257c2c --- /dev/null +++ b/tests/ui/stable-mir-print/basic_function.stdout @@ -0,0 +1,234 @@ +// WARNING: This is highly experimental output it's intended for stable-mir developers only. +// If you find a bug or want to improve the output open a issue at https://github.com/rust-lang/project-stable-mir. +fn foo(_0: i32) -> i32 { + let mut _0: (i32, bool); +} + bb0: { + _2 = 1 Add const 1_i32 + assert(!move _2 bool),"attempt to compute `{} + {}`, which would overflow", 1, const 1_i32) -> [success: bb1, unwind continue] + } + bb1: { + _0 = move _2 + return + } +fn bar(_0: &mut Ty { + id: 10, + kind: RigidTy( + Adt( + AdtDef( + DefId { + id: 3, + name: "std::vec::Vec", + }, + ), + GenericArgs( + [ + Type( + Ty { + id: 11, + kind: Param( + ParamTy { + index: 0, + name: "T", + }, + ), + }, + ), + Type( + Ty { + id: 12, + kind: Param( + ParamTy { + index: 1, + name: "A", + }, + ), + }, + ), + ], + ), + ), + ), +}) -> Ty { + id: 10, + kind: RigidTy( + Adt( + AdtDef( + DefId { + id: 3, + name: "std::vec::Vec", + }, + ), + GenericArgs( + [ + Type( + Ty { + id: 11, + kind: Param( + ParamTy { + index: 0, + name: "T", + }, + ), + }, + ), + Type( + Ty { + id: 12, + kind: Param( + ParamTy { + index: 1, + name: "A", + }, + ), + }, + ), + ], + ), + ), + ), +} { + let mut _0: Ty { + id: 10, + kind: RigidTy( + Adt( + AdtDef( + DefId { + id: 3, + name: "std::vec::Vec", + }, + ), + GenericArgs( + [ + Type( + Ty { + id: 11, + kind: Param( + ParamTy { + index: 0, + name: "T", + }, + ), + }, + ), + Type( + Ty { + id: 12, + kind: Param( + ParamTy { + index: 1, + name: "A", + }, + ), + }, + ), + ], + ), + ), + ), +}; + let mut _1: &Ty { + id: 10, + kind: RigidTy( + Adt( + AdtDef( + DefId { + id: 3, + name: "std::vec::Vec", + }, + ), + GenericArgs( + [ + Type( + Ty { + id: 11, + kind: Param( + ParamTy { + index: 0, + name: "T", + }, + ), + }, + ), + Type( + Ty { + id: 12, + kind: Param( + ParamTy { + index: 1, + name: "A", + }, + ), + }, + ), + ], + ), + ), + ), +}; + let _2: (); + let mut _3: &mut Ty { + id: 10, + kind: RigidTy( + Adt( + AdtDef( + DefId { + id: 3, + name: "std::vec::Vec", + }, + ), + GenericArgs( + [ + Type( + Ty { + id: 11, + kind: Param( + ParamTy { + index: 0, + name: "T", + }, + ), + }, + ), + Type( + Ty { + id: 12, + kind: Param( + ParamTy { + index: 1, + name: "A", + }, + ), + }, + ), + ], + ), + ), + ), +}; +} + bb0: { + _3 = refShared1 + _2 = const as Clone>::clone(move _3) -> [return: bb1, unwind continue] + } + bb1: { + _5 = refMut { + kind: TwoPhaseBorrow, +}2 + _4 = const Vec::::push(move _5, const 1_i32) -> [return: bb2, unwind: bb3] + } + bb2: { + _0 = move _2 + return + } + bb3: { + drop(_2) -> [return: bb4, unwind terminate] + } + bb4: { + resume + } +fn main() -> () { +} + bb0: { + return + } From 7e4c4271f44055b4dbd6f09aad19624254d67f22 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Dec 2023 13:40:24 +0100 Subject: [PATCH 3/3] fix computing the dynamic alignment of packed structs with dyn trait tails --- compiler/rustc_codegen_ssa/src/size_of_val.rs | 62 ++++++++++++------- .../src/interpret/eval_context.rs | 43 ++++++------- .../tests/pass/packed-struct-dyn-trait.rs | 21 +++++++ tests/ui/packed/dyn-trait.rs | 21 +++++++ 4 files changed, 99 insertions(+), 48 deletions(-) create mode 100644 src/tools/miri/tests/pass/packed-struct-dyn-trait.rs create mode 100644 tests/ui/packed/dyn-trait.rs diff --git a/compiler/rustc_codegen_ssa/src/size_of_val.rs b/compiler/rustc_codegen_ssa/src/size_of_val.rs index b8a4949d59ff9..087836ca37de0 100644 --- a/compiler/rustc_codegen_ssa/src/size_of_val.rs +++ b/compiler/rustc_codegen_ssa/src/size_of_val.rs @@ -84,10 +84,13 @@ 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 @@ -95,26 +98,26 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( 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"), } diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index bbebf329d2659..847d6503f20d8 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/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)?; diff --git a/src/tools/miri/tests/pass/packed-struct-dyn-trait.rs b/src/tools/miri/tests/pass/packed-struct-dyn-trait.rs new file mode 100644 index 0000000000000..bb73c26c18a0d --- /dev/null +++ b/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(u8, core::mem::ManuallyDrop); + + let p = Packed(0, core::mem::ManuallyDrop::new(1)); + let p: &Packed = &p; + let sized = (core::mem::size_of_val(p), core::mem::align_of_val(p)); + let sized_offset = unsafe { addr_of!(p.1).cast::().offset_from(addr_of!(p.0)) }; + let p: &Packed = 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::().offset_from(addr_of!(p.0)) }; + assert_eq!(sized, un_sized); + assert_eq!(sized_offset, un_sized_offset); +} diff --git a/tests/ui/packed/dyn-trait.rs b/tests/ui/packed/dyn-trait.rs new file mode 100644 index 0000000000000..bb73c26c18a0d --- /dev/null +++ b/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(u8, core::mem::ManuallyDrop); + + let p = Packed(0, core::mem::ManuallyDrop::new(1)); + let p: &Packed = &p; + let sized = (core::mem::size_of_val(p), core::mem::align_of_val(p)); + let sized_offset = unsafe { addr_of!(p.1).cast::().offset_from(addr_of!(p.0)) }; + let p: &Packed = 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::().offset_from(addr_of!(p.0)) }; + assert_eq!(sized, un_sized); + assert_eq!(sized_offset, un_sized_offset); +}