mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #67290 - jonas-schievink:leak-audit, r=KodrAus

Browse Source 
Audit liballoc for leaks in `Drop` impls when user destructor panics

Inspired by https://github.com/rust-lang/rust/pull/67243 and https://github.com/rust-lang/rust/pull/67235, this audits and hopefully fixes the remaining `Drop` impls in liballoc for resource leaks in the presence of panics in destructors called by the affected `Drop` impl.

This does not touch `Hash{Map,Set}` since they live in hashbrown. They have similar issues though.

r? @KodrAus
This commit is contained in:
bors 2020-02-26 12:48:53 +00:00
commit 892cb143e5
11 changed files with 497 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/liballoc/collections/binary_heap.rs
View File

@ -147,7 +147,7 @@
use core::fmt;
use core::iter::{FromIterator, FusedIterator, TrustedLen};
use core::mem::{size_of, swap, ManuallyDrop};
use core::mem::{self, size_of, swap, ManuallyDrop};
use core::ops::{Deref, DerefMut};
use core::ptr;
@ -1239,7 +1239,19 @@ pub struct DrainSorted<'a, T: Ord> {
impl<'a, T: Ord> Drop for DrainSorted<'a, T> {
/// Removes heap elements in heap order.
fn drop(&mut self) {
while let Some(_) = self.inner.pop() {}
struct DropGuard<'r, 'a, T: Ord>(&'r mut DrainSorted<'a, T>);
impl<'r, 'a, T: Ord> Drop for DropGuard<'r, 'a, T> {
fn drop(&mut self) {
while let Some(_) = self.0.inner.pop() {}
}
}
while let Some(item) = self.inner.pop() {
let guard = DropGuard(self);
drop(item);
mem::forget(guard);
}
}
}

17
src/liballoc/collections/btree/map.rs
View File

