Skip to content

Commit e5e5fb9

Browse files
committed
Fix drop handling in hint::select_unpredictable
This intrinsic doesn't drop the value that is not selected so this is manually done in the public function that wraps the intrinsic.
1 parent 94015d3 commit e5e5fb9

File tree

4 files changed

+48
-9
lines changed

4 files changed

+48
-9
lines changed

Diff for: library/core/src/hint.rs

+21-9
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//!
55
//! Hints may be compile time or runtime.
66
7+
use crate::mem::MaybeUninit;
78
use crate::{intrinsics, ub_checks};
89

910
/// Informs the compiler that the site which is calling this function is not
@@ -735,9 +736,9 @@ pub const fn cold_path() {
735736
crate::intrinsics::cold_path()
736737
}
737738

738-
/// Returns either `true_val` or `false_val` depending on the value of `b`,
739-
/// with a hint to the compiler that `b` is unlikely to be correctly
740-
/// predicted by a CPU’s branch predictor.
739+
/// Returns either `true_val` or `false_val` depending on the value of
740+
/// `condition`, with a hint to the compiler that `condition` is unlikely to be
741+
/// correctly predicted by a CPU’s branch predictor.
741742
///
742743
/// This method is functionally equivalent to
743744
/// ```ignore (this is just for illustrative purposes)
@@ -753,10 +754,10 @@ pub const fn cold_path() {
753754
/// search.
754755
///
755756
/// Note however that this lowering is not guaranteed (on any platform) and
756-
/// should not be relied upon when trying to write constant-time code. Also
757-
/// be aware that this lowering might *decrease* performance if `condition`
758-
/// is well-predictable. It is advisable to perform benchmarks to tell if
759-
/// this function is useful.
757+
/// should not be relied upon when trying to write cryptographic constant-time
758+
/// code. Also be aware that this lowering might *decrease* performance if
759+
/// `condition` is well-predictable. It is advisable to perform benchmarks to
760+
/// tell if this function is useful.
760761
///
761762
/// # Examples
762763
///
@@ -780,6 +781,17 @@ pub const fn cold_path() {
780781
/// ```
781782
#[inline(always)]
782783
#[unstable(feature = "select_unpredictable", issue = "133962")]
783-
pub fn select_unpredictable<T>(b: bool, true_val: T, false_val: T) -> T {
784-
crate::intrinsics::select_unpredictable(b, true_val, false_val)
784+
pub fn select_unpredictable<T>(condition: bool, true_val: T, false_val: T) -> T {
785+
// FIXME(https://github.com/rust-lang/unsafe-code-guidelines/issues/245):
786+
// Change this to use ManuallyDrop instead.
787+
let mut true_val = MaybeUninit::new(true_val);
788+
let mut false_val = MaybeUninit::new(false_val);
789+
// SAFETY: The value that is not selected is dropped, and the selected one
790+
// is returned. This is necessary because the intrinsic doesn't drop the
791+
// value that is not selected.
792+
unsafe {
793+
crate::intrinsics::select_unpredictable(!condition, &mut true_val, &mut false_val)
794+
.assume_init_drop();
795+
crate::intrinsics::select_unpredictable(condition, true_val, false_val).assume_init()
796+
}
785797
}

Diff for: library/core/src/intrinsics/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1327,6 +1327,8 @@ pub const fn unlikely(b: bool) -> bool {
13271327
/// any safety invariants.
13281328
///
13291329
/// The public form of this instrinsic is [`core::hint::select_unpredictable`].
1330+
/// However unlike the public form, the intrinsic will not drop the value that
1331+
/// is not selected.
13301332
#[unstable(feature = "core_intrinsics", issue = "none")]
13311333
#[rustc_intrinsic]
13321334
#[rustc_nounwind]

Diff for: library/coretests/tests/hint.rs

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#[test]
2+
fn select_unpredictable_drop() {
3+
use core::cell::Cell;
4+
struct X<'a>(&'a Cell<bool>);
5+
impl Drop for X<'_> {
6+
fn drop(&mut self) {
7+
self.0.set(true);
8+
}
9+
}
10+
11+
let a_dropped = Cell::new(false);
12+
let b_dropped = Cell::new(false);
13+
let a = X(&a_dropped);
14+
let b = X(&b_dropped);
15+
assert!(!a_dropped.get());
16+
assert!(!b_dropped.get());
17+
let selected = core::hint::select_unpredictable(core::hint::black_box(true), a, b);
18+
assert!(!a_dropped.get());
19+
assert!(b_dropped.get());
20+
drop(selected);
21+
assert!(a_dropped.get());
22+
assert!(b_dropped.get());
23+
}

Diff for: library/coretests/tests/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
#![feature(pointer_is_aligned_to)]
6969
#![feature(portable_simd)]
7070
#![feature(ptr_metadata)]
71+
#![feature(select_unpredictable)]
7172
#![feature(slice_from_ptr_range)]
7273
#![feature(slice_internals)]
7374
#![feature(slice_partition_dedup)]
@@ -147,6 +148,7 @@ mod ffi;
147148
mod fmt;
148149
mod future;
149150
mod hash;
151+
mod hint;
150152
mod intrinsics;
151153
mod io;
152154
mod iter;

0 commit comments

Comments
 (0)