From 12f2bcde63285bf2b2f9f9ac615d4edf139a3f0e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 18 Sep 2024 16:45:27 -0400 Subject: [PATCH 1/5] Check params for unsafety in THIR --- .../rustc_mir_build/src/check_unsafety.rs | 14 ++++++++++++++ tests/ui/unsafe/union-pat-in-param.rs | 19 +++++++++++++++++++ tests/ui/unsafe/union-pat-in-param.stderr | 19 +++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 tests/ui/unsafe/union-pat-in-param.rs create mode 100644 tests/ui/unsafe/union-pat-in-param.stderr diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index c7fcfe3ce2aa0..57390dd58f4d9 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -218,6 +218,13 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> { warnings: self.warnings, suggest_unsafe_block: self.suggest_unsafe_block, }; + // params in THIR may be unsafe, e.g. a union pattern. + for param in &inner_thir.params { + if let Some(param_pat) = param.pat.as_deref() { + inner_visitor.visit_pat(param_pat); + } + } + // Visit the body. inner_visitor.visit_expr(&inner_thir[expr]); // Unsafe blocks can be used in the inner body, make sure to take it into account self.safety_context = inner_visitor.safety_context; @@ -1032,6 +1039,13 @@ pub(crate) fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) { warnings: &mut warnings, suggest_unsafe_block: true, }; + // params in THIR may be unsafe, e.g. a union pattern. + for param in &thir.params { + if let Some(param_pat) = param.pat.as_deref() { + visitor.visit_pat(param_pat); + } + } + // Visit the body. visitor.visit_expr(&thir[expr]); warnings.sort_by_key(|w| w.block_span); diff --git a/tests/ui/unsafe/union-pat-in-param.rs b/tests/ui/unsafe/union-pat-in-param.rs new file mode 100644 index 0000000000000..8454bfb20dcb8 --- /dev/null +++ b/tests/ui/unsafe/union-pat-in-param.rs @@ -0,0 +1,19 @@ +union U { + a: &'static i32, + b: usize, +} + +fn fun(U { a }: U) { + //~^ ERROR access to union field is unsafe + dbg!(*a); +} + +fn main() { + fun(U { b: 0 }); + + let closure = |U { a }| { + //~^ ERROR access to union field is unsafe + dbg!(*a); + }; + closure(U { b: 0 }); +} diff --git a/tests/ui/unsafe/union-pat-in-param.stderr b/tests/ui/unsafe/union-pat-in-param.stderr new file mode 100644 index 0000000000000..b9709b7cc9bf1 --- /dev/null +++ b/tests/ui/unsafe/union-pat-in-param.stderr @@ -0,0 +1,19 @@ +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/union-pat-in-param.rs:6:12 + | +LL | fn fun(U { a }: U) { + | ^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/union-pat-in-param.rs:14:24 + | +LL | let closure = |U { a }| { + | ^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0133`. From e138e8760da3d75def4d23567ae944d65b2d4e89 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 18 Sep 2024 17:05:44 -0400 Subject: [PATCH 2/5] Never patterns constitute a read for unsafety --- .../rustc_mir_build/src/check_unsafety.rs | 9 +++++---- .../never-pattern-is-a-read.rs | 16 +++++++++++++++ .../never-pattern-is-a-read.stderr | 20 +++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.rs create mode 100644 tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.stderr diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index c7fcfe3ce2aa0..198698d06fa63 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -315,14 +315,15 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { | PatKind::DerefPattern { .. } | PatKind::Range { .. } | PatKind::Slice { .. } - | PatKind::Array { .. } => { + | PatKind::Array { .. } + // Never constitutes a witness of uninhabitedness. + | PatKind::Never => { self.requires_unsafe(pat.span, AccessToUnionField); return; // we can return here since this already requires unsafe } - // wildcard/never don't take anything + // wildcard doesn't read anything. PatKind::Wild | - PatKind::Never | - // these just wrap other patterns + // these just wrap other patterns, which we recurse on below. PatKind::Or { .. } | PatKind::InlineConstant { .. } | PatKind::AscribeUserType { .. } | diff --git a/tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.rs b/tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.rs new file mode 100644 index 0000000000000..a66c446441778 --- /dev/null +++ b/tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.rs @@ -0,0 +1,16 @@ +// Make sure we consider `!` to be a union read. + +#![feature(never_type, never_patterns)] +//~^ WARN the feature `never_patterns` is incomplete + +union U { + a: !, + b: usize, +} + +fn foo(u: U) -> ! { + let U { a: ! } = u; + //~^ ERROR access to union field is unsafe +} + +fn main() {} diff --git a/tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.stderr b/tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.stderr new file mode 100644 index 0000000000000..d7dc7a47e7283 --- /dev/null +++ b/tests/ui/rfcs/rfc-0000-never_patterns/never-pattern-is-a-read.stderr @@ -0,0 +1,20 @@ +warning: the feature `never_patterns` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/never-pattern-is-a-read.rs:3:24 + | +LL | #![feature(never_type, never_patterns)] + | ^^^^^^^^^^^^^^ + | + = note: see issue #118155 for more information + = note: `#[warn(incomplete_features)]` on by default + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/never-pattern-is-a-read.rs:12:16 + | +LL | let U { a: ! } = u; + | ^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error: aborting due to 1 previous error; 1 warning emitted + +For more information about this error, try `rustc --explain E0133`. From 7477f3eb350f37fbd376929b6b82b9e6bdd7621f Mon Sep 17 00:00:00 2001 From: ultrabear Date: Wed, 18 Sep 2024 20:08:52 -0700 Subject: [PATCH 3/5] stabilize `const_maybe_uninit_as_mut_ptr` --- library/core/src/mem/maybe_uninit.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index 0154caa7c24dd..d274445f5e4a7 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -393,7 +393,6 @@ impl MaybeUninit { // These are OK to allow since we do not leak &mut to user-visible API #[rustc_allow_const_fn_unstable(const_mut_refs)] #[rustc_allow_const_fn_unstable(const_ptr_write)] - #[rustc_allow_const_fn_unstable(const_maybe_uninit_as_mut_ptr)] #[rustc_const_stable(feature = "const_maybe_uninit_zeroed", since = "1.75.0")] pub const fn zeroed() -> MaybeUninit { let mut u = MaybeUninit::::uninit(); @@ -570,7 +569,8 @@ impl MaybeUninit { /// (Notice that the rules around references to uninitialized data are not finalized yet, but /// until they are, it is advisable to avoid them.) #[stable(feature = "maybe_uninit", since = "1.36.0")] - #[rustc_const_unstable(feature = "const_maybe_uninit_as_mut_ptr", issue = "75251")] + #[rustc_const_stable(feature = "const_maybe_uninit_as_mut_ptr", since = "CURRENT_RUSTC_VERSION")] + #[cfg_attr(bootstrap, rustc_allow_const_fn_unstable(const_mut_refs))] #[inline(always)] pub const fn as_mut_ptr(&mut self) -> *mut T { // `MaybeUninit` and `ManuallyDrop` are both `repr(transparent)` so we can cast the pointer. From 63f14b3a1ef7176e8bef3de4ba21579235cd534a Mon Sep 17 00:00:00 2001 From: ultrabear Date: Wed, 18 Sep 2024 20:18:05 -0700 Subject: [PATCH 4/5] remove feature attributes as const_maybe_uninit_as_mut_ptr is stabilized --- library/alloc/src/lib.rs | 1 - library/core/src/lib.rs | 1 - library/core/tests/lib.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index f98c0cca1db47..f0597f295b3f0 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -110,7 +110,6 @@ #![feature(const_cow_is_borrowed)] #![feature(const_eval_select)] #![feature(const_heap)] -#![feature(const_maybe_uninit_as_mut_ptr)] #![feature(const_maybe_uninit_write)] #![feature(const_option)] #![feature(const_pin)] diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 348ccebea3b84..e5a3d165a4351 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -131,7 +131,6 @@ #![feature(const_ipv4)] #![feature(const_ipv6)] #![feature(const_likely)] -#![feature(const_maybe_uninit_as_mut_ptr)] #![feature(const_maybe_uninit_assume_init)] #![feature(const_nonnull_new)] #![feature(const_num_midpoint)] diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index a0d6efc174392..5315ac856f692 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -26,7 +26,6 @@ #![feature(const_ipv4)] #![feature(const_ipv6)] #![feature(const_likely)] -#![feature(const_maybe_uninit_as_mut_ptr)] #![feature(const_nonnull_new)] #![feature(const_option)] #![feature(const_option_ext)] From b7ca2b6510b854cd44fc8c46b113ef3e45986ab6 Mon Sep 17 00:00:00 2001 From: ultrabear Date: Wed, 18 Sep 2024 20:49:53 -0700 Subject: [PATCH 5/5] run `x.py fmt` --- library/core/src/mem/maybe_uninit.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/core/src/mem/maybe_uninit.rs b/library/core/src/mem/maybe_uninit.rs index d274445f5e4a7..c67796ad3db83 100644 --- a/library/core/src/mem/maybe_uninit.rs +++ b/library/core/src/mem/maybe_uninit.rs @@ -569,7 +569,10 @@ impl MaybeUninit { /// (Notice that the rules around references to uninitialized data are not finalized yet, but /// until they are, it is advisable to avoid them.) #[stable(feature = "maybe_uninit", since = "1.36.0")] - #[rustc_const_stable(feature = "const_maybe_uninit_as_mut_ptr", since = "CURRENT_RUSTC_VERSION")] + #[rustc_const_stable( + feature = "const_maybe_uninit_as_mut_ptr", + since = "CURRENT_RUSTC_VERSION" + )] #[cfg_attr(bootstrap, rustc_allow_const_fn_unstable(const_mut_refs))] #[inline(always)] pub const fn as_mut_ptr(&mut self) -> *mut T {