From 8d0eded7ec508b40681979cb2bc8a9676be2752d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 8 Jul 2020 18:18:55 -0700 Subject: [PATCH 1/2] Don't truncate if new length is equal to old `truncate` should do nothing for collections if the new length is equal to the current length, but this was not reflected in the code. Instead, control flow continued until an empty slice was passed to `drop_in_place`. --- src/liballoc/collections/vec_deque.rs | 6 +++--- src/liballoc/string.rs | 8 +++++--- src/liballoc/vec.rs | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 15f3a94ca2d6a..2389186924bcf 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -913,12 +913,12 @@ impl VecDeque { // Safe because: // // * Any slice passed to `drop_in_place` is valid; the second case has - // `len <= front.len()` and returning on `len > self.len()` ensures - // `begin <= back.len()` in the first case + // `len <= front.len()` and returning on `len >= self.len()` ensures + // `begin < back.len()` in the first case // * The head of the VecDeque is moved before calling `drop_in_place`, // so no value is dropped twice if `drop_in_place` panics unsafe { - if len > self.len() { + if len >= self.len() { return; } let num_dropped = self.len() - len; diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs index 5b671b41b5bf6..029de70d0c8b7 100644 --- a/src/liballoc/string.rs +++ b/src/liballoc/string.rs @@ -1149,10 +1149,12 @@ impl String { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn truncate(&mut self, new_len: usize) { - if new_len <= self.len() { - assert!(self.is_char_boundary(new_len)); - self.vec.truncate(new_len) + if new_len >= self.len() { + return; } + + assert!(self.is_char_boundary(new_len)); + self.vec.truncate(new_len) } /// Removes the last character from the string buffer and returns it. diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 5db96a504a6a6..3b5ce8193733a 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -736,13 +736,13 @@ impl Vec { pub fn truncate(&mut self, len: usize) { // This is safe because: // - // * the slice passed to `drop_in_place` is valid; the `len > self.len` + // * 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 { - if len > self.len { + if len >= self.len { return; } let remaining_len = self.len - len; From d88f09ba9720df92610c66ac81ea69fd8f964a7e Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 9 Jul 2020 10:43:00 -0700 Subject: [PATCH 2/2] Implement `Vec::clear` by hand The new version of `truncate` compiles up a branch when called with a constant `0` and `T: !Drop`. --- src/liballoc/vec.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 3b5ce8193733a..ff2a1a76d3201 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -1375,7 +1375,19 @@ impl Vec { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn clear(&mut self) { - self.truncate(0) + let elems: *mut [T] = self.as_mut_slice(); + + // SAFETY: + // + // * `elems` comes directly from `as_mut_slice` and is therefore valid. + // + // * Setting `self.len = 0` before calling `drop_in_place` means that, if an element's + // `Drop` impl panics, the vector's `Drop` impl will do nothing (leaking the rest of + // the elements) instead of dropping some twice. + unsafe { + self.len = 0; + ptr::drop_in_place(elems); + } } /// Returns the number of elements in the vector, also referred to