diff --git a/CHANGELOG.md b/CHANGELOG.md index 76db5231b2..93e0090b99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed * Fixed `pool` example in docstring. +* Fixed undefined behavior in `Vec::truncate()`, `Vec::swap_remove_unchecked()`, + and `Hole::move_to()` (internal to the binary heap implementation). ### Added diff --git a/src/binary_heap.rs b/src/binary_heap.rs index f82f3d7d2f..dc9ea590cc 100644 --- a/src/binary_heap.rs +++ b/src/binary_heap.rs @@ -428,8 +428,9 @@ impl<'a, T> Hole<'a, T> { unsafe fn move_to(&mut self, index: usize) { debug_assert!(index != self.pos); debug_assert!(index < self.data.len()); - let index_ptr: *const _ = self.data.get_unchecked(index); - let hole_ptr = self.data.get_unchecked_mut(self.pos); + let ptr = self.data.as_mut_ptr(); + let index_ptr: *const _ = ptr.add(index); + let hole_ptr = ptr.add(self.pos); ptr::copy_nonoverlapping(index_ptr, hole_ptr, 1); self.pos = index; } diff --git a/src/vec.rs b/src/vec.rs index f2866c0c81..cc8202b751 100644 --- a/src/vec.rs +++ b/src/vec.rs @@ -263,13 +263,24 @@ impl Vec { /// Shortens the vector, keeping the first `len` elements and dropping the rest. pub fn truncate(&mut self, len: usize) { - // drop any extra elements - while len < self.len { - // decrement len before the drop_in_place(), so a panic on Drop - // doesn't re-drop the just-failed value. - self.len -= 1; - let len = self.len; - unsafe { ptr::drop_in_place(self.as_mut_slice().get_unchecked_mut(len)) }; + // This is safe because: + // + // * the slice passed to `drop_in_place` is valid; the `len > self.len` + // case avoids creating an invalid slice, and + // * the `len` of the vector is shrunk before calling `drop_in_place`, + // such that no value will be dropped twice in case `drop_in_place` + // were to panic once (if it panics twice, the program aborts). + unsafe { + // Note: It's intentional that this is `>` and not `>=`. + // Changing it to `>=` has negative performance + // implications in some cases. See rust-lang/rust#78884 for more. + if len > self.len { + return; + } + let remaining_len = self.len - len; + let s = ptr::slice_from_raw_parts_mut(self.as_mut_ptr().add(len), remaining_len); + self.len = len; + ptr::drop_in_place(s); } } @@ -471,11 +482,11 @@ impl Vec { pub unsafe fn swap_remove_unchecked(&mut self, index: usize) -> T { let length = self.len(); debug_assert!(index < length); - ptr::swap( - self.as_mut_slice().get_unchecked_mut(index), - self.as_mut_slice().get_unchecked_mut(length - 1), - ); - self.pop_unchecked() + let value = ptr::read(self.as_ptr().add(index)); + let base_ptr = self.as_mut_ptr(); + ptr::copy(base_ptr.add(length - 1), base_ptr.add(index), 1); + self.len -= 1; + value } /// Returns true if the vec is full