Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable Miri leak checker #209

Merged
merged 2 commits into from
Apr 11, 2020
Merged

enable Miri leak checker #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
562568b
manually clean up leaked Boxes, to enable Miri's leak checker
RalfJung Apr 11, 2020
cbe3620
note the leak in insert_many docs
RalfJung Apr 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,8 @@ impl<A: Array> SmallVec<A> {

/// Insert multiple elements at position `index`, shifting all following elements toward the
/// back.
///
/// Note: when the iterator panics, this can leak memory.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this when I read that the leak was intentional; if this is still considered a bug I can also remove it again.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it for now, and remove it if/when the leak is fixed.

pub fn insert_many<I: IntoIterator<Item = A::Item>>(&mut self, index: usize, iterable: I) {
let iter = iterable.into_iter();
if index == self.len() {
Expand Down Expand Up @@ -2092,19 +2094,27 @@ mod tests {
}
}

// These boxes are leaked on purpose by panicking `insert_many`,
// so we clean them up manually to appease Miri's leak checker.
let mut box1 = Box::new(false);
let mut box2 = Box::new(false);

let mut vec: SmallVec<[PanicOnDoubleDrop; 0]> = vec![
PanicOnDoubleDrop {
dropped: Box::new(false),
dropped: unsafe { Box::from_raw(&mut *box1) },
},
PanicOnDoubleDrop {
dropped: Box::new(false),
dropped: unsafe { Box::from_raw(&mut *box2) },
},
]
.into();
let result = ::std::panic::catch_unwind(move || {
vec.insert_many(0, BadIter);
});
assert!(result.is_err());

drop(box1);
drop(box2);
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions scripts/run_miri.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ rustup default "$MIRI_NIGHTLY"
rustup component add miri
cargo miri setup

cargo miri test --verbose -- -Zmiri-ignore-leaks
cargo miri test --verbose --features union -- -Zmiri-ignore-leaks
cargo miri test --verbose --all-features -- -Zmiri-ignore-leaks
cargo miri test --verbose
cargo miri test --verbose --features union
cargo miri test --verbose --all-features