@ -1470,7 +1470,22 @@ impl<K, V> IntoIterator for BTreeMap<K, V> {
#[stable(feature = "btree_drop", since = "1.7.0")]
impl<K, V> Drop for IntoIter<K, V> {
fn drop(&mut self) {
self.for_each(drop);
struct DropGuard<'a, K, V>(&'a mut IntoIter<K, V>);
impl<'a, K, V> Drop for DropGuard<'a, K, V> {
fn drop(&mut self) {
// Continue the same loop we perform below. This only runs when unwinding, so we
// don't have to care about panics this time (they'll abort).
while let Some(_) = self.0.next() {}
}
}
while let Some(pair) = self.next() {
let guard = DropGuard(self);
drop(pair);
mem::forget(guard);
}
unsafe {
let leaf_node = ptr::read(&self.front).into_node();
if leaf_node.is_shared_root() {

19
src/liballoc/collections/linked_list.rs
View File

@ -1611,7 +1611,24 @@ where
F: FnMut(&mut T) -> bool,
{
fn drop(&mut self) {
self.for_each(drop);
struct DropGuard<'r, 'a, T, F>(&'r mut DrainFilter<'a, T, F>)
where
F: FnMut(&mut T) -> bool;
impl<'r, 'a, T, F> Drop for DropGuard<'r, 'a, T, F>
where
F: FnMut(&mut T) -> bool,
{
fn drop(&mut self) {
self.0.for_each(drop);
}
}
while let Some(item) = self.next() {
let guard = DropGuard(self);
drop(item);
mem::forget(guard);
}
}
}

129
src/liballoc/collections/vec_deque.rs
View File

@ -22,6 +22,11 @@ use crate::collections::TryReserveError;
use crate::raw_vec::RawVec;
use crate::vec::Vec;
#[stable(feature = "drain", since = "1.6.0")]
pub use self::drain::Drain;
mod drain;
#[cfg(test)]
mod tests;
@ -866,6 +871,18 @@ impl<T> VecDeque<T> {
/// ```
#[stable(feature = "deque_extras", since = "1.16.0")]
pub fn truncate(&mut self, len: usize) {
/// Runs the destructor for all items in the slice when it gets dropped (normally or
/// during unwinding).
struct Dropper<'a, T>(&'a mut [T]);
impl<'a, T> Drop for Dropper<'a, T> {
fn drop(&mut self) {
unsafe {
ptr::drop_in_place(self.0);
}
}
}
// Safe because:
//
// * Any slice passed to `drop_in_place` is valid; the second case has
@ -888,8 +905,11 @@ impl<T> VecDeque<T> {
let drop_back = back as *mut _;
let drop_front = front.get_unchecked_mut(len..) as *mut _;
self.head = self.wrap_sub(self.head, num_dropped);
// Make sure the second half is dropped even when a destructor
// in the first one panics.
let _back_dropper = Dropper(&mut *drop_back);
ptr::drop_in_place(drop_front);
ptr::drop_in_place(drop_back);
}
}
}
@ -2526,113 +2546,6 @@ impl<T> ExactSizeIterator for IntoIter<T> {
#[stable(feature = "fused", since = "1.26.0")]
impl<T> FusedIterator for IntoIter<T> {}
/// A draining iterator over the elements of a `VecDeque`.
///
/// This `struct` is created by the [`drain`] method on [`VecDeque`]. See its
/// documentation for more.
///
/// [`drain`]: struct.VecDeque.html#method.drain
/// [`VecDeque`]: struct.VecDeque.html
#[stable(feature = "drain", since = "1.6.0")]
pub struct Drain<'a, T: 'a> {
after_tail: usize,
after_head: usize,
iter: Iter<'a, T>,
deque: NonNull<VecDeque<T>>,
}
#[stable(feature = "collection_debug", since = "1.17.0")]
impl<T: fmt::Debug> fmt::Debug for Drain<'_, T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_tuple("Drain")
.field(&self.after_tail)
.field(&self.after_head)
.field(&self.iter)
.finish()
}
}
#[stable(feature = "drain", since = "1.6.0")]
unsafe impl<T: Sync> Sync for Drain<'_, T> {}
#[stable(feature = "drain", since = "1.6.0")]
unsafe impl<T: Send> Send for Drain<'_, T> {}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> Drop for Drain<'_, T> {
fn drop(&mut self) {
self.for_each(drop);
let source_deque = unsafe { self.deque.as_mut() };
// T = source_deque_tail; H = source_deque_head; t = drain_tail; h = drain_head
//
// T t h H
// [. . . o o x x o o . . .]
//
let orig_tail = source_deque.tail;
let drain_tail = source_deque.head;
let drain_head = self.after_tail;
let orig_head = self.after_head;
let tail_len = count(orig_tail, drain_tail, source_deque.cap());
let head_len = count(drain_head, orig_head, source_deque.cap());
// Restore the original head value
source_deque.head = orig_head;
match (tail_len, head_len) {
(0, 0) => {
source_deque.head = 0;
source_deque.tail = 0;
}
(0, _) => {
source_deque.tail = drain_head;
}
(_, 0) => {
source_deque.head = drain_tail;
}
_ => unsafe {
if tail_len <= head_len {
source_deque.tail = source_deque.wrap_sub(drain_head, tail_len);
source_deque.wrap_copy(source_deque.tail, orig_tail, tail_len);
} else {
source_deque.head = source_deque.wrap_add(drain_tail, head_len);
source_deque.wrap_copy(drain_tail, drain_head, head_len);
}
},
}
}
}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> Iterator for Drain<'_, T> {
type Item = T;
#[inline]
fn next(&mut self) -> Option<T> {
self.iter.next().map(|elt| unsafe { ptr::read(elt) })
}
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.iter.size_hint()
}
}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> DoubleEndedIterator for Drain<'_, T> {
#[inline]
fn next_back(&mut self) -> Option<T> {
self.iter.next_back().map(|elt| unsafe { ptr::read(elt) })
}
}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> ExactSizeIterator for Drain<'_, T> {}
#[stable(feature = "fused", since = "1.26.0")]
impl<T> FusedIterator for Drain<'_, T> {}
#[stable(feature = "rust1", since = "1.0.0")]
impl<A: PartialEq> PartialEq for VecDeque<A> {
fn eq(&self, other: &VecDeque<A>) -> bool {

126
src/liballoc/collections/vec_deque/drain.rs Normal file
View File

@ -0,0 +1,126 @@
use core::iter::FusedIterator;
use core::ptr::{self, NonNull};
use core::{fmt, mem};
use super::{count, Iter, VecDeque};
/// A draining iterator over the elements of a `VecDeque`.
///
/// This `struct` is created by the [`drain`] method on [`VecDeque`]. See its
/// documentation for more.
///
/// [`drain`]: struct.VecDeque.html#method.drain
/// [`VecDeque`]: struct.VecDeque.html
#[stable(feature = "drain", since = "1.6.0")]
pub struct Drain<'a, T: 'a> {
pub(crate) after_tail: usize,
pub(crate) after_head: usize,
pub(crate) iter: Iter<'a, T>,
pub(crate) deque: NonNull<VecDeque<T>>,
}
#[stable(feature = "collection_debug", since = "1.17.0")]
impl<T: fmt::Debug> fmt::Debug for Drain<'_, T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_tuple("Drain")
.field(&self.after_tail)
.field(&self.after_head)
.field(&self.iter)
.finish()
}
}
#[stable(feature = "drain", since = "1.6.0")]
unsafe impl<T: Sync> Sync for Drain<'_, T> {}
#[stable(feature = "drain", since = "1.6.0")]
unsafe impl<T: Send> Send for Drain<'_, T> {}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> Drop for Drain<'_, T> {
fn drop(&mut self) {
struct DropGuard<'r, 'a, T>(&'r mut Drain<'a, T>);
impl<'r, 'a, T> Drop for DropGuard<'r, 'a, T> {
fn drop(&mut self) {
self.0.for_each(drop);
let source_deque = unsafe { self.0.deque.as_mut() };
// T = source_deque_tail; H = source_deque_head; t = drain_tail; h = drain_head
//
// T t h H
// [. . . o o x x o o . . .]
//
let orig_tail = source_deque.tail;
let drain_tail = source_deque.head;
let drain_head = self.0.after_tail;
let orig_head = self.0.after_head;
let tail_len = count(orig_tail, drain_tail, source_deque.cap());
let head_len = count(drain_head, orig_head, source_deque.cap());
// Restore the original head value
source_deque.head = orig_head;
match (tail_len, head_len) {
(0, 0) => {
source_deque.head = 0;
source_deque.tail = 0;
}
(0, _) => {
source_deque.tail = drain_head;
}
(_, 0) => {
source_deque.head = drain_tail;
}
_ => unsafe {
if tail_len <= head_len {
source_deque.tail = source_deque.wrap_sub(drain_head, tail_len);
source_deque.wrap_copy(source_deque.tail, orig_tail, tail_len);
} else {
source_deque.head = source_deque.wrap_add(drain_tail, head_len);
source_deque.wrap_copy(drain_tail, drain_head, head_len);
}
},
}
}
}
while let Some(item) = self.next() {
let guard = DropGuard(self);
drop(item);
mem::forget(guard);
}
DropGuard(self);
}
}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> Iterator for Drain<'_, T> {
type Item = T;
#[inline]
fn next(&mut self) -> Option<T> {
self.iter.next().map(|elt| unsafe { ptr::read(elt) })
}
#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.iter.size_hint()
}
}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> DoubleEndedIterator for Drain<'_, T> {
#[inline]
fn next_back(&mut self) -> Option<T> {
self.iter.next_back().map(|elt| unsafe { ptr::read(elt) })
}
}
#[stable(feature = "drain", since = "1.6.0")]
impl<T> ExactSizeIterator for Drain<'_, T> {}
#[stable(feature = "fused", since = "1.26.0")]
impl<T> FusedIterator for Drain<'_, T> {}

