Skip to content

Commit 54d82d0

Browse files
borsbors
committedAug 23, 2018
Auto merge of #53571 - MaloJaffre:vecdeque-emergency, r=RalfJung
Fix unsoundness for VecDeque See individual commit for more details. r? @RalfJung. Fixes #53566, fixes #53529
2 parents e5284b0 + f8d5ed4 commit 54d82d0

File tree

1 file changed

+27
-167
lines changed

1 file changed

+27
-167
lines changed
 

‎src/liballoc/collections/vec_deque.rs

+27-167
Original file line numberDiff line numberDiff line change
@@ -202,23 +202,6 @@ impl<T> VecDeque<T> {
202202
len);
203203
}
204204

205-
/// Returns a pair of slices which contain the contents of the buffer not used by the VecDeque.
206-
#[inline]
207-
unsafe fn unused_as_mut_slices<'a>(&'a mut self) -> (&'a mut [T], &'a mut [T]) {
208-
let head = self.head;
209-
let tail = self.tail;
210-
let buf = self.buffer_as_mut_slice();
211-
if head != tail {
212-
// In buf, head..tail contains the VecDeque and tail..head is unused.
213-
// So calling `ring_slices` with tail and head swapped returns unused slices.
214-
RingSlices::ring_slices(buf, tail, head)
215-
} else {
216-
// Swapping doesn't help when head == tail.
217-
let (before, after) = buf.split_at_mut(head);
218-
(after, before)
219-
}
220-
}
221-
222205
/// Copies a potentially wrapping block of memory len long from src to dest.
223206
/// (abs(dst - src) + len) must be no larger than cap() (There must be at
224207
/// most one continuous overlapping region between src and dest).
@@ -1851,148 +1834,8 @@ impl<T> VecDeque<T> {
18511834
#[inline]
18521835
#[stable(feature = "append", since = "1.4.0")]
18531836
pub fn append(&mut self, other: &mut Self) {
1854-
// Copies all values from `src_slice` to the start of `dst_slice`.
1855-
unsafe fn copy_whole_slice<T>(src_slice: &[T], dst_slice: &mut [T]) {
1856-
let len = src_slice.len();
1857-
ptr::copy_nonoverlapping(src_slice.as_ptr(), dst_slice[..len].as_mut_ptr(), len);
1858-
}
1859-
1860-
let src_total = other.len();
1861-
1862-
// Guarantees there is space in `self` for `other`.
1863-
self.reserve(src_total);
1864-
1865-
self.head = {
1866-
let original_head = self.head;
1867-
1868-
// The goal is to copy all values from `other` into `self`. To avoid any
1869-
// mismatch, all valid values in `other` are retrieved...
1870-
let (src_high, src_low) = other.as_slices();
1871-
// and unoccupied parts of self are retrieved.
1872-
let (dst_high, dst_low) = unsafe { self.unused_as_mut_slices() };
1873-
1874-
// Then all that is needed is to copy all values from
1875-
// src (src_high and src_low) to dst (dst_high and dst_low).
1876-
//
1877-
// other [o o o . . . . . o o o o]
1878-
// [5 6 7] [1 2 3 4]
1879-
// src_low src_high
1880-
//
1881-
// self [. . . . . . o o o o . .]
1882-
// [3 4 5 6 7 .] [1 2]
1883-
// dst_low dst_high
1884-
//
1885-
// Values are not copied one by one but as slices in `copy_whole_slice`.
1886-
// What slices are used depends on various properties of src and dst.
1887-
// There are 6 cases in total:
1888-
// 1. `src` is contiguous and fits in dst_high
1889-
// 2. `src` is contiguous and does not fit in dst_high
1890-
// 3. `src` is discontiguous and fits in dst_high
1891-
// 4. `src` is discontiguous and does not fit in dst_high
1892-
// + src_high is smaller than dst_high
1893-
// 5. `src` is discontiguous and does not fit in dst_high
1894-
// + dst_high is smaller than src_high
1895-
// 6. `src` is discontiguous and does not fit in dst_high
1896-
// + dst_high is the same size as src_high
1897-
let src_contiguous = src_low.is_empty();
1898-
let dst_high_fits_src = dst_high.len() >= src_total;
1899-
match (src_contiguous, dst_high_fits_src) {
1900-
(true, true) => {
1901-
// 1.
1902-
// other [. . . o o o . . . . . .]
1903-
// [] [1 1 1]
1904-
//
1905-
// self [. o o o o o . . . . . .]
1906-
// [.] [1 1 1 . . .]
1907-
1908-
unsafe {
1909-
copy_whole_slice(src_high, dst_high);
1910-
}
1911-
original_head + src_total
1912-
}
1913-
(true, false) => {
1914-
// 2.
1915-
// other [. . . o o o o o . . . .]
1916-
// [] [1 1 2 2 2]
1917-
//
1918-
// self [. . . . . . . o o o . .]
1919-
// [2 2 2 . . . .] [1 1]
1920-
1921-
let (src_1, src_2) = src_high.split_at(dst_high.len());
1922-
unsafe {
1923-
copy_whole_slice(src_1, dst_high);
1924-
copy_whole_slice(src_2, dst_low);
1925-
}
1926-
src_total - dst_high.len()
1927-
}
1928-
(false, true) => {
1929-
// 3.
1930-
// other [o o . . . . . . . o o o]
1931-
// [2 2] [1 1 1]
1932-
//
1933-
// self [. o o . . . . . . . . .]
1934-
// [.] [1 1 1 2 2 . . . .]
1935-
1936-
let (dst_1, dst_2) = dst_high.split_at_mut(src_high.len());
1937-
unsafe {
1938-
copy_whole_slice(src_high, dst_1);
1939-
copy_whole_slice(src_low, dst_2);
1940-
}
1941-
original_head + src_total
1942-
}
1943-
(false, false) => {
1944-
if src_high.len() < dst_high.len() {
1945-
// 4.
1946-
// other [o o o . . . . . . o o o]
1947-
// [2 3 3] [1 1 1]
1948-
//
1949-
// self [. . . . . . o o . . . .]
1950-
// [3 3 . . . .] [1 1 1 2]
1951-
1952-
let (dst_1, dst_2) = dst_high.split_at_mut(src_high.len());
1953-
let (src_2, src_3) = src_low.split_at(dst_2.len());
1954-
unsafe {
1955-
copy_whole_slice(src_high, dst_1);
1956-
copy_whole_slice(src_2, dst_2);
1957-
copy_whole_slice(src_3, dst_low);
1958-
}
1959-
src_3.len()
1960-
} else if src_high.len() > dst_high.len() {
1961-
// 5.
1962-
// other [o o o . . . . . o o o o]
1963-
// [3 3 3] [1 1 2 2]
1964-
//
1965-
// self [. . . . . . o o o o . .]
1966-
// [2 2 3 3 3 .] [1 1]
1967-
1968-
let (src_1, src_2) = src_high.split_at(dst_high.len());
1969-
let (dst_2, dst_3) = dst_low.split_at_mut(src_2.len());
1970-
unsafe {
1971-
copy_whole_slice(src_1, dst_high);
1972-
copy_whole_slice(src_2, dst_2);
1973-
copy_whole_slice(src_low, dst_3);
1974-
}
1975-
dst_2.len() + src_low.len()
1976-
} else {
1977-
// 6.
1978-
// other [o o . . . . . . . o o o]
1979-
// [2 2] [1 1 1]
1980-
//
1981-
// self [. . . . . . . o o . . .]
1982-
// [2 2 . . . . .] [1 1 1]
1983-
1984-
unsafe {
1985-
copy_whole_slice(src_high, dst_high);
1986-
copy_whole_slice(src_low, dst_low);
1987-
}
1988-
src_low.len()
1989-
}
1990-
}
1991-
}
1992-
};
1993-
1994-
// Some values now exist in both `other` and `self` but are made inaccessible in `other`.
1995-
other.tail = other.head;
1837+
// naive impl
1838+
self.extend(other.drain(..));
19961839
}
19971840

19981841
/// Retains only the elements specified by the predicate.
@@ -2145,11 +1988,11 @@ pub struct Iter<'a, T: 'a> {
21451988
#[stable(feature = "collection_debug", since = "1.17.0")]
21461989
impl<'a, T: 'a + fmt::Debug> fmt::Debug for Iter<'a, T> {
21471990
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
1991+
let (front, back) = RingSlices::ring_slices(self.ring, self.head, self.tail);
21481992
f.debug_tuple("Iter")
2149-
.field(&self.ring)
2150-
.field(&self.tail)
2151-
.field(&self.head)
2152-
.finish()
1993+
.field(&front)
1994+
.field(&back)
1995+
.finish()
21531996
}
21541997
}
21551998

