Skip to content

Update/relax str/String utf8 safety docs #134598

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.

Sign up for GitHub

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

Open
zachs18 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Update/relax str/String utf8 safety docs #134598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d9786c
Update str safety docs to refer to the Invariant section of its docs.
zachs18 Dec 21, 2024
43252a0
Add Invariant section to String's docs, analogous to str.
zachs18 Dec 21, 2024
3b27e71
Document `str::as_bytes_mut` and `String::as_mut_vec` as exceptions t…
zachs18 Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions library/alloc/src/string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,18 @@ 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.
///
/// 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"
Expand Down Expand Up @@ -998,7 +1010,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
///
Expand Down Expand Up @@ -1818,9 +1830,12 @@ 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.
/// the original `String` after the `&mut Vec<u8>` 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
///
Expand Down
4 changes: 4 additions & 0 deletions library/core/src/primitive_docs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}

Expand Down
30 changes: 21 additions & 9 deletions library/core/src/str/converts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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.

@scottmcm scottmcm Dec 21, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep this requirement, to preserve the freedom for tooling (miri, ubchecks, kani, whatever) to validate it, even if we don't in release builds.

People can always pointer-cast if they want to do turn a &[u8] into &str without involving any standard library methods.

@zachs18 zachs18 Apr 23, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true that you can use pointer casting to get a &str with invalid UTF-8 from a &[u8] even if this function requires valid UTF-8 on pain of immediate UB, but String::from_utf8_unchecked currently does not require valid UTF-8 on pain of immediate UB (only on pain of later UB if the String is misused), so there's an asymmetry here.

So, we can either:

  • Relax str::from_utf8_unchecked(_mut) to not be immediate UB on invalid UTF-8 (what this PR does)
  • Restrict String::from_utf8_unchecked to be immediate UB on invalid UTF-8 (probably a breaking change)
  • Status quo with str::from_utf8_unchecked and String::from_utf8_unchecked having different preconditions w.r.t. UTF-8
  • Something else? (maybe declare calling _::from_utf8_unchecked on invalid UTF-8 "library erroneous behaviour"?)

/// 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
///
Expand All @@ -176,17 +179,26 @@ 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
/// that the string contains valid UTF-8; mutable version.
///
/// 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
///
Expand All @@ -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 {
Comment thread
zachs18 marked this conversation as resolved.
// 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) }
}