33
src/liballoc/tests/binary_heap.rs
View File

@ -1,6 +1,8 @@
use std::collections::binary_heap::{Drain, PeekMut};
use std::collections::BinaryHeap;
use std::iter::TrustedLen;
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::sync::atomic::{AtomicU32, Ordering};
#[test]
fn test_iterator() {
@ -275,6 +277,37 @@ fn test_drain_sorted() {
assert!(q.is_empty());
}
#[test]
fn test_drain_sorted_leak() {
static DROPS: AtomicU32 = AtomicU32::new(0);
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
struct D(u32, bool);
impl Drop for D {
fn drop(&mut self) {
DROPS.fetch_add(1, Ordering::SeqCst);
if self.1 {
panic!("panic in `drop`");
}
}
}
let mut q = BinaryHeap::from(vec![
D(0, false),
D(1, false),
D(2, false),
D(3, true),
D(4, false),
D(5, false),
]);
catch_unwind(AssertUnwindSafe(|| drop(q.drain_sorted()))).ok();
assert_eq!(DROPS.load(Ordering::SeqCst), 6);
}
#[test]
fn test_extend_ref() {
let mut a = BinaryHeap::new();

28
src/liballoc/tests/btree/map.rs
View File

@ -5,7 +5,9 @@ use std::fmt::Debug;
use std::iter::FromIterator;
use std::ops::Bound::{self, Excluded, Included, Unbounded};
use std::ops::RangeBounds;
use std::panic::catch_unwind;
use std::rc::Rc;
use std::sync::atomic::{AtomicU32, Ordering};
use super::DeterministicRng;
@ -1017,3 +1019,29 @@ fn test_split_off_large_random_sorted() {
assert!(map.into_iter().eq(data.clone().into_iter().filter(|x| x.0 < key)));
assert!(right.into_iter().eq(data.into_iter().filter(|x| x.0 >= key)));
}
#[test]
fn test_into_iter_drop_leak() {
static DROPS: AtomicU32 = AtomicU32::new(0);
struct D;
impl Drop for D {
fn drop(&mut self) {
if DROPS.fetch_add(1, Ordering::SeqCst) == 3 {
panic!("panic in `drop`");
}
}
}
let mut map = BTreeMap::new();
map.insert("a", D);
map.insert("b", D);
map.insert("c", D);
map.insert("d", D);
map.insert("e", D);
catch_unwind(move || drop(map.into_iter())).ok();
assert_eq!(DROPS.load(Ordering::SeqCst), 5);
}

70
src/liballoc/tests/linked_list.rs
View File

@ -1,5 +1,5 @@
use std::collections::LinkedList;
use std::panic::catch_unwind;
use std::panic::{catch_unwind, AssertUnwindSafe};
#[test]
fn test_basic() {
@ -531,6 +531,74 @@ fn drain_filter_complex() {
}
}
#[test]
fn drain_filter_drop_panic_leak() {
static mut DROPS: i32 = 0;
struct D(bool);
impl Drop for D {
fn drop(&mut self) {
unsafe {
DROPS += 1;
}
if self.0 {
panic!("panic in `drop`");
}
}
}
let mut q = LinkedList::new();
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_front(D(false));
q.push_front(D(true));
q.push_front(D(false));
catch_unwind(AssertUnwindSafe(|| drop(q.drain_filter(|_| true)))).ok();
assert_eq!(unsafe { DROPS }, 8);
assert!(q.is_empty());
}
#[test]
fn drain_filter_pred_panic_leak() {
static mut DROPS: i32 = 0;
#[derive(Debug)]
struct D(u32);
impl Drop for D {
fn drop(&mut self) {
unsafe {
DROPS += 1;
}
}
}
let mut q = LinkedList::new();
q.push_back(D(3));
q.push_back(D(4));
q.push_back(D(5));
q.push_back(D(6));
q.push_back(D(7));
q.push_front(D(2));
q.push_front(D(1));
q.push_front(D(0));
catch_unwind(AssertUnwindSafe(|| {
drop(q.drain_filter(|item| if item.0 >= 2 { panic!() } else { true }))
}))
.ok();
assert_eq!(unsafe { DROPS }, 2); // 0 and 1
assert_eq!(q.len(), 6);
}
#[test]
fn test_drop() {
static mut DROPS: i32 = 0;

64
src/liballoc/tests/vec.rs
View File

@ -1,6 +1,7 @@
use std::borrow::Cow;
use std::collections::TryReserveError::*;
use std::mem::size_of;
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::vec::{Drain, IntoIter};
use std::{isize, usize};
@ -585,6 +586,44 @@ fn test_drain_inclusive_out_of_bounds() {
v.drain(5..=5);
}
#[test]
fn test_drain_leak() {
static mut DROPS: i32 = 0;
#[derive(Debug, PartialEq)]
struct D(u32, bool);
impl Drop for D {
fn drop(&mut self) {
unsafe {
DROPS += 1;
}
if self.1 {
panic!("panic in `drop`");
}
}
}
let mut v = vec![
D(0, false),
D(1, false),
D(2, false),
D(3, false),
D(4, true),
D(5, false),
D(6, false),
];
catch_unwind(AssertUnwindSafe(|| {
v.drain(2..=5);
}))
.ok();
assert_eq!(unsafe { DROPS }, 4);
assert_eq!(v, vec![D(0, false), D(1, false), D(6, false),]);
}
#[test]
fn test_splice() {
let mut v = vec![1, 2, 3, 4, 5];
@ -726,6 +765,31 @@ fn test_into_iter_clone() {
assert_eq!(it.next(), None);
}
#[test]
fn test_into_iter_leak() {
static mut DROPS: i32 = 0;
struct D(bool);
impl Drop for D {
fn drop(&mut self) {
unsafe {
DROPS += 1;
}
if self.0 {
panic!("panic in `drop`");
}
}
}
let v = vec![D(false), D(true), D(false)];
catch_unwind(move || drop(v.into_iter())).ok();
assert_eq!(unsafe { DROPS }, 3);
}
#[test]
fn test_cow_from() {
let borrowed: &[_] = &["borrowed", "(slice)"];

74
src/liballoc/tests/vec_deque.rs
View File

@ -2,7 +2,7 @@ use std::collections::TryReserveError::*;
use std::collections::{vec_deque::Drain, VecDeque};
use std::fmt::Debug;
use std::mem::size_of;
use std::panic::catch_unwind;
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::{isize, usize};
use crate::hash;
@ -1573,3 +1573,75 @@ fn test_try_rfold_moves_iter() {
assert_eq!(iter.try_rfold(0_i8, |acc, &x| acc.checked_add(x)), None);
assert_eq!(iter.next_back(), Some(&70));
}
#[test]
fn truncate_leak() {
static mut DROPS: i32 = 0;
struct D(bool);
impl Drop for D {
fn drop(&mut self) {
unsafe {
DROPS += 1;
}
if self.0 {
panic!("panic in `drop`");
}
}
}
let mut q = VecDeque::new();
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_front(D(true));
q.push_front(D(false));
q.push_front(D(false));
catch_unwind(AssertUnwindSafe(|| q.truncate(1))).ok();
assert_eq!(unsafe { DROPS }, 7);
}
#[test]
fn test_drain_leak() {
static mut DROPS: i32 = 0;
#[derive(Debug, PartialEq)]
struct D(u32, bool);
impl Drop for D {
fn drop(&mut self) {
unsafe {
DROPS += 1;
}
if self.1 {
panic!("panic in `drop`");
}
}
}
let mut v = VecDeque::new();
v.push_back(D(4, false));
v.push_back(D(5, false));
v.push_back(D(6, false));
v.push_front(D(3, false));
v.push_front(D(2, true));
v.push_front(D(1, false));
v.push_front(D(0, false));
catch_unwind(AssertUnwindSafe(|| {
v.drain(1..=4);
}))
.ok();
assert_eq!(unsafe { DROPS }, 4);
assert_eq!(v.len(), 3);
drop(v);
assert_eq!(unsafe { DROPS }, 7);
}

49
src/liballoc/vec.rs
View File

@ -2622,7 +2622,9 @@ impl<T: Clone> Clone for IntoIter<T> {
unsafe impl<#[may_dangle] T> Drop for IntoIter<T> {
fn drop(&mut self) {
// destroy the remaining elements
for _x in self.by_ref() {}
unsafe {
ptr::drop_in_place(self.as_mut_slice());
}
// RawVec handles deallocation
let _ = unsafe { RawVec::from_raw_parts(self.buf.as_ptr(), self.cap) };
@ -2702,23 +2704,42 @@ impl<T> DoubleEndedIterator for Drain<'_, T> {
#[stable(feature = "drain", since = "1.6.0")]
impl<T> Drop for Drain<'_, T> {
fn drop(&mut self) {
// exhaust self first
self.for_each(drop);
/// Continues dropping the remaining elements in the `Drain`, then moves back the
/// un-`Drain`ed elements to restore the original `Vec`.
struct DropGuard<'r, 'a, T>(&'r mut Drain<'a, T>);
if self.tail_len > 0 {
unsafe {
let source_vec = self.vec.as_mut();
// memmove back untouched tail, update to new length
let start = source_vec.len();
let tail = self.tail_start;
if tail != start {
let src = source_vec.as_ptr().add(tail);
let dst = source_vec.as_mut_ptr().add(start);
ptr::copy(src, dst, self.tail_len);
impl<'r, 'a, T> Drop for DropGuard<'r, 'a, T> {
fn drop(&mut self) {
// Continue the same loop we have below. If the loop already finished, this does
// nothing.
self.0.for_each(drop);
if self.0.tail_len > 0 {
unsafe {
let source_vec = self.0.vec.as_mut();
// memmove back untouched tail, update to new length
let start = source_vec.len();
let tail = self.0.tail_start;
if tail != start {
let src = source_vec.as_ptr().add(tail);
let dst = source_vec.as_mut_ptr().add(start);
ptr::copy(src, dst, self.0.tail_len);
}
source_vec.set_len(start + self.0.tail_len);
}
}
source_vec.set_len(start + self.tail_len);
}
}
// exhaust self first
while let Some(item) = self.next() {
let guard = DropGuard(self);
drop(item);
mem::forget(guard);
}
// Drop a `DropGuard` to move back the non-drained tail of `self`.
DropGuard(self);
}
}