Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 0e7119f

Browse files
Unrolled build for #145174
Rollup merge of #145174 - 197g:issue-145148-select-unpredictable-drop, r=joboet Ensure consistent drop for panicking drop in hint::select_unpredictable There are a few alternatives to the implementation. The principal problem is that the selected value must be owned (in the sense of having a drop flag of sorts) when the unselected value is dropped, such that panic unwind goes through the drop of both. This ownership must then be passed on in return when the drop went smoothly. The basic way of achieving this is by extracting the selected value first, at the cost of relying on the optimizer a little more for detecting the copy as constructing the return value despite having a place in the body. Unfortunately, that causes LLVM to discard the !unpredictable annotation (for some reason that is beyond my comprehension of LLVM). <details> <summary>Extract from the build log showing an unannotated select being used</summary> ``` 2025年08月09日T16:51:06.8790764Z 39: define noundef i64 `@test_int2(i1` noundef zeroext %p, i64 noundef %a, i64 noundef %b) unnamed_addr #0 personality ptr `@rust_eh_personality` { 2025年08月09日T16:51:06.8791368Z check:47'0 X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found 2025年08月09日T16:51:06.8791700Z 40: start: 2025年08月09日T16:51:06.8791858Z check:47'0 ~~~~~~~ 2025年08月09日T16:51:06.8792043Z 41: %ret.i = select i1 %p, i64 %a, i64 %b 2025年08月09日T16:51:06.8792293Z check:47'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2025年08月09日T16:51:06.8792686Z check:47'1 ? possible intended match 2025年08月09日T16:51:06.8792946Z 42: ret i64 %ret.i 2025年08月09日T16:51:06.8793127Z check:47'0 ~~~~~~~~~~~~~~~~ ``` </details> So instead, this PR includes a guard to drop the selected `MaybeUnit<T>` which is active only for the section where the unselected value is dropped. That leaves the code for selecting the result intact leading to the expected ir. That complicates the 'unselection' process a little bit since we require _both_ values as a result of that intrinsic call. Since the arguments alias, this portion as well as the drop guard uses raw pointers. Closes: #145148 Prior: #139977 
2 parents 1bc901e + ee9803a commit 0e7119f

File tree

2 files changed

+71
-2
lines changed

2 files changed

+71
-2
lines changed

‎library/core/src/hint.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -776,12 +776,45 @@ pub fn select_unpredictable<T>(condition: bool, true_val: T, false_val: T) -> T
776776
// Change this to use ManuallyDrop instead.
777777
let mut true_val = MaybeUninit::new(true_val);
778778
let mut false_val = MaybeUninit::new(false_val);
779+
780+
struct DropOnPanic<T> {
781+
// Invariant: valid pointer and points to an initialized value that is not further used,
782+
// i.e. it can be dropped by this guard.
783+
inner: *mut T,
784+
}
785+
786+
impl<T> Drop for DropOnPanic<T> {
787+
fn drop(&mut self) {
788+
// SAFETY: Must be guaranteed on construction of local type `DropOnPanic`.
789+
unsafe { self.inner.drop_in_place() }
790+
}
791+
}
792+
793+
let true_ptr = true_val.as_mut_ptr();
794+
let false_ptr = false_val.as_mut_ptr();
795+
779796
// SAFETY: The value that is not selected is dropped, and the selected one
780797
// is returned. This is necessary because the intrinsic doesn't drop the
781798
// value that is not selected.
782799
unsafe {
783-
crate::intrinsics::select_unpredictable(!condition, &mut true_val, &mut false_val)
784-
.assume_init_drop();
800+
// Extract the selected value first, ensure it is dropped as well if dropping the unselected
801+
// value panics. We construct a temporary by-pointer guard around the selected value while
802+
// dropping the unselected value. Arguments overlap here, so we can not use mutable
803+
// reference for these arguments.
804+
let guard = crate::intrinsics::select_unpredictable(condition, true_ptr, false_ptr);
805+
let drop = crate::intrinsics::select_unpredictable(condition, false_ptr, true_ptr);
806+
807+
// SAFETY: both pointers are well-aligned and point to initialized values inside a
808+
// `MaybeUninit` each. In both possible values for `condition` the pointer `guard` and
809+
// `drop` do not alias (even though the two argument pairs we have selected from did alias
810+
// each other).
811+
let guard = DropOnPanic { inner: guard };
812+
drop.drop_in_place();
813+
crate::mem::forget(guard);
814+
815+
// Note that it is important to use the values here. Reading from the pointer we got makes
816+
// LLVM forget the !unpredictable annotation sometimes (in tests, integer sized values in
817+
// particular seemed to confuse it, also observed in llvm/llvm-project #82340).
785818
crate::intrinsics::select_unpredictable(condition, true_val, false_val).assume_init()
786819
}
787820
}

‎library/coretests/tests/hint.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,39 @@ fn select_unpredictable_drop() {
2121
assert!(a_dropped.get());
2222
assert!(b_dropped.get());
2323
}
24+
25+
#[test]
26+
#[should_panic = "message canary"]
27+
fn select_unpredictable_drop_on_panic() {
28+
use core::cell::Cell;
29+
30+
struct X<'a> {
31+
cell: &'a Cell<u16>,
32+
expect: u16,
33+
write: u16,
34+
}
35+
36+
impl Drop for X<'_> {
37+
fn drop(&mut self) {
38+
let value = self.cell.get();
39+
self.cell.set(self.write);
40+
assert_eq!(value, self.expect, "message canary");
41+
}
42+
}
43+
44+
let cell = Cell::new(0);
45+
46+
// Trigger a double-panic if the selected cell was not dropped during panic.
47+
let _armed = X { cell: &cell, expect: 0xdead, write: 0 };
48+
let selected = X { cell: &cell, write: 0xdead, expect: 1 };
49+
let unselected = X { cell: &cell, write: 1, expect: 0xff };
50+
51+
// The correct drop order is:
52+
//
53+
// 1. `unselected` drops, writes 1, and panics as 0 != 0xff
54+
// 2. `selected` drops during unwind, writes 0xdead and does not panic as 1 == 1
55+
// 3. `armed` drops during unwind, writes 0 and does not panic as 0xdead == 0xdead
56+
//
57+
// If `selected` is not dropped, `armed` panics as 1 != 0xdead
58+
let _unreachable = core::hint::select_unpredictable(true, selected, unselected);
59+
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /