Skip to content

Commit 8fd2b9b

Browse files
author
bors-servo
authored
Auto merge of #103 - mbrubeck:panic, r=jdm
Panic-safety fixes * Make from_elem panic-safe. Fixes #101. * Make insert_many panic-safe. Fixes #96. r? @SimonSapin or @jdm. cc @Vurich <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/103) <!-- Reviewable:end -->
2 parents 5a8ab46 + 1f40252 commit 8fd2b9b

File tree

1 file changed

+83
-18
lines changed

1 file changed

+83
-18
lines changed

lib.rs

Lines changed: 83 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -771,24 +771,33 @@ impl<A: Array> SmallVec<A> {
771771
unsafe {
772772
let old_len = self.len();
773773
assert!(index <= old_len);
774-
let ptr = self.as_mut_ptr().offset(index as isize);
774+
let mut ptr = self.as_mut_ptr().offset(index as isize);
775+
776+
// Move the trailing elements.
775777
ptr::copy(ptr, ptr.offset(lower_size_bound as isize), old_len - index);
776-
for (off, element) in iter.enumerate() {
777-
if off < lower_size_bound {
778-
ptr::write(ptr.offset(off as isize), element);
779-
let len = self.len() + 1;
780-
self.set_len(len);
781-
} else {
782-
// Iterator provided more elements than the hint.
783-
assert!(index + off >= index); // Protect against overflow.
784-
self.insert(index + off, element);
778+
779+
// In case the iterator panics, don't double-drop the items we just copied above.
780+
self.set_len(index);
781+
782+
let mut num_added = 0;
783+
for element in iter {
784+
let mut cur = ptr.offset(num_added as isize);
785+
if num_added >= lower_size_bound {
786+
// Iterator provided more elements than the hint. Move trailing items again.
787+
self.reserve(1);
788+
ptr = self.as_mut_ptr().offset(index as isize);
789+
cur = ptr.offset(num_added as isize);
790+
ptr::copy(cur, cur.offset(1), old_len - index);
785791
}
792+
ptr::write(cur, element);
793+
num_added += 1;
786794
}
787-
let num_added = self.len() - old_len;
788795
if num_added < lower_size_bound {
789796
// Iterator provided fewer elements than the hint
790797
ptr::copy(ptr.offset(lower_size_bound as isize), ptr.offset(num_added as isize), old_len - index);
791798
}
799+
800+
self.set_len(old_len + num_added);
792801
}
793802
}
794803

@@ -937,19 +946,17 @@ impl<A: Array> SmallVec<A> where A::Item: Clone {
937946
if n > A::size() {
938947
vec![elem; n].into()
939948
} else {
949+
let mut v = SmallVec::<A>::new();
940950
unsafe {
941-
let mut arr: A = ::std::mem::uninitialized();
942-
let ptr = arr.ptr_mut();
951+
let (ptr, len_ptr, _) = v.triple_mut();
952+
let mut local_len = SetLenOnDrop::new(len_ptr);
943953

944954
for i in 0..n as isize {
945955
::std::ptr::write(ptr.offset(i), elem.clone());
946-
}
947-
948-
SmallVec {
949-
capacity: n,
950-
data: SmallVecData::from_inline(arr),
956+
local_len.increment_len(1);
951957
}
952958
}
959+
v
953960
}
954961
}
955962
}
@@ -1337,6 +1344,33 @@ pub unsafe trait Array {
13371344
fn ptr_mut(&mut self) -> *mut Self::Item;
13381345
}
13391346

1347+
/// Set the length of the vec when the `SetLenOnDrop` value goes out of scope.
1348+
///
1349+
/// Copied from https://github.com/rust-lang/rust/pull/36355
1350+
struct SetLenOnDrop<'a> {
1351+
len: &'a mut usize,
1352+
local_len: usize,
1353+
}
1354+
1355+
impl<'a> SetLenOnDrop<'a> {
1356+
#[inline]
1357+
fn new(len: &'a mut usize) -> Self {
1358+
SetLenOnDrop { local_len: *len, len: len }
1359+
}
1360+
1361+
#[inline]
1362+
fn increment_len(&mut self, increment: usize) {
1363+
self.local_len += increment;
1364+
}
1365+
}
1366+
1367+
impl<'a> Drop for SetLenOnDrop<'a> {
1368+
#[inline]
1369+
fn drop(&mut self) {
1370+
*self.len = self.local_len;
1371+
}
1372+
}
1373+
13401374
macro_rules! impl_array(
13411375
($($size:expr),+) => {
13421376
$(
@@ -1645,6 +1679,37 @@ mod tests {
16451679
assert_eq!(&v.iter().map(|v| *v).collect::<Vec<_>>(), &[0, 5, 6, 1, 2, 3]);
16461680
}
16471681

1682+
#[test]
1683+
// https://github.com/servo/rust-smallvec/issues/96
1684+
fn test_insert_many_panic() {
1685+
struct PanicOnDoubleDrop {
1686+
dropped: Box<bool>
1687+
}
1688+
1689+
impl Drop for PanicOnDoubleDrop {
1690+
fn drop(&mut self) {
1691+
assert!(!*self.dropped, "already dropped");
1692+
*self.dropped = true;
1693+
}
1694+
}
1695+
1696+
struct BadIter;
1697+
impl Iterator for BadIter {
1698+
type Item = PanicOnDoubleDrop;
1699+
fn size_hint(&self) -> (usize, Option<usize>) { (1, None) }
1700+
fn next(&mut self) -> Option<Self::Item> { panic!() }
1701+
}
1702+
1703+
let mut vec: SmallVec<[PanicOnDoubleDrop; 0]> = vec![
1704+
PanicOnDoubleDrop { dropped: Box::new(false) },
1705+
PanicOnDoubleDrop { dropped: Box::new(false) },
1706+
].into();
1707+
let result = ::std::panic::catch_unwind(move || {
1708+
vec.insert_many(0, BadIter);
1709+
});
1710+
assert!(result.is_err());
1711+
}
1712+
16481713
#[test]
16491714
#[should_panic]
16501715
fn test_invalid_grow() {

0 commit comments

Comments
 (0)