Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 22 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,17 +492,30 @@ impl<T, const N: usize> Drain<'_, T, N> {
let vec = unsafe { self.vec.as_mut() };
let range_start = vec.len();
let range_end = self.tail_start;
// Whether the buffer lives on the heap does not change while filling, so
// read it once, before `range_slice` is created.
let on_heap = vec.len.on_heap();
let range_slice = unsafe {
core::slice::from_raw_parts_mut(
vec.as_mut_ptr().add(range_start),
range_end - range_start,
)
};

let mut len = range_start;
for place in range_slice {
if let Some(new_item) = replace_with.next() {
unsafe { core::ptr::write(place, new_item) };
vec.set_len(vec.len() + 1);
len += 1;
// Update the length by writing the `len` field directly rather
// than calling `vec.set_len(..)`. Calling a `&mut self` method
// reborrows the whole `SmallVec`; when the buffer is inline it
// aliases `range_slice`, so that reborrow would invalidate
// `range_slice` (and the `place` reference derived from it) under
// Stacked Borrows, causing undefined behavior on the next
// iteration. This mirrors the standard library's `Drain::fill`,
// which bumps `vec.len` directly for the same reason.
vec.len = TaggedLen::new(len, on_heap);
} else {
return false;
}
Expand All @@ -524,8 +537,14 @@ impl<T, const N: usize> Drain<'_, T, N> {

let new_tail_start = self.tail_start + additional;
unsafe {
let src = vec.as_ptr().add(self.tail_start);
let dst = vec.as_mut_ptr().add(new_tail_start);
// Derive both the source and destination from a single mutable base
// pointer. Taking a `*const` via `vec.as_ptr()` and then reborrowing
// the whole `SmallVec` again via `vec.as_mut_ptr()` would invalidate
// the former under Stacked Borrows when the buffer is inline (the
// buffer aliases the `SmallVec` that `as_mut_ptr` reborrows).
let base = vec.as_mut_ptr();
let src = base.add(self.tail_start);
let dst = base.add(new_tail_start);
core::ptr::copy(src, dst, self.tail_len);
}
self.tail_start = new_tail_start;
Expand Down
45 changes: 45 additions & 0 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,51 @@ fn splice() {
assert_eq!(u, [0, 1, 2]);
}

// Splicing an *inline* `SmallVec` used to trigger undefined behavior (a Stacked
// Borrows aliasing violation): `Drain::fill` and `Drain::move_tail` held a
// reference/pointer into the inline buffer while reborrowing the whole
// `SmallVec` (via `set_len`/`as_mut_ptr`), which aliases that buffer. All of
// these cases keep the vector inline (`N` is large), so they exercise the fix.
// Run under `cargo miri test` to detect a regression.
#[test]
fn splice_inline() {
// more replacements than removed -> fill + move_tail + fill
let mut v: SmallVec<i32, 16> = smallvec![1, 2, 3, 4, 5];
let u: SmallVec<i32, 16> = v.splice(1..3, [10, 11, 12]).collect();
assert!(!v.spilled());
assert_eq!(u, [2, 3]);
assert_eq!(v, [1, 10, 11, 12, 4, 5]);

// equal count -> fill only
let mut v: SmallVec<i32, 16> = smallvec![1, 2, 3, 4, 5];
let u: SmallVec<i32, 16> = v.splice(1..3, [20, 21]).collect();
assert!(!v.spilled());
assert_eq!(u, [2, 3]);
assert_eq!(v, [1, 20, 21, 4, 5]);

// fewer replacements -> partial fill, tail moved back on drop
let mut v: SmallVec<i32, 16> = smallvec![1, 2, 3, 4, 5, 6];
let u: SmallVec<i32, 16> = v.splice(1..4, [30]).collect();
assert!(!v.spilled());
assert_eq!(u, [2, 3, 4]);
assert_eq!(v, [1, 30, 5, 6]);

// empty tail, growing -> move_tail with tail_len == 0
let mut v: SmallVec<i32, 16> = smallvec![1, 2, 3];
let u: SmallVec<i32, 16> = v.splice(2..3, [40, 41, 42, 43]).collect();
assert!(!v.spilled());
assert_eq!(u, [3]);
assert_eq!(v, [1, 2, 40, 41, 42, 43]);

// loose size_hint -> "collect the remainder" branch of Splice::drop
let mut v: SmallVec<i32, 16> = smallvec![1, 2, 3, 4, 5];
let repl = (100..110).filter(|x| x % 2 == 0);
let u: SmallVec<i32, 16> = v.splice(1..3, repl).collect();
assert!(!v.spilled());
assert_eq!(u, [2, 3]);
assert_eq!(v, [1, 100, 102, 104, 106, 108, 4, 5]);
}

#[test]
fn into_iter() {
let mut v: SmallVec<u8, 2> = SmallVec::new();
Expand Down