diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index 2218780092287..521043e3f6c1a 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -1132,6 +1132,11 @@ impl LayoutCalculator { // If `-Z randomize-layout` was enabled for the type definition we can shuffle // the field ordering to try and catch some code making assumptions about layouts // we don't guarantee. + // In the future, we might do more than shuffle field order (e.g. introduce extra padding), + // but never for `repr(Rust)` structs with only zero-sized fields, single-variant + // `repr(Rust)` enums with only zero-sized fields, or zero-variant `repr(Rust)` enums, + // which must remain zero-sized as per T-lang decisions in + // https://github.com/rust-lang/reference/pull/2262 and https://github.com/rust-lang/reference/pull/2293 if repr.can_randomize_type_layout() && cfg!(feature = "randomize") { #[cfg(feature = "randomize")] { diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index bff4c9bdf47ef..679523341c7e3 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -90,6 +90,10 @@ bitflags! { /// If true, the type's crate has opted into layout randomization. /// Other flags can still inhibit reordering and thus randomization. /// The seed stored in `ReprOptions.field_shuffle_seed`. + /// + /// `repr(Rust)` structs with only zero-sized fields, single-variant `repr(Rust)` enums with only + /// zero-sized fields, and zero-variant `repr(Rust)` enums must remain zero-sized as per + /// T-lang decisions in https://github.com/rust-lang/reference/pull/2262 and https://github.com/rust-lang/reference/pull/2293 const RANDOMIZE_LAYOUT = 1 << 4; /// If true, the type is always passed indirectly by non-Rustic ABIs. /// See [`TyAndLayout::pass_indirectly_in_non_rustic_abis`] for details. diff --git a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs index 2b786b7e9e1a2..7adeebd235384 100644 --- a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs +++ b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs @@ -243,7 +243,7 @@ fn evaluate_candidate<'tcx>( return None; } - // We only handle: + // For now, we only handle: // ``` // bb4: { // _8 = discriminant((_3.1: Enum1)); @@ -262,41 +262,8 @@ fn evaluate_candidate<'tcx>( // When thie BB has exactly one statement, this statement should be discriminant. let need_hoist_discriminant = bbs[child].statements.len() == 1; + let otherwise_is_empty_unreachable = bbs[targets.otherwise()].is_empty_unreachable(); let child_place = if need_hoist_discriminant { - if !bbs[targets.otherwise()].is_empty_unreachable() { - // Someone could write code like this: - // ```rust - // let Q = val; - // if discriminant(P) == otherwise { - // let ptr = &mut Q as *mut _ as *mut u8; - // // It may be difficult for us to effectively determine whether values are valid. - // // Invalid values can come from all sorts of corners. - // unsafe { *ptr = 10; } - // } - // - // match P { - // A => match Q { - // A => { - // // code - // } - // _ => { - // // don't use Q - // } - // } - // _ => { - // // don't use Q - // } - // }; - // ``` - // - // Hoisting the `discriminant(Q)` out of the `A` arm causes us to compute the discriminant of an - // invalid value, which is UB. - // In order to fix this, **we would either need to show that the discriminant computation of - // `place` is computed in all branches**. - // FIXME(#95162) For the moment, we adopt a conservative approach and - // consider only the `otherwise` branch has no statements and an unreachable terminator. - return None; - } // Handle: // ``` // bb4: { @@ -325,8 +292,7 @@ fn evaluate_candidate<'tcx>( }; *child_place }; - let destination = if need_hoist_discriminant || bbs[targets.otherwise()].is_empty_unreachable() - { + let destination = if otherwise_is_empty_unreachable { child_targets.otherwise() } else { targets.otherwise() @@ -340,6 +306,7 @@ fn evaluate_candidate<'tcx>( child_place, destination, need_hoist_discriminant, + otherwise_is_empty_unreachable, ) { return None; } @@ -359,11 +326,67 @@ fn verify_candidate_branch<'tcx>( place: Place<'tcx>, destination: BasicBlock, need_hoist_discriminant: bool, + otherwise_is_empty_unreachable: bool, ) -> bool { // In order for the optimization to be correct, the terminator must be a `SwitchInt`. let TerminatorKind::SwitchInt { discr: switch_op, targets } = &branch.terminator().kind else { return false; }; + if !otherwise_is_empty_unreachable { + // Someone could write code like this: + // ```rust + // let Q = val; + // if discriminant(P) == otherwise { + // let ptr = &mut Q as *mut _ as *mut u8; + // // It may be difficult for us to effectively determine whether values are valid. + // // Invalid values can come from all sorts of corners. + // unsafe { *ptr = 10; } + // } + // + // match P { + // A => match Q { + // A => { + // // code + // } + // _ => { + // // don't use Q + // } + // } + // _ => { + // // don't use Q + // } + // }; + // ``` + // + // Hoisting the `discriminant(Q)` out of the `A` arm causes us to compute the discriminant of an + // invalid value, which is UB. + // In order to fix this, **we would either need to show that the discriminant computation of + // `place` is computed in all branches**. + // For , we adopt a conservative approach and + // consider only the `otherwise` branch has no statements and an unreachable terminator. + if need_hoist_discriminant { + return false; + } + // For : + // ``` + // bb0: { + // switchInt(copy _1) -> [1: bb1, 2: bb2, otherwise: bb5]; + // } + // bb1: { + // switchInt(copy (*_2)) -> [1: bb3, otherwise: bb5]; + // } + // bb2: { + // switchInt(copy (*_2)) -> [2: bb4, otherwise: bb5]; + // } + // ``` + // We cannot hoist the dereference of `_2` to `bb0`, + // because execution can reach `bb5` without dereferencing `_2`. + if let Some(place) = switch_op.place() + && !place.is_stable_offset() + { + return false; + } + } if need_hoist_discriminant { // If we need hoist discriminant, the branch must have exactly one statement. let [statement] = branch.statements.as_slice() else { diff --git a/src/doc/rustc/src/symbol-mangling/index.md b/src/doc/rustc/src/symbol-mangling/index.md index 3f7d55063ca6a..67df4f0a9b07f 100644 --- a/src/doc/rustc/src/symbol-mangling/index.md +++ b/src/doc/rustc/src/symbol-mangling/index.md @@ -17,13 +17,16 @@ The [`#[export_name]`attribute][reference-export_name] can be used to specify th Items listed in an [`extern` block][reference-extern-block] use the identifier of the item without mangling to refer to the item. The [`#[link_name]` attribute][reference-link_name] can be used to change that name. - +### WebAssembly import modules + +On WebAssembly targets, foreign items in `extern` blocks can use the same import name without +conflicting when they use different [`#[link(wasm_import_module = "...")]`][reference-link-attribute] +values. [reference-no_mangle]: ../../reference/abi.html#the-no_mangle-attribute [reference-export_name]: ../../reference/abi.html#the-export_name-attribute [reference-link_name]: ../../reference/items/external-blocks.html#the-link_name-attribute +[reference-link-attribute]: ../../reference/items/external-blocks.html#the-link-attribute [reference-extern-block]: ../../reference/items/external-blocks.html ## Decoding diff --git a/tests/assembly-llvm/asm/aarch64-types.rs b/tests/assembly-llvm/asm/aarch64-types.rs index 21f9294dc647b..c171ba3b11e13 100644 --- a/tests/assembly-llvm/asm/aarch64-types.rs +++ b/tests/assembly-llvm/asm/aarch64-types.rs @@ -1,8 +1,10 @@ //@ add-minicore -//@ revisions: aarch64 arm64ec +//@ revisions: aarch64 aarch64_be arm64ec //@ assembly-output: emit-asm //@ [aarch64] compile-flags: --target aarch64-unknown-linux-gnu //@ [aarch64] needs-llvm-components: aarch64 +//@ [aarch64_be] compile-flags: --target aarch64_be-unknown-linux-gnu +//@ [aarch64_be] needs-llvm-components: aarch64 //@ [arm64ec] compile-flags: --target arm64ec-pc-windows-msvc //@ [arm64ec] needs-llvm-components: aarch64 //@ compile-flags: -Zmerge-functions=disabled diff --git a/tests/mir-opt/early_otherwise_branch.dont_hoist_deref.EarlyOtherwiseBranch.diff b/tests/mir-opt/early_otherwise_branch.dont_hoist_deref.EarlyOtherwiseBranch.diff new file mode 100644 index 0000000000000..66891f818a6cf --- /dev/null +++ b/tests/mir-opt/early_otherwise_branch.dont_hoist_deref.EarlyOtherwiseBranch.diff @@ -0,0 +1,38 @@ +- // MIR for `dont_hoist_deref` before EarlyOtherwiseBranch ++ // MIR for `dont_hoist_deref` after EarlyOtherwiseBranch + + fn dont_hoist_deref(_1: u64, _2: *const u64) -> u64 { + let mut _0: u64; + + bb0: { + switchInt(copy _1) -> [1: bb1, 2: bb2, otherwise: bb5]; + } + + bb1: { + switchInt(copy (*_2)) -> [1: bb3, otherwise: bb5]; + } + + bb2: { + switchInt(copy (*_2)) -> [2: bb4, otherwise: bb5]; + } + + bb3: { + _0 = const 100_u64; + goto -> bb6; + } + + bb4: { + _0 = const 200_u64; + goto -> bb6; + } + + bb5: { + _0 = const 999_u64; + goto -> bb6; + } + + bb6: { + return; + } + } + diff --git a/tests/mir-opt/early_otherwise_branch.rs b/tests/mir-opt/early_otherwise_branch.rs index 19a5d25de2dfb..54406f41f8fc1 100644 --- a/tests/mir-opt/early_otherwise_branch.rs +++ b/tests/mir-opt/early_otherwise_branch.rs @@ -156,6 +156,56 @@ fn target_self(val: i32) { } } +// EMIT_MIR early_otherwise_branch.dont_hoist_deref.EarlyOtherwiseBranch.diff +#[custom_mir(dialect = "runtime")] +fn dont_hoist_deref(q: u64, p: *const u64) -> u64 { + // The dereference of `p` cannot be hoisted because the `otherwise` branch + // can be taken without dereferencing `p`. + // Hoisting the dereference could therefore cause UB when `p` is null. + // Hoisting a dereference also requires proving that the dereference is safe to reorder. + // CHECK-LABEL: fn dont_hoist_deref( + // CHECK: bb0: { + // CHECK-NEXT: switchInt(copy _1) + // CHECK: switchInt(copy (*_2)) + // CHECK: switchInt(copy (*_2)) + mir! { + { + match q { + 1 => bb1, + 2 => bb2, + _ => bb5, + } + } + bb1 = { + match *p { + 1 => bb3, + _ => bb5, + } + } + bb2 = { + match *p { + 2 => bb4, + _ => bb5, + } + } + bb3 = { + RET = 100; + Goto(bb6) + } + bb4 = { + RET = 200; + Goto(bb6) + } + bb5 = { + RET = 999; + Goto(bb6) + } + bb6 = { + Return() + } + } +} + fn main() { opt1(None, Some(0)); opt2(None, Some(0)); @@ -164,4 +214,5 @@ fn main() { opt5(0, 0); opt5_failed(0, 0); target_self(1); + dont_hoist_deref(3, std::ptr::null()); } diff --git a/tests/ui/layout/randomize.rs b/tests/ui/layout/randomize.rs index 27e99327a3196..eaeb527f191cb 100644 --- a/tests/ui/layout/randomize.rs +++ b/tests/ui/layout/randomize.rs @@ -49,6 +49,52 @@ const _: () = { assert!(std::mem::offset_of!(Result::<&usize, ()>, Ok.0) == 0); }; +// these types only have their size checked, they're never constructed. +// these repr(Rust) types must remain zero-sized. +#[allow(dead_code)] +pub struct UnitStruct; +#[allow(dead_code)] +pub struct EmptyTupleStruct(); +#[allow(dead_code)] +pub struct EmptyStruct {} +#[allow(dead_code)] +pub struct ZstFieldsTupleStruct((), [u64; 0], [u8; 0], [(); 42]); +#[allow(dead_code)] +pub struct ZstFieldsStruct { + a: (), + b: [u64; 0], + c: [u8; 0], + d: [(); 42], +} +#[allow(dead_code)] +pub enum EmptyEnum {} +#[allow(dead_code)] +pub enum SingleUnitVariantEnum { A } +#[allow(dead_code)] +pub enum SingleZstFieldTupleVariantEnum { A((), [u64; 0], [u8; 0], [(); 42]) } +#[allow(dead_code)] +pub enum SingleZstFieldVariantEnum { + A { + a: (), + b: [u64; 0], + c: [u8; 0], + d: [(); 42], + } +} + +// all these types must remain zero-sized. +const _: () = { + assert!(size_of::() == 0); + assert!(size_of::() == 0); + assert!(size_of::() == 0); + assert!(size_of::() == 0); + assert!(size_of::() == 0); + assert!(size_of::() == 0); + assert!(size_of::() == 0); + assert!(size_of::() == 0); + assert!(size_of::() == 0); +}; + #[allow(dead_code)] struct Unsizable(usize, T); diff --git a/tests/ui/mir/hoist_deref_early_else.rs b/tests/ui/mir/hoist_deref_early_else.rs new file mode 100644 index 0000000000000..fb749e96d7c08 --- /dev/null +++ b/tests/ui/mir/hoist_deref_early_else.rs @@ -0,0 +1,36 @@ +//! Regression test for issue . +//! The null pointer `p` should never be dereferenced. +//@ run-pass +//@ revisions: noopt opt +//@ check-run-results +//@[noopt] compile-flags: -C opt-level=0 +//@[opt] compile-flags: -C opt-level=3 + +use std::hint::black_box; + +#[inline(never)] +fn foo(q: u64, p: *const u64) -> u64 { + unsafe { + 'a: { + match q { + 1 => match *p { + 1 => break 'a 100, + _ => {} + }, + 2 => match *p { + 2 => break 'a 200, + _ => {} + }, + _ => {} + } + 999 + } + } +} + +fn main() { + let q: u64 = black_box(3); + let p: *const u64 = black_box(std::ptr::null()); + let r = foo(q, p); + assert_eq!(999, black_box(r)); +}