From 8d9786c470b51d6c9522e49868968d089a66d8e6 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Fri, 20 Dec 2024 20:48:45 -0600 Subject: [PATCH 1/3] Update str safety docs to refer to the Invariant section of its docs. * (str,core::str)::from_utf8_unchecked: doc: do not declare that invalid UTF-8 is immediate UB, analogous to `String::from_utf8_unchecked`. * (str,core::str)::from_utf8_unchecked: update internal safety comments, and use pointer cast instead of transmute * (str,core::str)::from_utf8_unchecked_mut: add `# Safety` section mirroring `from_utf8_unchecked`, instead of just referring to it. * str::as_bytes_mut: doc: do not declare that invalid UTF-8 after borrow ends is immediate UB, analogous to `String::as_mut_vec`. --- library/core/src/str/converts.rs | 30 +++++++--- library/core/src/str/mod.rs | 23 ++++++-- ..._conditions.JumpThreading.panic-abort.diff | 56 +++++++++---------- ...conditions.JumpThreading.panic-unwind.diff | 56 +++++++++---------- 4 files changed, 94 insertions(+), 71 deletions(-) diff --git a/library/core/src/str/converts.rs b/library/core/src/str/converts.rs index 6da9dce2d8707..26fac5c3f19b0 100644 --- a/library/core/src/str/converts.rs +++ b/library/core/src/str/converts.rs @@ -2,7 +2,7 @@ use super::Utf8Error; use super::validations::run_utf8_validation; -use crate::{mem, ptr}; +use crate::ptr; /// Converts a slice of bytes to a string slice. /// @@ -152,7 +152,10 @@ pub const fn from_utf8_mut(v: &mut [u8]) -> Result<&mut str, Utf8Error> { /// /// # Safety /// -/// The bytes passed in must be valid UTF-8. +/// This function is unsafe because it does not check that the bytes passed +/// to it are valid UTF-8. If this constraint is violated, it may cause +/// memory unsafety issues with future users of the `str`, as the rest of +/// the standard library [assumes that `str`s are valid UTF-8](prim@str#invariant). /// /// # Examples /// @@ -176,9 +179,11 @@ pub const fn from_utf8_mut(v: &mut [u8]) -> Result<&mut str, Utf8Error> { #[rustc_const_stable(feature = "const_str_from_utf8_unchecked", since = "1.55.0")] #[rustc_diagnostic_item = "str_from_utf8_unchecked"] pub const unsafe fn from_utf8_unchecked(v: &[u8]) -> &str { - // SAFETY: the caller must guarantee that the bytes `v` are valid UTF-8. - // Also relies on `&str` and `&[u8]` having the same layout. - unsafe { mem::transmute(v) } + // SAFETY: the pointer dereference is safe because that pointer + // comes from a reference which is guaranteed to be valid for reads. + // If the input bytes are not valid UTF-8, then the returned `&str` will + // have invalid UTF-8, which is unsafe but not immediate UB. + unsafe { &*(v as *const [u8] as *const str) } } /// Converts a slice of bytes to a string slice without checking @@ -186,7 +191,14 @@ pub const unsafe fn from_utf8_unchecked(v: &[u8]) -> &str { /// /// This is an alias to [`str::from_utf8_unchecked_mut`]. /// -/// See the immutable version, [`from_utf8_unchecked()`] for documentation and safety requirements. +/// See the immutable version, [`from_utf8_unchecked()`] for more information. +/// +/// # Safety +/// +/// This function is unsafe because it does not check that the bytes passed +/// to it are valid UTF-8. If this constraint is violated, it may cause +/// memory unsafety issues with future users of the `str`, as the rest of +/// the standard library [assumes that `str`s are valid UTF-8](prim@str#invariant). /// /// # Examples /// @@ -206,10 +218,10 @@ pub const unsafe fn from_utf8_unchecked(v: &[u8]) -> &str { #[rustc_const_stable(feature = "const_str_from_utf8_unchecked_mut", since = "1.83.0")] #[rustc_diagnostic_item = "str_from_utf8_unchecked_mut"] pub const unsafe fn from_utf8_unchecked_mut(v: &mut [u8]) -> &mut str { - // SAFETY: the caller must guarantee that the bytes `v` - // are valid UTF-8, thus the cast to `*mut str` is safe. - // Also, the pointer dereference is safe because that pointer + // SAFETY: the pointer dereference is safe because that pointer // comes from a reference which is guaranteed to be valid for writes. + // If the input bytes are not valid UTF-8, then the returned `&mut str` will + // have invalid UTF-8, which is unsafe but not immediate UB. unsafe { &mut *(v as *mut [u8] as *mut str) } } diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index 2128ce328a9c6..05fba848e57d8 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -293,7 +293,10 @@ impl str { /// /// # Safety /// - /// The bytes passed in must be valid UTF-8. + /// This function is unsafe because it does not check that the bytes passed + /// to it are valid UTF-8. If this constraint is violated, it may cause + /// memory unsafety issues with future users of the `str`, as the rest of + /// the standard library [assumes that `str`s are valid UTF-8](prim@str#invariant). /// /// # Examples /// @@ -322,7 +325,14 @@ impl str { /// Converts a slice of bytes to a string slice without checking /// that the string contains valid UTF-8; mutable version. /// - /// See the immutable version, [`from_utf8_unchecked()`] for documentation and safety requirements. + /// See the immutable version, [`from_utf8_unchecked()`] for more information. + /// + /// # Safety + /// + /// This function is unsafe because it does not check that the bytes passed + /// to it are valid UTF-8. If this constraint is violated, it may cause + /// memory unsafety issues with future users of the `str`, as the rest of + /// the standard library [assumes that `str`s are valid UTF-8](prim@str#invariant). /// /// # Examples /// @@ -519,10 +529,11 @@ impl str { /// /// # Safety /// - /// The caller must ensure that the content of the slice is valid UTF-8 - /// before the borrow ends and the underlying `str` is used. - /// - /// Use of a `str` whose contents are not valid UTF-8 is undefined behavior. + /// This function is unsafe because the returned `&mut [u8]` allows writing + /// bytes which are not valid UTF-8. If this constraint is violated, using + /// the original `str` after the `&mut [u8]` borrow expires may violate memory + /// safety, as the rest of the standard library [assumes that `str`s are + /// valid UTF-8](prim@str#invariant). /// /// # Examples /// diff --git a/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-abort.diff b/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-abort.diff index 5355d0d83ff89..792d161a1ced1 100644 --- a/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-abort.diff +++ b/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-abort.diff @@ -47,16 +47,15 @@ scope 6 (inlined #[track_caller] >::index) { let _29: &str; scope 7 (inlined String::as_str) { - let _30: &[u8]; scope 8 (inlined Vec::::as_slice) { - let _31: *const [u8]; - let mut _32: *const u8; - let mut _33: usize; + let _30: *const [u8]; + let mut _31: *const u8; + let mut _32: usize; scope 9 (inlined Vec::::as_ptr) { scope 10 (inlined alloc::raw_vec::RawVec::::ptr) { scope 11 (inlined alloc::raw_vec::RawVecInner::ptr::) { scope 12 (inlined alloc::raw_vec::RawVecInner::non_null::) { - let mut _34: std::ptr::NonNull; + let mut _33: std::ptr::NonNull; scope 13 (inlined std::ptr::Unique::::cast::) { scope 14 (inlined NonNull::::cast::) { scope 15 (inlined NonNull::::as_ptr) { @@ -73,6 +72,7 @@ } } scope 18 (inlined from_utf8_unchecked) { + let _34: *const str; } } scope 19 (inlined #[track_caller] core::str::traits:: for RangeFull>::index) { @@ -123,16 +123,15 @@ scope 34 (inlined #[track_caller] >::index) { let _49: &str; scope 35 (inlined String::as_str) { - let _50: &[u8]; scope 36 (inlined Vec::::as_slice) { - let _51: *const [u8]; - let mut _52: *const u8; - let mut _53: usize; + let _50: *const [u8]; + let mut _51: *const u8; + let mut _52: usize; scope 37 (inlined Vec::::as_ptr) { scope 38 (inlined alloc::raw_vec::RawVec::::ptr) { scope 39 (inlined alloc::raw_vec::RawVecInner::ptr::) { scope 40 (inlined alloc::raw_vec::RawVecInner::non_null::) { - let mut _54: std::ptr::NonNull; + let mut _53: std::ptr::NonNull; scope 41 (inlined std::ptr::Unique::::cast::) { scope 42 (inlined NonNull::::cast::) { scope 43 (inlined NonNull::::as_ptr) { @@ -149,6 +148,7 @@ } } scope 46 (inlined from_utf8_unchecked) { + let _54: *const str; } } scope 47 (inlined #[track_caller] core::str::traits:: for RangeFull>::index) { @@ -236,19 +236,19 @@ StorageLive(_29); StorageLive(_30); StorageLive(_31); - StorageLive(_32); - StorageLive(_34); - _34 = copy ((((((*_27).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); - _32 = copy _34 as *const u8 (Transmute); - StorageDead(_34); StorageLive(_33); - _33 = copy (((*_27).0: std::vec::Vec).1: usize); - _31 = *const [u8] from (copy _32, move _33); + _33 = copy ((((((*_27).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); + _31 = copy _33 as *const u8 (Transmute); StorageDead(_33); + StorageLive(_32); + _32 = copy (((*_27).0: std::vec::Vec).1: usize); + _30 = *const [u8] from (copy _31, move _32); StorageDead(_32); - _30 = &(*_31); StorageDead(_31); - _29 = copy _30 as &str (Transmute); + StorageLive(_34); + _34 = copy _30 as *const str (PtrToPtr); + _29 = &(*_34); + StorageDead(_34); StorageDead(_30); StorageLive(_36); StorageLive(_38); @@ -298,19 +298,19 @@ StorageLive(_49); StorageLive(_50); StorageLive(_51); - StorageLive(_52); - StorageLive(_54); - _54 = copy ((((((*_47).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); - _52 = copy _54 as *const u8 (Transmute); - StorageDead(_54); StorageLive(_53); - _53 = copy (((*_47).0: std::vec::Vec).1: usize); - _51 = *const [u8] from (copy _52, move _53); + _53 = copy ((((((*_47).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); + _51 = copy _53 as *const u8 (Transmute); StorageDead(_53); + StorageLive(_52); + _52 = copy (((*_47).0: std::vec::Vec).1: usize); + _50 = *const [u8] from (copy _51, move _52); StorageDead(_52); - _50 = &(*_51); StorageDead(_51); - _49 = copy _50 as &str (Transmute); + StorageLive(_54); + _54 = copy _50 as *const str (PtrToPtr); + _49 = &(*_54); + StorageDead(_54); StorageDead(_50); StorageLive(_56); StorageLive(_58); diff --git a/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-unwind.diff b/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-unwind.diff index 25f6cc39b42ff..e0d08ef97a1eb 100644 --- a/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-unwind.diff +++ b/tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-unwind.diff @@ -47,16 +47,15 @@ scope 6 (inlined #[track_caller] >::index) { let _29: &str; scope 7 (inlined String::as_str) { - let _30: &[u8]; scope 8 (inlined Vec::::as_slice) { - let _31: *const [u8]; - let mut _32: *const u8; - let mut _33: usize; + let _30: *const [u8]; + let mut _31: *const u8; + let mut _32: usize; scope 9 (inlined Vec::::as_ptr) { scope 10 (inlined alloc::raw_vec::RawVec::::ptr) { scope 11 (inlined alloc::raw_vec::RawVecInner::ptr::) { scope 12 (inlined alloc::raw_vec::RawVecInner::non_null::) { - let mut _34: std::ptr::NonNull; + let mut _33: std::ptr::NonNull; scope 13 (inlined std::ptr::Unique::::cast::) { scope 14 (inlined NonNull::::cast::) { scope 15 (inlined NonNull::::as_ptr) { @@ -73,6 +72,7 @@ } } scope 18 (inlined from_utf8_unchecked) { + let _34: *const str; } } scope 19 (inlined #[track_caller] core::str::traits:: for RangeFull>::index) { @@ -123,16 +123,15 @@ scope 34 (inlined #[track_caller] >::index) { let _49: &str; scope 35 (inlined String::as_str) { - let _50: &[u8]; scope 36 (inlined Vec::::as_slice) { - let _51: *const [u8]; - let mut _52: *const u8; - let mut _53: usize; + let _50: *const [u8]; + let mut _51: *const u8; + let mut _52: usize; scope 37 (inlined Vec::::as_ptr) { scope 38 (inlined alloc::raw_vec::RawVec::::ptr) { scope 39 (inlined alloc::raw_vec::RawVecInner::ptr::) { scope 40 (inlined alloc::raw_vec::RawVecInner::non_null::) { - let mut _54: std::ptr::NonNull; + let mut _53: std::ptr::NonNull; scope 41 (inlined std::ptr::Unique::::cast::) { scope 42 (inlined NonNull::::cast::) { scope 43 (inlined NonNull::::as_ptr) { @@ -149,6 +148,7 @@ } } scope 46 (inlined from_utf8_unchecked) { + let _54: *const str; } } scope 47 (inlined #[track_caller] core::str::traits:: for RangeFull>::index) { @@ -236,19 +236,19 @@ StorageLive(_29); StorageLive(_30); StorageLive(_31); - StorageLive(_32); - StorageLive(_34); - _34 = copy ((((((*_27).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); - _32 = copy _34 as *const u8 (Transmute); - StorageDead(_34); StorageLive(_33); - _33 = copy (((*_27).0: std::vec::Vec).1: usize); - _31 = *const [u8] from (copy _32, move _33); + _33 = copy ((((((*_27).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); + _31 = copy _33 as *const u8 (Transmute); StorageDead(_33); + StorageLive(_32); + _32 = copy (((*_27).0: std::vec::Vec).1: usize); + _30 = *const [u8] from (copy _31, move _32); StorageDead(_32); - _30 = &(*_31); StorageDead(_31); - _29 = copy _30 as &str (Transmute); + StorageLive(_34); + _34 = copy _30 as *const str (PtrToPtr); + _29 = &(*_34); + StorageDead(_34); StorageDead(_30); StorageLive(_36); StorageLive(_38); @@ -298,19 +298,19 @@ StorageLive(_49); StorageLive(_50); StorageLive(_51); - StorageLive(_52); - StorageLive(_54); - _54 = copy ((((((*_47).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); - _52 = copy _54 as *const u8 (Transmute); - StorageDead(_54); StorageLive(_53); - _53 = copy (((*_47).0: std::vec::Vec).1: usize); - _51 = *const [u8] from (copy _52, move _53); + _53 = copy ((((((*_47).0: std::vec::Vec).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); + _51 = copy _53 as *const u8 (Transmute); StorageDead(_53); + StorageLive(_52); + _52 = copy (((*_47).0: std::vec::Vec).1: usize); + _50 = *const [u8] from (copy _51, move _52); StorageDead(_52); - _50 = &(*_51); StorageDead(_51); - _49 = copy _50 as &str (Transmute); + StorageLive(_54); + _54 = copy _50 as *const str (PtrToPtr); + _49 = &(*_54); + StorageDead(_54); StorageDead(_50); StorageLive(_56); StorageLive(_58); From 43252a0a8973d4a6d57dbc184d94cd9098bc7553 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Fri, 20 Dec 2024 20:54:32 -0600 Subject: [PATCH 2/3] Add Invariant section to String's docs, analogous to str. Documents that invalid UTF-8 in a `String` is not immediate UB, but other code may assume that `String`s are valid UTF-8, so it can lead to UB later. --- library/alloc/src/string.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index eb00db4568b63..41c59768f7ded 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -341,6 +341,14 @@ use crate::vec::{self, Vec}; /// /// Here, there's no need to allocate more memory inside the loop. /// +/// # Invariant +/// +/// Rust libraries may assume that `String`s are always valid UTF-8, [just like `str`s](prim@str#invariant). +/// +/// Constructing a non-UTF-8 `String` is not immediate undefined behavior, but any function +/// called on a `String` may assume that it is valid UTF-8, which means that a non-UTF-8 `String` +/// can lead to undefined behavior down the road. +/// /// [str]: prim@str "str" /// [`str`]: prim@str "str" /// [`&str`]: prim@str "&str" @@ -998,7 +1006,7 @@ impl String { /// This function is unsafe because it does not check that the bytes passed /// to it are valid UTF-8. If this constraint is violated, it may cause /// memory unsafety issues with future users of the `String`, as the rest of - /// the standard library assumes that `String`s are valid UTF-8. + /// the standard library [assumes that `String`s are valid UTF-8](String#invariant). /// /// # Examples /// @@ -1819,8 +1827,8 @@ impl String { /// This function is unsafe because the returned `&mut Vec` allows writing /// bytes which are not valid UTF-8. If this constraint is violated, using /// the original `String` after dropping the `&mut Vec` may violate memory - /// safety, as the rest of the standard library assumes that `String`s are - /// valid UTF-8. + /// safety, as the rest of the standard library [assumes that `String`s are + /// valid UTF-8](String#invariant). /// /// # Examples /// From 3b27e7107b45a685666f4554f314d170bc334e86 Mon Sep 17 00:00:00 2001 From: Zachary S Date: Thu, 9 Jul 2026 15:35:59 -0500 Subject: [PATCH 3/3] Document `str::as_bytes_mut` and `String::as_mut_vec` as exceptions to str/String's UTF-8 requirements. --- library/alloc/src/string.rs | 9 ++++++++- library/core/src/primitive_docs.rs | 4 ++++ library/core/src/str/mod.rs | 3 +++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index 41c59768f7ded..cf30ff2d88a92 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -349,6 +349,10 @@ use crate::vec::{self, Vec}; /// called on a `String` may assume that it is valid UTF-8, which means that a non-UTF-8 `String` /// can lead to undefined behavior down the road. /// +/// Some functions explicitly allow invalid UTF-8, and will not immediately cause undefined behavior +/// if called on a `String` containing invalid UTF-8. Such functions explicitly specify this in their +/// documentation. +/// /// [str]: prim@str "str" /// [`str`]: prim@str "str" /// [`&str`]: prim@str "&str" @@ -1826,10 +1830,13 @@ impl String { /// /// This function is unsafe because the returned `&mut Vec` allows writing /// bytes which are not valid UTF-8. If this constraint is violated, using - /// the original `String` after dropping the `&mut Vec` may violate memory + /// the original `String` after the `&mut Vec` borrow expires may violate memory /// safety, as the rest of the standard library [assumes that `String`s are /// valid UTF-8](String#invariant). /// + /// As an exception to the [general rule](String#invariant), this function will not cause + /// immediate undefined behavior if called on a `String` containing invalid UTF-8. + /// /// # Examples /// /// ``` diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index c6fbe52ee887b..cabf86170248d 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -1014,6 +1014,10 @@ mod prim_slice {} /// Constructing a non-UTF-8 string slice is not immediate undefined behavior, but any function /// called on a string slice may assume that it is valid UTF-8, which means that a non-UTF-8 string /// slice can lead to undefined behavior down the road. +/// +/// Some functions explicitly allow invalid UTF-8, and will not immediately cause undefined +/// behavior if called on a string slice containing invalid UTF-8. Such functions explicitly +/// specify this in their documentation. #[stable(feature = "rust1", since = "1.0.0")] mod prim_str {} diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index 05fba848e57d8..f04dd1d6e3baf 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -535,6 +535,9 @@ impl str { /// safety, as the rest of the standard library [assumes that `str`s are /// valid UTF-8](prim@str#invariant). /// + /// As an exception to the [general rule](prim@str#invariant), this function will not cause + /// immediate undefined behavior if called on a `str` containing invalid UTF-8. + /// /// # Examples /// /// Basic usage: