Skip to content

Commit

Permalink
Auto merge of #99182 - RalfJung:mitigate-uninit, r=scottmcm
Browse files Browse the repository at this point in the history
mem::uninitialized: mitigate many incorrect uses of this function

Alternative to rust-lang/rust#98966: fill memory with `0x01` rather than leaving it uninit. This is definitely bitewise valid for all `bool` and nonnull types, and also those `Option<&T>` that we started putting `noundef` on. However it is still invalid for `char` and some enums, and on references the `dereferenceable` attribute is still violated, so the generated LLVM IR still has UB -- but in fewer cases, and `dereferenceable` is hopefully less likely to cause problems than clearly incorrect range annotations.

This can make using `mem::uninitialized` a lot slower, but that function has been deprecated for years and we keep telling everyone to move to `MaybeUninit` because it is basically impossible to use `mem::uninitialized` correctly. For the cases where that hasn't helped (and all the old code out there that nobody will ever update), we can at least mitigate the effect of using this API. Note that this is *not* in any way a stable guarantee -- it is still UB to call `mem::uninitialized::<bool>()`, and Miri will call it out as such.

This is somewhat similar to rust-lang/rust#87032, which proposed to make `uninitialized` return a buffer filled with 0x00. However
- That PR also proposed to reduce the situations in which we panic, which I don't think we should do at this time.
- The 0x01 bit pattern means that nonnull requirements are satisfied, which (due to references) is the most common validity invariant.

`@5225225` I hope I am using `cfg(sanitize)` the right way; I was not sure for which ones to test here.
Cc rust-lang/rust#66151
Fixes rust-lang/rust#87675
  • Loading branch information
bors committed Jul 28, 2022
2 parents f7b41a3 + 36c458d commit d29b9e0
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
1 change: 1 addition & 0 deletions core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@
#![feature(allow_internal_unstable)]
#![feature(associated_type_bounds)]
#![feature(auto_traits)]
#![feature(cfg_sanitize)]
#![feature(cfg_target_has_atomic)]
#![feature(cfg_target_has_atomic_equal_alignment)]
#![feature(const_fn_floating_point_arithmetic)]
Expand Down
12 changes: 11 additions & 1 deletion core/src/mem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,8 @@ pub unsafe fn zeroed<T>() -> T {
/// produce a value of type `T`, while doing nothing at all.
///
/// **This function is deprecated.** Use [`MaybeUninit<T>`] instead.
/// It also might be slower than using `MaybeUninit<T>` due to mitigations that were put in place to
/// limit the potential harm caused by incorrect use of this function in legacy code.
///
/// The reason for deprecation is that the function basically cannot be used
/// correctly: it has the same effect as [`MaybeUninit::uninit().assume_init()`][uninit].
Expand Down Expand Up @@ -683,7 +685,15 @@ pub unsafe fn uninitialized<T>() -> T {
// SAFETY: the caller must guarantee that an uninitialized value is valid for `T`.
unsafe {
intrinsics::assert_uninit_valid::<T>();
MaybeUninit::uninit().assume_init()
let mut val = MaybeUninit::<T>::uninit();

// Fill memory with 0x01, as an imperfect mitigation for old code that uses this function on
// bool, nonnull, and noundef types. But don't do this if we actively want to detect UB.
if !cfg!(any(miri, sanitize = "memory")) {
val.as_mut_ptr().write_bytes(0x01, 1);
}

val.assume_init()
}
}

Expand Down

0 comments on commit d29b9e0

Please sign in to comment.