-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Fix drop handling in hint::select_unpredictable
#139977
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
Conversation
This intrinsic doesn't drop the value that is not selected so this is manually done in the public function that wraps the intrinsic.
Good catch! |
I tried that in #139933 but unfortunately that prevents the use of |
Ah, we'd want r=me for the implementation part of this PR once CI is happy. t-libs-api will have to make the final call regarding whether we want to support this function on types with drop glue. |
@bors r=RalfJung |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#139351 (Autodiff batching2) - rust-lang#139483 (f*::NAN: guarantee that this is a quiet NaN) - rust-lang#139498 (Ignore zero-sized types in wasm future-compat warning) - rust-lang#139967 (Introduce and use specialized `//@ ignore-auxiliary` for test support files instead of using `//@ ignore-test`) - rust-lang#139969 (update libc) - rust-lang#139971 (Make C string merging test work on MIPS) - rust-lang#139974 (Change `InterpCx::instantiate*` function visibility to pub) - rust-lang#139977 (Fix drop handling in `hint::select_unpredictable`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#139977 - Amanieu:select_unpredictable_drop, r=RalfJung 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.
… r=RalfJung 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.
…able-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-09T16: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-09T16:51:06.8791368Z check:47'0 X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found 2025-08-09T16:51:06.8791700Z 40: start: 2025-08-09T16:51:06.8791858Z check:47'0 ~~~~~~~ 2025-08-09T16:51:06.8792043Z 41: %ret.i = select i1 %p, i64 %a, i64 %b 2025-08-09T16:51:06.8792293Z check:47'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2025-08-09T16:51:06.8792686Z check:47'1 ? possible intended match 2025-08-09T16:51:06.8792946Z 42: ret i64 %ret.i 2025-08-09T16: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: rust-lang#145148 Prior: rust-lang#139977
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-09T16: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-09T16:51:06.8791368Z check:47'0 X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found 2025-08-09T16:51:06.8791700Z 40: start: 2025-08-09T16:51:06.8791858Z check:47'0 ~~~~~~~ 2025-08-09T16:51:06.8792043Z 41: %ret.i = select i1 %p, i64 %a, i64 %b 2025-08-09T16:51:06.8792293Z check:47'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2025-08-09T16:51:06.8792686Z check:47'1 ? possible intended match 2025-08-09T16:51:06.8792946Z 42: ret i64 %ret.i 2025-08-09T16: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
…, 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-09T16: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-09T16:51:06.8791368Z check:47'0 X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found 2025-08-09T16:51:06.8791700Z 40: start: 2025-08-09T16:51:06.8791858Z check:47'0 ~~~~~~~ 2025-08-09T16:51:06.8792043Z 41: %ret.i = select i1 %p, i64 %a, i64 %b 2025-08-09T16:51:06.8792293Z check:47'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2025-08-09T16:51:06.8792686Z check:47'1 ? possible intended match 2025-08-09T16:51:06.8792946Z 42: ret i64 %ret.i 2025-08-09T16: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: rust-lang/rust#145148 Prior: rust-lang/rust#139977
…, 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-09T16: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-09T16:51:06.8791368Z check:47'0 X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found 2025-08-09T16:51:06.8791700Z 40: start: 2025-08-09T16:51:06.8791858Z check:47'0 ~~~~~~~ 2025-08-09T16:51:06.8792043Z 41: %ret.i = select i1 %p, i64 %a, i64 %b 2025-08-09T16:51:06.8792293Z check:47'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2025-08-09T16:51:06.8792686Z check:47'1 ? possible intended match 2025-08-09T16:51:06.8792946Z 42: ret i64 %ret.i 2025-08-09T16: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: rust-lang/rust#145148 Prior: rust-lang/rust#139977
This intrinsic doesn't drop the value that is not selected so this is manually done in the public function that wraps the intrinsic.