Skip to content

example on String::from_raw_parts is correct in-context, but raise Miri error with seemingly sound modification #106593

@trinity-1686a

Description

@trinity-1686a
Contributor

Location

String::from_raw_parts example

Summary

The example is technically correct, however adding s.reserve(1) makes Miri complain about an invalid deallocation. From the documentation there is no obvious reason why that is.

problematic code
use std::mem;

unsafe {
    let mut s = String::from("hello");
    s.reserve(1); // <= not in std example

    // 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);
}

I opened an issue on rust-lang/miri#2751, where @bjorn3 clarified that there is indeed an issue:

This seems to be an issue in the documentation, albeit a tricky one. The as_mut_ptr method comes from str, not String. The deref from String to str shrinks the region to which the str and thus as_mut_ptr result is valid for to fit exactly the size and not the capacity of the string. I think the only way to do this currently are s.into_bytes().as_mut_ptr() (which uses Vec::as_mut_ptr, which does cover the whole capacity) or s.into_raw_parts(), which is unstable.

They also added:

One way to fix this would be to add an as_mut_ptr method directly to String.

I tagged this A-docs but the solution may be to actually create a String::as_mut_ptr and not update the documentation. An other solution is to describe the issue I experienced and change the ManuallyDrop line to be

    let mut s = mem::ManuallyDrop::new(s.into_bytes());

Activity

added
A-docsArea: Documentation for any part of the project, including the compiler, standard library, and tools
on Jan 8, 2023
saethlin

saethlin commented on Jan 8, 2023

@saethlin
Member

I think you are asking for #97483

trinity-1686a

trinity-1686a commented on Jan 8, 2023

@trinity-1686a
ContributorAuthor

I'm not sure exactly what I'm asking for, 97483 is indeed one of the possibilities

RalfJung

RalfJung commented on Apr 26, 2023

@RalfJung
Member

FWIW with Tree Borrows, this code will be accepted by Miri.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-docsArea: Documentation for any part of the project, including the compiler, standard library, and tools

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @RalfJung@saethlin@trinity-1686a

        Issue actions

          example on String::from_raw_parts is correct in-context, but raise Miri error with seemingly sound modification · Issue #106593 · rust-lang/rust