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.

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

Add safety preconditions to alloc/src/collections/binary_heap/mod.rs #120

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions library/alloc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ edition = "2021"
[dependencies]
core = { path = "../core" }
compiler_builtins = { version = "0.1.123", features = ['rustc-dep-of-std'] }
safety = { path = "../contracts/safety" }

[dev-dependencies]
rand = { version = "0.8.5", default-features = false, features = ["alloc"] }
Expand Down
69 changes: 69 additions & 0 deletions library/alloc/src/collections/binary_heap/mod.rs

Choose a reason for hiding this comment

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

Do these function require self to be safe?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you asking whether there are requirements that the object is in some sort of sane state? I would believe so, but why would we ever permit for that not to be the case?

Choose a reason for hiding this comment

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

I agree that we shouldn't, but we do.

Conceptually, internal functions can temporary break the safety invariants of the types.

Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@
#![allow(missing_docs)]
#![stable(feature = "rust1", since = "1.0.0")]

use safety::requires;
#[cfg(kani)]
#[unstable(feature="kani", issue="none")]
use core::kani;

use core::alloc::Allocator;
use core::iter::{FusedIterator, InPlaceIterable, SourceIter, TrustedFused, TrustedLen};
use core::mem::{self, swap, ManuallyDrop};
Expand Down Expand Up @@ -672,6 +677,8 @@ impl<T: Ord, A: Allocator> BinaryHeap<T, A> {
/// # Safety
///
/// The caller must guarantee that `pos < self.len()`.
#[requires(pos < self.len())]
#[cfg_attr(kani, kani::modifies(self.data.as_ptr()))]
unsafe fn sift_up(&mut self, start: usize, pos: usize) -> usize {
// Take out the value at `pos` and create a hole.
// SAFETY: The caller guarantees that pos < self.len()
Expand Down Expand Up @@ -701,6 +708,8 @@ impl<T: Ord, A: Allocator> BinaryHeap<T, A> {
/// # Safety
///
/// The caller must guarantee that `pos < end <= self.len()`.
#[requires(pos < end && end <= self.len())]
#[cfg_attr(kani, kani::modifies(self.data.as_ptr()))]
unsafe fn sift_down_range(&mut self, pos: usize, end: usize) {
// SAFETY: The caller guarantees that pos < end <= self.len().
let mut hole = unsafe { Hole::new(&mut self.data, pos) };
Expand Down Expand Up @@ -741,6 +750,8 @@ impl<T: Ord, A: Allocator> BinaryHeap<T, A> {
/// # Safety
///
/// The caller must guarantee that `pos < self.len()`.
#[requires(pos < self.len())]
#[cfg_attr(kani, kani::modifies(self.data.as_ptr()))]
unsafe fn sift_down(&mut self, pos: usize) {
let len = self.len();
// SAFETY: pos < len is guaranteed by the caller and
Expand All @@ -757,6 +768,8 @@ impl<T: Ord, A: Allocator> BinaryHeap<T, A> {
/// # Safety
///
/// The caller must guarantee that `pos < self.len()`.
#[requires(pos < self.len())]
#[cfg_attr(kani, kani::modifies(self.data.as_ptr()))]
unsafe fn sift_down_to_bottom(&mut self, mut pos: usize) {
let end = self.len();
let start = pos;
Expand Down Expand Up @@ -1897,3 +1910,59 @@ impl<'a, T: 'a + Ord + Copy, A: Allocator> Extend<&'a T> for BinaryHeap<T, A> {
self.reserve(additional);
}
}

#[cfg(kani)]
#[unstable(feature="kani", issue="none")]
mod verify {
use super::*;

// TODO: Kani reports as failing property "Only a single top-level call", which does not

Choose a reason for hiding this comment

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

what exactly is the error?

// obviously make sense. Requires investigation.
// // unsafe fn sift_up(&mut self, start: usize, pos: usize) -> usize
// #[kani::proof_for_contract(BinaryHeap<T, A>::sift_up)]
// #[kani::unwind(1)]
// pub fn check_sift_up() {
// // TODO: this isn't exactly an arbitrary heap
// let mut heap = BinaryHeap::new_in(Global);
// heap.push(kani::any::<usize>());
// unsafe {
// let _ = heap.sift_up(kani::any(), kani::any());
// }
// }

// unsafe fn sift_down_range(&mut self, pos: usize, end: usize)
#[kani::proof_for_contract(BinaryHeap<T, A>::sift_down_range)]
#[kani::unwind(1)]
pub fn check_sift_down_range() {
// TODO: this isn't exactly an arbitrary heap
let mut heap = BinaryHeap::new_in(Global);
heap.push(kani::any::<usize>());
unsafe {
let _ = heap.sift_down_range(kani::any(), kani::any());
}
}

// unsafe fn sift_down(&mut self, pos: usize)
#[kani::proof_for_contract(BinaryHeap<T, A>::sift_down)]
#[kani::unwind(1)]
pub fn check_sift_down() {
// TODO: this isn't exactly an arbitrary heap
let mut heap = BinaryHeap::new_in(Global);
heap.push(kani::any::<usize>());
unsafe {
let _ = heap.sift_down(kani::any());
}
}

// unsafe fn sift_down_to_bottom(&mut self, mut pos: usize)
#[kani::proof_for_contract(BinaryHeap<T, A>::sift_down_to_bottom)]
#[kani::unwind(1)]
pub fn check_sift_down_to_bottom() {
// TODO: this isn't exactly an arbitrary heap
let mut heap = BinaryHeap::new_in(Global);
heap.push(kani::any::<usize>());
unsafe {
let _ = heap.sift_down_to_bottom(kani::any());
}
}
}
1 change: 1 addition & 0 deletions library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
//
// Library features:
// tidy-alphabetical-start
#![cfg_attr(kani, feature(kani))]
#![cfg_attr(not(no_global_oom_handling), feature(const_alloc_error))]
#![cfg_attr(not(no_global_oom_handling), feature(const_btree_len))]
#![feature(alloc_layout_extra)]
Expand Down
1 change: 1 addition & 0 deletions library/std/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ hashbrown = { version = "0.14", default-features = false, features = [
std_detect = { path = "../stdarch/crates/std_detect", default-features = false, features = [
'rustc-dep-of-std',
] }
safety = { path = "../contracts/safety" }

# Dependencies of the `backtrace` crate
rustc-demangle = { version = "0.1.24", features = ['rustc-dep-of-std'] }
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@
#![cfg_attr(any(windows, target_os = "uefi"), feature(round_char_boundary))]
#![cfg_attr(target_family = "wasm", feature(stdarch_wasm_atomic_wait))]
#![cfg_attr(target_arch = "wasm64", feature(simd_wasm64))]
#![cfg_attr(kani, feature(kani))]
//
// Language features:
// tidy-alphabetical-start
Expand Down
Loading