From 374607d6b9abb6e005200e79d1817b4a4f609e41 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 12:53:28 +0100 Subject: [PATCH 1/2] 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 | 42 ++++++++++--------- 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 + .../intrinsics/const-eval-select-backtrace.rs | 5 +-- .../const-eval-select-backtrace.run.stderr | 2 +- .../intrinsics/const-eval-select-stability.rs | 2 +- .../ui/intrinsics/const-eval-select-x86_64.rs | 4 +- tests/ui/intrinsics/const-eval-select.rs | 4 +- .../effects/minicore.rs | 3 +- 18 files changed, 84 insertions(+), 43 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..9f8f99420f128 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,45 +2526,47 @@ 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 /// #![feature(const_eval_select)] /// #![feature(core_intrinsics)] /// # #![allow(internal_features)] - /// use std::hint::unreachable_unchecked; + /// # #![cfg_attr(bootstrap, allow(unused))] /// use std::intrinsics::const_eval_select; /// - /// // Crate A + /// // Standard library + /// # #[cfg(not(bootstrap))] /// 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) /// } + /// # #[cfg(bootstrap)] + /// # pub const fn inconsistent() -> i32 { 0 } /// - /// // 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( diff --git a/tests/ui/intrinsics/const-eval-select-backtrace.rs b/tests/ui/intrinsics/const-eval-select-backtrace.rs index d6b1c865bdfc7..ea4374f5d328e 100644 --- a/tests/ui/intrinsics/const-eval-select-backtrace.rs +++ b/tests/ui/intrinsics/const-eval-select-backtrace.rs @@ -12,8 +12,5 @@ fn uhoh() { const fn c() {} fn main() { - // safety: this is unsound and just used to test - unsafe { - std::intrinsics::const_eval_select((), c, uhoh); - } + std::intrinsics::const_eval_select((), c, uhoh); } diff --git a/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr b/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr index 3f196bd8abc39..8f38d54146b88 100644 --- a/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr +++ b/tests/ui/intrinsics/const-eval-select-backtrace.run.stderr @@ -1,3 +1,3 @@ -thread 'main' panicked at $DIR/const-eval-select-backtrace.rs:17:9: +thread 'main' panicked at $DIR/const-eval-select-backtrace.rs:15:5: Aaah! note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/tests/ui/intrinsics/const-eval-select-stability.rs b/tests/ui/intrinsics/const-eval-select-stability.rs index f9554decec16b..575bc0cadda1b 100644 --- a/tests/ui/intrinsics/const-eval-select-stability.rs +++ b/tests/ui/intrinsics/const-eval-select-stability.rs @@ -13,7 +13,7 @@ const fn nothing(){} #[stable(since = "1.0", feature = "hey")] #[rustc_const_stable(since = "1.0", feature = "const_hey")] -pub const unsafe fn hey() { +pub const fn hey() { const_eval_select((), nothing, log); //~^ ERROR `const_eval_select` is not yet stable as a const fn } diff --git a/tests/ui/intrinsics/const-eval-select-x86_64.rs b/tests/ui/intrinsics/const-eval-select-x86_64.rs index 5ba7a443d0bb9..c17be2ddaca39 100644 --- a/tests/ui/intrinsics/const-eval-select-x86_64.rs +++ b/tests/ui/intrinsics/const-eval-select-x86_64.rs @@ -22,9 +22,7 @@ fn eq_rt(x: [i32; 4], y: [i32; 4]) -> bool { } const fn eq(x: [i32; 4], y: [i32; 4]) -> bool { - unsafe { - const_eval_select((x, y), eq_ct, eq_rt) - } + const_eval_select((x, y), eq_ct, eq_rt) } fn main() { diff --git a/tests/ui/intrinsics/const-eval-select.rs b/tests/ui/intrinsics/const-eval-select.rs index 353105cb0a163..66d83585fcec8 100644 --- a/tests/ui/intrinsics/const-eval-select.rs +++ b/tests/ui/intrinsics/const-eval-select.rs @@ -13,9 +13,9 @@ fn no() -> bool { false } -// not a sound use case; testing only +// not allowed on stable; testing only const fn is_const_eval() -> bool { - unsafe { const_eval_select((), yes, no) } + const_eval_select((), yes, no) } fn main() { diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs index 5ee38078a2950..1b380c989fa4a 100644 --- a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs @@ -511,6 +511,7 @@ const fn drop(_: T) {} extern "rust-intrinsic" { #[rustc_const_stable(feature = "const_eval_select", since = "1.0.0")] + #[rustc_safe_intrinsic] fn const_eval_select( arg: ARG, called_in_const: F, @@ -525,5 +526,5 @@ fn test_const_eval_select() { const fn const_fn() {} fn rt_fn() {} - unsafe { const_eval_select((), const_fn, rt_fn); } + const_eval_select((), const_fn, rt_fn); } From d858809ca5f4a945377c47eb18fc7ed9202da24c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Mar 2024 19:03:50 +0100 Subject: [PATCH 2/2] typo Co-authored-by: bjorn3 <17426603+bjorn3@users.noreply.github.com> --- library/core/src/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 9f8f99420f128..5e93f495ddf1d 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2532,7 +2532,7 @@ extern "rust-intrinsic" { /// 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" + /// modes of execution. (Here, Undefined Behavior is considered "the same" /// as any other behavior, so if the function exhibits UB at runtime then /// it may do whatever it wants at compile-time.) ///