@@ -2242,11 +2085,11 @@ pub struct IterMut<'a, T: 'a> {
22422085
#[stable(feature = "collection_debug", since = "1.17.0")]
22432086
impl<'a, T: 'a + fmt::Debug> fmt::Debug for IterMut<'a, T> {
22442087
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
2088+
let (front, back) = RingSlices::ring_slices(&*self.ring, self.head, self.tail);
22452089
f.debug_tuple("IterMut")
2246-
.field(&self.ring)
2247-
.field(&self.tail)
2248-
.field(&self.head)
2249-
.finish()
2090+
.field(&front)
2091+
.field(&back)
2092+
.finish()
22502093
}
22512094
}
22522095

@@ -3124,4 +2967,21 @@ mod tests {
31242967
}
31252968
}
31262969

2970+
#[test]
2971+
fn issue_53529() {
2972+
use boxed::Box;
2973+
2974+
let mut dst = VecDeque::new();
2975+
dst.push_front(Box::new(1));
2976+
dst.push_front(Box::new(2));
2977+
assert_eq!(*dst.pop_back().unwrap(), 1);
2978+
2979+
let mut src = VecDeque::new();
2980+
src.push_front(Box::new(2));
2981+
dst.append(&mut src);
2982+
for a in dst {
2983+
assert_eq!(*a, 2);
2984+
}
2985+
}
2986+
31272987
}

0 commit comments

Comments
 (0)