From 4dbd9a9db6614f3f3f53fc4ef98b5b2abdce31d2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 12:53:28 +0100 Subject: [PATCH] const_eval_select: make it safe but be careful with what we expose on stable for now --- .../rustc_hir_analysis/src/check/intrinsic.rs | 3 +- library/alloc/src/alloc.rs | 1 + library/core/src/ffi/c_str.rs | 10 ++++- library/core/src/intrinsics.rs | 37 ++++++++++--------- library/core/src/num/f32.rs | 10 ++++- library/core/src/num/f64.rs | 10 ++++- library/core/src/panicking.rs | 1 + library/core/src/ptr/const_ptr.rs | 11 +++++- library/core/src/ptr/mod.rs | 1 + library/core/src/ptr/mut_ptr.rs | 10 ++++- library/core/src/slice/index.rs | 7 +++- library/core/src/str/mod.rs | 1 + 12 files changed, 72 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 6e9b4236e2088..98e81a0e0f902 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -128,7 +128,8 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) - | sym::fsub_algebraic | sym::fmul_algebraic | sym::fdiv_algebraic - | sym::frem_algebraic => hir::Unsafety::Normal, + | sym::frem_algebraic + | sym::const_eval_select => hir::Unsafety::Normal, _ => hir::Unsafety::Unsafe, }; diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 0b1429397559d..e99d6268e535b 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -385,6 +385,7 @@ pub const fn handle_alloc_error(layout: Layout) -> ! { } #[cfg(not(feature = "panic_immediate_abort"))] + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block unsafe { core::intrinsics::const_eval_select((layout,), ct_error, rt_error) } diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 20186a2de0fd8..0825281090cd1 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -428,10 +428,13 @@ impl CStr { unsafe { &*(bytes as *const [u8] as *const CStr) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The const and runtime versions have identical behavior // unless the safety contract of `from_bytes_with_nul_unchecked` is // violated, which is UB. - unsafe { intrinsics::const_eval_select((bytes,), const_impl, rt_impl) } + unsafe { + intrinsics::const_eval_select((bytes,), const_impl, rt_impl) + } } /// Returns the inner pointer to this C string. @@ -719,6 +722,9 @@ const unsafe fn const_strlen(ptr: *const c_char) -> usize { unsafe { strlen(s) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: the two functions always provide equivalent functionality - unsafe { intrinsics::const_eval_select((ptr,), strlen_ct, strlen_rt) } + unsafe { + intrinsics::const_eval_select((ptr,), strlen_ct, strlen_rt) + } } diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 96e667d63c5f3..a96a851770f56 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2515,6 +2515,8 @@ extern "rust-intrinsic" { /// intrinsic will be replaced with a call to `called_in_const`. It gets /// replaced with a call to `called_at_rt` otherwise. /// + /// This function is safe to call, but note the stability concerns below. + /// /// # Type Requirements /// /// The two functions must be both function items. They cannot be function @@ -2524,13 +2526,15 @@ extern "rust-intrinsic" { /// the two functions, therefore, both functions must accept the same type of /// arguments. Both functions must return RET. /// - /// # Safety + /// # Stability concerns /// - /// The two functions must behave observably equivalent. Safe code in other - /// crates may assume that calling a `const fn` at compile-time and at run-time - /// produces the same result. A function that produces a different result when - /// evaluated at run-time, or has any other observable side-effects, is - /// *unsound*. + /// Rust has not yet decided that `const fn` are allowed to tell whether + /// they run at compile-time or at runtime. Therefore, when using this + /// intrinsic anywhere that can be reached from stable, it is crucial that + /// the end-to-end behavior of the stable `const fn` is the same for both + /// modes of execution. (Here, Undefined Behavior is considerd "the same" + /// as any other behavior, so if the function exhibits UB at runtime then + /// it may do whatever it wants at compile-time.) /// /// Here is an example of how this could cause a problem: /// ```no_run @@ -2540,29 +2544,26 @@ extern "rust-intrinsic" { /// use std::hint::unreachable_unchecked; /// use std::intrinsics::const_eval_select; /// - /// // Crate A + /// // Standard library /// pub const fn inconsistent() -> i32 { /// fn runtime() -> i32 { 1 } /// const fn compiletime() -> i32 { 2 } /// - /// unsafe { - // // ⚠ This code violates the required equivalence of `compiletime` - /// // and `runtime`. - /// const_eval_select((), compiletime, runtime) - /// } + // // ⚠ This code violates the required equivalence of `compiletime` + /// // and `runtime`. + /// const_eval_select((), compiletime, runtime) /// } /// - /// // Crate B + /// // User Crate /// const X: i32 = inconsistent(); /// let x = inconsistent(); - /// if x != X { unsafe { unreachable_unchecked(); }} + /// assert_eq!(x, X); /// ``` /// - /// This code causes Undefined Behavior when being run, since the - /// `unreachable_unchecked` is actually being reached. The bug is in *crate A*, - /// which violates the principle that a `const fn` must behave the same at - /// compile-time and at run-time. The unsafe code in crate B is fine. + /// Currently such an assertion would always succeed; until Rust decides + /// otherwise, that principle should not be violated. #[rustc_const_unstable(feature = "const_eval_select", issue = "none")] + #[cfg_attr(not(bootstrap), rustc_safe_intrinsic)] pub fn const_eval_select( arg: ARG, called_in_const: F, diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index 47e16018a47ce..abdcb7099cacb 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -1153,8 +1153,11 @@ impl f32 { // Stability concerns. unsafe { mem::transmute(x) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((self,), ct_f32_to_u32, rt_f32_to_u32) } + unsafe { + intrinsics::const_eval_select((self,), ct_f32_to_u32, rt_f32_to_u32) + } } /// Raw transmutation from `u32`. @@ -1245,8 +1248,11 @@ impl f32 { // Stability concerns. unsafe { mem::transmute(x) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((v,), ct_u32_to_f32, rt_u32_to_f32) } + unsafe { + intrinsics::const_eval_select((v,), ct_u32_to_f32, rt_u32_to_f32) + } } /// Return the memory representation of this floating point number as a byte array in diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index cd69e758d28da..f4d2a4f216734 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -1146,8 +1146,11 @@ impl f64 { // Stability concerns. unsafe { mem::transmute::(rt) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((self,), ct_f64_to_u64, rt_f64_to_u64) } + unsafe { + intrinsics::const_eval_select((self,), ct_f64_to_u64, rt_f64_to_u64) + } } /// Raw transmutation from `u64`. @@ -1243,8 +1246,11 @@ impl f64 { // Stability concerns. unsafe { mem::transmute::(rt) } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: We use internal implementations that either always work or fail at compile time. - unsafe { intrinsics::const_eval_select((v,), ct_u64_to_f64, rt_u64_to_f64) } + unsafe { + intrinsics::const_eval_select((v,), ct_u64_to_f64, rt_u64_to_f64) + } } /// Return the memory representation of this floating point number as a byte array in diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 0819334b600b6..9e1184c8f5b88 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -117,6 +117,7 @@ pub const fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: boo panic_fmt(fmt); } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: const panic does not care about unwinding unsafe { super::intrinsics::const_eval_select((fmt, force_no_backtrace), comptime, runtime); diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 85a56d37ab75c..f2566fe9bccae 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -48,8 +48,11 @@ impl *const T { } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self as *const u8,), const_impl, runtime_impl) } + unsafe { + const_eval_select((self as *const u8,), const_impl, runtime_impl) + } } /// Casts to a pointer of another type. @@ -806,6 +809,7 @@ impl *const T { where T: Sized, { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The comparison has no side-effects, and the intrinsic // does this check internally in the CTFE implementation. unsafe { @@ -1623,8 +1627,11 @@ impl *const T { ptr.align_offset(align) == 0 } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) } + unsafe { + const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) + } } } diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index fc5b08c9801a8..c0a3bbdb7048e 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -991,6 +991,7 @@ pub const unsafe fn swap_nonoverlapping(x: *mut T, y: *mut T, count: usize) { }; } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: the caller must guarantee that `x` and `y` are // valid for writes and properly aligned. unsafe { diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 28ba26f5c16c4..8b0b22a02f2ba 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -48,8 +48,11 @@ impl *mut T { } } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self as *mut u8,), const_impl, runtime_impl) } + unsafe { + const_eval_select((self as *mut u8,), const_impl, runtime_impl) + } } /// Casts to a pointer of another type. @@ -1896,8 +1899,11 @@ impl *mut T { ptr.align_offset(align) == 0 } + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. - unsafe { const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) } + unsafe { + const_eval_select((self.cast::<()>(), align), const_impl, runtime_impl) + } } } diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index 0e2d45c4ada6d..c771ea7047242 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -35,6 +35,7 @@ where #[track_caller] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")] const fn slice_start_index_len_fail(index: usize, len: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: we are just panicking here unsafe { const_eval_select( @@ -63,6 +64,7 @@ const fn slice_start_index_len_fail_ct(_: usize, _: usize) -> ! { #[track_caller] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")] const fn slice_end_index_len_fail(index: usize, len: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: we are just panicking here unsafe { const_eval_select((index, len), slice_end_index_len_fail_ct, slice_end_index_len_fail_rt) @@ -87,8 +89,11 @@ const fn slice_end_index_len_fail_ct(_: usize, _: usize) -> ! { #[track_caller] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")] const fn slice_index_order_fail(index: usize, end: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: we are just panicking here - unsafe { const_eval_select((index, end), slice_index_order_fail_ct, slice_index_order_fail_rt) } + unsafe { + const_eval_select((index, end), slice_index_order_fail_ct, slice_index_order_fail_rt) + } } // FIXME const-hack diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index f965a50058b1b..4943bbc45d07a 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -86,6 +86,7 @@ use iter::{MatchesInternal, SplitNInternal}; #[rustc_allow_const_fn_unstable(const_eval_select)] #[cfg(not(feature = "panic_immediate_abort"))] const fn slice_error_fail(s: &str, begin: usize, end: usize) -> ! { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: panics for both branches unsafe { crate::intrinsics::const_eval_select(