String::push_str invalidates interior references even when it does not reallocate
matklad opened this issue · 5 comments
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
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;
}The problem is in this line:
Line 2125 in 8ff7850
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.
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.
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);
}
}
}