Expand Down
24 changes: 19 additions & 5 deletions library/core/src/str/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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
///
Expand Down Expand Up @@ -519,10 +529,14 @@ 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.
/// 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).
///
/// Use of a `str` whose contents are not valid UTF-8 is undefined behavior.
/// 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
///
Expand Down
56 changes: 28 additions & 28 deletions tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,15 @@
scope 6 (inlined #[track_caller] <String as Index<RangeFull>>::index) {
let _29: &str;
scope 7 (inlined String::as_str) {
let _30: &[u8];
scope 8 (inlined Vec::<u8>::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::<u8>::as_ptr) {
scope 10 (inlined alloc::raw_vec::RawVec::<u8>::ptr) {
scope 11 (inlined alloc::raw_vec::RawVecInner::ptr::<u8>) {
scope 12 (inlined alloc::raw_vec::RawVecInner::non_null::<u8>) {
let mut _34: std::ptr::NonNull<u8>;
let mut _33: std::ptr::NonNull<u8>;
scope 13 (inlined std::ptr::Unique::<u8>::cast::<u8>) {
scope 14 (inlined NonNull::<u8>::cast::<u8>) {
scope 15 (inlined NonNull::<u8>::as_ptr) {
Expand All @@ -73,6 +72,7 @@
}
}
scope 18 (inlined from_utf8_unchecked) {
let _34: *const str;
}
}
scope 19 (inlined #[track_caller] core::str::traits::<impl SliceIndex<str> for RangeFull>::index) {
Expand Down Expand Up @@ -123,16 +123,15 @@
scope 34 (inlined #[track_caller] <String as Index<RangeFull>>::index) {
let _49: &str;
scope 35 (inlined String::as_str) {
let _50: &[u8];
scope 36 (inlined Vec::<u8>::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::<u8>::as_ptr) {
scope 38 (inlined alloc::raw_vec::RawVec::<u8>::ptr) {
scope 39 (inlined alloc::raw_vec::RawVecInner::ptr::<u8>) {
scope 40 (inlined alloc::raw_vec::RawVecInner::non_null::<u8>) {
let mut _54: std::ptr::NonNull<u8>;
let mut _53: std::ptr::NonNull<u8>;
scope 41 (inlined std::ptr::Unique::<u8>::cast::<u8>) {
scope 42 (inlined NonNull::<u8>::cast::<u8>) {
scope 43 (inlined NonNull::<u8>::as_ptr) {
Expand All @@ -149,6 +148,7 @@
}
}
scope 46 (inlined from_utf8_unchecked) {
let _54: *const str;
}
}
scope 47 (inlined #[track_caller] core::str::traits::<impl SliceIndex<str> for RangeFull>::index) {
Expand Down Expand Up @@ -236,19 +236,19 @@
StorageLive(_29);
StorageLive(_30);
StorageLive(_31);
StorageLive(_32);
StorageLive(_34);
_34 = copy ((((((*_27).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_32 = copy _34 as *const u8 (Transmute);
StorageDead(_34);
StorageLive(_33);
_33 = copy (((*_27).0: std::vec::Vec<u8>).1: usize);
_31 = *const [u8] from (copy _32, move _33);
_33 = copy ((((((*_27).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_31 = copy _33 as *const u8 (Transmute);
StorageDead(_33);
StorageLive(_32);
_32 = copy (((*_27).0: std::vec::Vec<u8>).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);
Expand Down Expand Up @@ -298,19 +298,19 @@
StorageLive(_49);
StorageLive(_50);
StorageLive(_51);
StorageLive(_52);
StorageLive(_54);
_54 = copy ((((((*_47).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_52 = copy _54 as *const u8 (Transmute);
StorageDead(_54);
StorageLive(_53);
_53 = copy (((*_47).0: std::vec::Vec<u8>).1: usize);
_51 = *const [u8] from (copy _52, move _53);
_53 = copy ((((((*_47).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_51 = copy _53 as *const u8 (Transmute);
StorageDead(_53);
StorageLive(_52);
_52 = copy (((*_47).0: std::vec::Vec<u8>).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);
Expand Down
56 changes: 28 additions & 28 deletions tests/mir-opt/jump_threading.chained_conditions.JumpThreading.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,15 @@
scope 6 (inlined #[track_caller] <String as Index<RangeFull>>::index) {
let _29: &str;
scope 7 (inlined String::as_str) {
let _30: &[u8];
scope 8 (inlined Vec::<u8>::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::<u8>::as_ptr) {
scope 10 (inlined alloc::raw_vec::RawVec::<u8>::ptr) {
scope 11 (inlined alloc::raw_vec::RawVecInner::ptr::<u8>) {
scope 12 (inlined alloc::raw_vec::RawVecInner::non_null::<u8>) {
let mut _34: std::ptr::NonNull<u8>;
let mut _33: std::ptr::NonNull<u8>;
scope 13 (inlined std::ptr::Unique::<u8>::cast::<u8>) {
scope 14 (inlined NonNull::<u8>::cast::<u8>) {
scope 15 (inlined NonNull::<u8>::as_ptr) {
Expand All @@ -73,6 +72,7 @@
}
}
scope 18 (inlined from_utf8_unchecked) {
let _34: *const str;
}
}
scope 19 (inlined #[track_caller] core::str::traits::<impl SliceIndex<str> for RangeFull>::index) {
Expand Down Expand Up @@ -123,16 +123,15 @@
scope 34 (inlined #[track_caller] <String as Index<RangeFull>>::index) {
let _49: &str;
scope 35 (inlined String::as_str) {
let _50: &[u8];
scope 36 (inlined Vec::<u8>::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::<u8>::as_ptr) {
scope 38 (inlined alloc::raw_vec::RawVec::<u8>::ptr) {
scope 39 (inlined alloc::raw_vec::RawVecInner::ptr::<u8>) {
scope 40 (inlined alloc::raw_vec::RawVecInner::non_null::<u8>) {
let mut _54: std::ptr::NonNull<u8>;
let mut _53: std::ptr::NonNull<u8>;
scope 41 (inlined std::ptr::Unique::<u8>::cast::<u8>) {
scope 42 (inlined NonNull::<u8>::cast::<u8>) {
scope 43 (inlined NonNull::<u8>::as_ptr) {
Expand All @@ -149,6 +148,7 @@
}
}
scope 46 (inlined from_utf8_unchecked) {
let _54: *const str;
}
}
scope 47 (inlined #[track_caller] core::str::traits::<impl SliceIndex<str> for RangeFull>::index) {
Expand Down Expand Up @@ -236,19 +236,19 @@
StorageLive(_29);
StorageLive(_30);
StorageLive(_31);
StorageLive(_32);
StorageLive(_34);
_34 = copy ((((((*_27).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_32 = copy _34 as *const u8 (Transmute);
StorageDead(_34);
StorageLive(_33);
_33 = copy (((*_27).0: std::vec::Vec<u8>).1: usize);
_31 = *const [u8] from (copy _32, move _33);
_33 = copy ((((((*_27).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_31 = copy _33 as *const u8 (Transmute);
StorageDead(_33);
StorageLive(_32);
_32 = copy (((*_27).0: std::vec::Vec<u8>).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);
Expand Down Expand Up @@ -298,19 +298,19 @@
StorageLive(_49);
StorageLive(_50);
StorageLive(_51);
StorageLive(_52);
StorageLive(_54);
_54 = copy ((((((*_47).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_52 = copy _54 as *const u8 (Transmute);
StorageDead(_54);
StorageLive(_53);
_53 = copy (((*_47).0: std::vec::Vec<u8>).1: usize);
_51 = *const [u8] from (copy _52, move _53);
_53 = copy ((((((*_47).0: std::vec::Vec<u8>).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
_51 = copy _53 as *const u8 (Transmute);
StorageDead(_53);
StorageLive(_52);
_52 = copy (((*_47).0: std::vec::Vec<u8>).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);
Expand Down
Loading