Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

const_eval_select: make it safe but be careful with what we expose on stable for now #121894

Merged
merged 2 commits into from Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_hir_analysis/src/check/intrinsic.rs
Expand Up @@ -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,
};

Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/alloc.rs
Expand Up @@ -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)
}
Expand Down
10 changes: 8 additions & 2 deletions library/core/src/ffi/c_str.rs
Expand Up @@ -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)
}
Comment on lines 434 to +437
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rustfmt changes block wrapping based on whether there is an attribute? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird, asked about it here rust-lang/rustfmt#6106

}

/// Returns the inner pointer to this C string.
Expand Down Expand Up @@ -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)
}
}
42 changes: 23 additions & 19 deletions library/core/src/intrinsics.rs
Expand Up @@ -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
Expand All @@ -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 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.)
///
/// 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: Tuple, F, G, RET>(
arg: ARG,
called_in_const: F,
Expand Down
10 changes: 8 additions & 2 deletions library/core/src/num/f32.rs
Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions library/core/src/num/f64.rs
Expand Up @@ -1146,8 +1146,11 @@ impl f64 {
// Stability concerns.
unsafe { mem::transmute::<f64, u64>(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`.
Expand Down Expand Up @@ -1243,8 +1246,11 @@ impl f64 {
// Stability concerns.
unsafe { mem::transmute::<u64, f64>(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
Expand Down
1 change: 1 addition & 0 deletions library/core/src/panicking.rs
Expand Up @@ -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);
Expand Down
11 changes: 9 additions & 2 deletions library/core/src/ptr/const_ptr.rs
Expand Up @@ -48,8 +48,11 @@ impl<T: ?Sized> *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.
Expand Down Expand Up @@ -806,6 +809,7 @@ impl<T: ?Sized> *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 {
Expand Down Expand Up @@ -1623,8 +1627,11 @@ impl<T: ?Sized> *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)
}
}
}

Expand Down
1 change: 1 addition & 0 deletions library/core/src/ptr/mod.rs
Expand Up @@ -991,6 +991,7 @@ pub const unsafe fn swap_nonoverlapping<T>(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 {
Expand Down
10 changes: 8 additions & 2 deletions library/core/src/ptr/mut_ptr.rs
Expand Up @@ -48,8 +48,11 @@ impl<T: ?Sized> *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.
Expand Down Expand Up @@ -1896,8 +1899,11 @@ impl<T: ?Sized> *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)
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion library/core/src/slice/index.rs
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions library/core/src/str/mod.rs
Expand Up @@ -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(
Expand Down
5 changes: 1 addition & 4 deletions tests/ui/intrinsics/const-eval-select-backtrace.rs
Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion 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
2 changes: 1 addition & 1 deletion tests/ui/intrinsics/const-eval-select-stability.rs
Expand Up @@ -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
}
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/intrinsics/const-eval-select-x86_64.rs
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/intrinsics/const-eval-select.rs
Expand Up @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs
Expand Up @@ -511,6 +511,7 @@ const fn drop<T: ~const Destruct>(_: T) {}

extern "rust-intrinsic" {
#[rustc_const_stable(feature = "const_eval_select", since = "1.0.0")]
#[rustc_safe_intrinsic]
fn const_eval_select<ARG: Tuple, F, G, RET>(
arg: ARG,
called_in_const: F,
Expand All @@ -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);
}