Skip to content

String::from_raw_parts example is potentially problematic #122743

Closed as not planned
@CAD97

Description

@CAD97

Location

The example for String::from_raw_parts's documentation.

Summary

Two potential issues, which are potentially unsound depending on the exact dynamic borrow model and implementation of String:

  • The example uses .as_mut_ptr() to get the raw pointer before calling .len() and .capacity(). It's better practice to extract the raw pointer last because calling other methods could potentially impact the provenance validity of the pointer.
  • The as_mut_ptr called is str::as_mut_ptr. With a strict subslicing interpretation of provenance, the result pointer doesn't have provenance over the entire capacity

The example itself doesn't trip Miri, but a slight adjustment to get a nonexact capacity does trip UB because of the latter point: [playground]

use std::mem;

unsafe {
    let mut s = String::from("hello");
    s.push('!'); // added

    // Prevent automatically dropping the String's data
    let mut s = mem::ManuallyDrop::new(s);

    let ptr = s.as_mut_ptr();
    let len = s.len();
    let capacity = s.capacity();

    let s = String::from_raw_parts(ptr, len, capacity);

    assert_eq!(String::from("hello!"), s);
}

Vec::from_raw_parts's example also calls .as_mut_ptr() first, but has Vec::as_mut_ptr specifically to shadow <[_]>::as_mut_ptr and avoid an intermediate slice reference. String should definitely get its own as_mut_ptr method like Vec has.

Changing the order the deconstruction is done is more debatable, since it's unlikely to ever actually cause UB, this order is more natural (to the point of TB treating all mut borrows as "two-phase" in order to allow it), and the reason for the order is subtle enough that changing the example is unlikely to assist people in writing it in the better order when it actually matters.

cc @rust-lang/opsem (yet another case of ptr before len)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions