String::push_str invalidates interior references even when it does not reallocate

[matklad]

To my knowledge, the following code is intended to be legal:

fn main() {
    let mut buf = String::with_capacity(11);
    buf.push_str("hello");
    let hello: &str = unsafe { &*(buf.as_str() as *const _) }; // laundering the lifetime -- we take care that `buf` does not reallocate, so that's okay.
    buf.push_str(" world");
    println!("{}", hello);
}

However, Miri currently flags this as UB.

I believe this is #60847, but for String. Discovered while writing this post.

cc @RalfJung

[RalfJung]

I believe this is #60847, but for String

Yeah this looks pretty similar. String uses Vec under the hood though, so I wonder why the issue still exists here.

Looks like push_str is implemented via extend_from_slice -- and indeed, this errors, too:

fn main() {
    let mut v = Vec::with_capacity(10);
    v.push(0);
    let v0 = unsafe { &*(&v[0] as *const _) }; // laundering the lifetime -- we take care that `v` does not reallocate, so that's okay.
    v.extend_from_slice(&[1]);
    let _val = *v0;
}

[RalfJung]

The problem is in this line:

rust/src/liballoc/vec.rs

Line 2125 in 8ff7850

self.get_unchecked_mut(len..).copy_from_slice(slice);

The get_unchecked_mut here is a slice method, so this creates a slice covering the entire Vec, which overlaps with the mutable reference created before and thus invalidates it.

[RalfJung]

It shouldn't be too hard to locally fix this, but that duplicates some code and there are some other uses of get_unchecked_mut in vec.rs. Ideally, this would all use raw slices to avoid making any uniqueness promises. But without #60639, raw slices are not very useful.

[RalfJung]

Here's a possible fix:

diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs
index 4769091183a..fd2b336eceb 100644
--- a/src/liballoc/vec.rs
+++ b/src/liballoc/vec.rs
@@ -2121,8 +2121,9 @@ where
         self.reserve(slice.len());
         unsafe {
             let len = self.len();
+            let dst_slice = slice::from_raw_parts_mut(self.as_mut_ptr().add(len), slice.len());
+            dst_slice.copy_from_slice(slice);
             self.set_len(len + slice.len());
-            self.get_unchecked_mut(len..).copy_from_slice(slice);
         }
     }
 }

[RalfJung]

Fix PR is up: #70558