From f6247ffa5afb29fd86d54db8062ff031daa10555 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Jul 2022 09:38:07 -0400 Subject: [PATCH 1/4] clarify how write_bytes can lead to UB due to invalid values --- library/core/src/intrinsics.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 2895c923adc13..4c8619f313540 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2550,10 +2550,10 @@ pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { /// /// * `dst` must be properly aligned. /// -/// Additionally, the caller must ensure that writing `count * -/// size_of::()` bytes to the given region of memory results in a valid -/// value of `T`. Using a region of memory typed as a `T` that contains an -/// invalid value of `T` is undefined behavior. +/// Additionally, note that changing `*dst` in this way can lead to undefined behavior later if the +/// written bytes are not a valid representation of some `T`. For instance, if `dst: *mut bool`, a +/// `dst.write_bytes(0xFFu8, 1)` followed by `dst.read()` is undefined behavior since the `read` +/// tries to construct a `bool` value from `0xFF` which does not represent any `bool`. /// /// Note that even if the effectively copied size (`count * size_of::()`) is /// `0`, the pointer must be non-null and properly aligned. From 2e0ca9472ba3ec1532bb752f7ea1f477f8c34c90 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Jul 2022 10:48:43 -0400 Subject: [PATCH 2/4] add a concrete example --- library/core/src/intrinsics.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 4c8619f313540..d7ed82e71b6a4 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2550,14 +2550,23 @@ pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { /// /// * `dst` must be properly aligned. /// -/// Additionally, note that changing `*dst` in this way can lead to undefined behavior later if the -/// written bytes are not a valid representation of some `T`. For instance, if `dst: *mut bool`, a -/// `dst.write_bytes(0xFFu8, 1)` followed by `dst.read()` is undefined behavior since the `read` -/// tries to construct a `bool` value from `0xFF` which does not represent any `bool`. -/// /// Note that even if the effectively copied size (`count * size_of::()`) is /// `0`, the pointer must be non-null and properly aligned. /// +/// Additionally, note that changing `*dst` in this way can easily lead to undefined behavior (UB) +/// later if the written bytes are not a valid representation of some `T`. For instance, the +/// follwing is an **incorrect** use of this function: +/// +/// ```rust,no_run +/// unsafe { +/// let mut value: u8 = 0; +/// let ptr: *mut bool = &mut value as *mut u8 as *mut bool; +/// let _bool = ptr.read(); // This is fine, `ptr` points to a valid `bool`. +/// ptr.write_bytes(42u8, 1); // This function itself does not cause UB... +/// let _bool = ptr.read(); // ...but it makes this operation UB! ⚠️ +/// } +/// ``` +/// /// [valid]: crate::ptr#safety /// /// # Examples From eed5df52f63dbf19030fb99045334252fcd5b366 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 11 Jul 2022 09:49:55 -0400 Subject: [PATCH 3/4] typo Co-authored-by: Ben Kimock --- library/core/src/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index d7ed82e71b6a4..5f40a59f0b8ee 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2555,7 +2555,7 @@ pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { /// /// Additionally, note that changing `*dst` in this way can easily lead to undefined behavior (UB) /// later if the written bytes are not a valid representation of some `T`. For instance, the -/// follwing is an **incorrect** use of this function: +/// following is an **incorrect** use of this function: /// /// ```rust,no_run /// unsafe { From 1b3870e4271ca7b93de0b17ea6544e749fba3483 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 11 Jul 2022 11:36:08 -0400 Subject: [PATCH 4/4] remove a dubious example --- library/core/src/intrinsics.rs | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 5f40a59f0b8ee..a9330a869200f 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2583,38 +2583,6 @@ pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { /// } /// assert_eq!(vec, [0xfefefefe, 0xfefefefe, 0, 0]); /// ``` -/// -/// Creating an invalid value: -/// -/// ``` -/// use std::ptr; -/// -/// let mut v = Box::new(0i32); -/// -/// unsafe { -/// // Leaks the previously held value by overwriting the `Box` with -/// // a null pointer. -/// ptr::write_bytes(&mut v as *mut Box, 0, 1); -/// } -/// -/// // At this point, using or dropping `v` results in undefined behavior. -/// // drop(v); // ERROR -/// -/// // Even leaking `v` "uses" it, and hence is undefined behavior. -/// // mem::forget(v); // ERROR -/// -/// // In fact, `v` is invalid according to basic type layout invariants, so *any* -/// // operation touching it is undefined behavior. -/// // let v2 = v; // ERROR -/// -/// unsafe { -/// // Let us instead put in a valid value -/// ptr::write(&mut v as *mut Box, Box::new(42i32)); -/// } -/// -/// // Now the box is fine -/// assert_eq!(*v, 42); -/// ``` #[doc(alias = "memset")] #[stable(feature = "rust1", since = "1.0.0")] #[rustc_const_unstable(feature = "const_ptr_write", issue = "86302")]