Skip to content

Commit

Permalink
Introduced unwind safety
Browse files Browse the repository at this point in the history
Previous versions are unsound if closures in _with methods panic and the
unwinding panic runtime is used.
  • Loading branch information
kotauskas committed Nov 2, 2020
1 parent c044c4b commit cf0906c
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 13 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ slotmap = {version = "0.4", optional = true}
[features]
default = [
"std",
"unwind_safety",
"alloc",

"binary_tree",
Expand All @@ -33,6 +34,7 @@ default = [
"slotmap",
]
std = ["alloc"]
unwind_safety = ["std"]
alloc = []

binary_tree = []
Expand All @@ -46,6 +48,7 @@ doc_cfg = []
[package.metadata.docs.rs]
features = [
"std",
"unwind_safety",
"alloc",

"binary_tree",
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ By default, all trees use a technique called "sparse storage" to significantly s
- As elements get removed, they will leave holes behind themselves. Those are usually cleaned up automatically as new elements are inserted, **but if you need to clean up all at once, you can use `defragment`.**

## Feature flags
- `std` (**enabled by default**) - enables the full standard library, disabling `no_std` for the crate. Currently, this only adds [`Error`] trait implementations for some types.
- `std` (**enabled by default**) — enables the full standard library, disabling `no_std` for the crate. Currently, this only adds [`Error`] trait implementations for some types.
- `unwind_safety` (**enabled by default**) — **Must be enabled when using the unwinding panic implementation, otherwise using methods which accept closures is undefined behavior.** Requires `std`. Not a concern in `no_std` builds, since those do not have a panicking runtime by default.
- `alloc` (**enabled by default**) — adds `ListStorage` trait implementations for standard library containers, except for `LinkedList`, which is temporarily unsupported. *This does not require standard library support and will only panic at runtime in `no_std` environments without an allocator.*
- `smallvec` (**enabled by default**) — adds a `ListStorage` trait implementation for [`SmallVec`].
- `slotmap` (**enabled by default**) — adds `Storage` trait implementations for [`SlotMap`], [`HopSlotMap`] and [`DenseSlotMap`].
Expand Down
17 changes: 13 additions & 4 deletions src/binary_tree/node_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use core::{
};
use crate::{
storage::{Storage, DefaultStorage},
util::unreachable_debugchecked,
util::{unreachable_debugchecked, abort_on_panic},
TryRemoveLeafError,
TryRemoveBranchError,
TryRemoveChildrenError,
Expand Down Expand Up @@ -513,7 +513,9 @@ debug key check failed: tried to reference key {:?} which is not present in the
// SAFETY: as above
ptr::write(
&mut self.tree.storage.get_unchecked_mut(&parent_key).value,
NodeData::Leaf(f(old_payload)),
NodeData::Leaf(
abort_on_panic(|| f(old_payload))
),
);
}
}
Expand Down Expand Up @@ -602,7 +604,9 @@ debug key check failed: tried to reference key {:?} which is not present in the
// SAFETY: as above
ptr::write(
&mut self.tree.storage.get_unchecked_mut(&parent_key).value,
NodeData::Leaf(f(old_payload)),
NodeData::Leaf(
abort_on_panic(|| f(old_payload))
),
);
}
}
Expand Down Expand Up @@ -711,7 +715,12 @@ debug key check failed: tried to reference key {:?} which is not present in the
};
unsafe {
// SAFETY: as above
ptr::write(&mut self.node_mut().value, NodeData::Leaf(f(old_payload)));
ptr::write(
&mut self.node_mut().value,
NodeData::Leaf(
abort_on_panic(|| f(old_payload))
)
);
}
Ok((left_child_payload, right_child_payload))
}
Expand Down
19 changes: 17 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
//! - As elements get removed, they will leave holes behind themselves. Those are usually cleaned up automatically as new elements are inserted, **but if you need to clean up all at once, you can use `defragment`.**
//!
//! # Feature flags
//! - `std` (**enabled by default**) - enables the full standard library, disabling `no_std` for the crate. Currently, this only adds [`Error`] trait implementations for some types.
//! - `std` (**enabled by default**) — enables the full standard library, disabling `no_std` for the crate. Currently, this only adds [`Error`] trait implementations for some types.
//! - `unwind_safety` (**enabled by default**) — **Must be enabled when using the unwinding panic implementation, otherwise using methods which accept closures is undefined behavior.** Requires `std`. Not a concern in `no_std` builds, since those do not have a panicking runtime by default.
//! - `alloc` (**enabled by default**) — adds `ListStorage` trait implementations for standard library containers, except for `LinkedList`, which is temporarily unsupported. *This does not require standard library support and will only panic at runtime in `no_std` environments without an allocator.*
//! - `smallvec` (**enabled by default**) — adds a `ListStorage` trait implementation for [`SmallVec`].
//! - `slotmap` (**enabled by default**) — adds `Storage` trait implementations for [`SlotMap`], [`HopSlotMap`] and [`DenseSlotMap`].
Expand Down Expand Up @@ -133,7 +134,6 @@
#![deny(
anonymous_parameters,
bare_trait_objects,
clippy::exit,
)]
#![allow(clippy::use_self)] // FIXME reenable when it gets fixed
#![cfg_attr(not(feature = "std"), no_std)]
Expand Down Expand Up @@ -170,6 +170,13 @@ pub mod quadtree;
#[cfg_attr(feature = "doc_cfg", doc(cfg(feature = "quadtree")))]
pub use quadtree::{Quadtree};

#[cfg(feature = "freeform_tree")]
#[cfg_attr(feature = "doc_cfg", doc(cfg(feature = "freeform_tree")))]
pub mod freeform_tree;
#[cfg(feature = "freeform_tree")]
#[cfg_attr(feature = "doc_cfg", doc(cfg(feature = "freeform_tree")))]
pub use freeform_tree::{FreeformTree};

pub mod traversal;
pub use traversal::{Visitor, VisitorMut, Traversable, TraversableMut};

Expand Down Expand Up @@ -205,6 +212,14 @@ pub mod prelude {
NodeRef as QuadtreeNodeRef,
NodeRefMut as QuadtreeNodeRefMut,
};
#[cfg(feature = "freeform_tree")]
#[cfg_attr(feature = "doc_cfg", doc(cfg(feature = "freeform_tree")))]
#[doc(no_inline)]
pub use crate::freeform_tree::{
FreeformTree,
NodeRef as FreeformTreeNodeRef,
NodeRefMut as FreeformTreeNodeRefMut,
};
}

pub(crate) mod util;
Expand Down
2 changes: 1 addition & 1 deletion src/octree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl<B, L> MoveFix for Node<B, L, usize> {
return;
}
}
unsafe {
/*unsafe*/ {
// SAFETY: this mismatch is assumed to never happen as a guarantee
// of key validity
unreachable_debugchecked("failed to find node in parent's child list")
Expand Down
6 changes: 4 additions & 2 deletions src/octree/node_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
TryRemoveChildrenError,
MakeBranchError,
traversal::algorithms,
util::ArrayMap,
util::{ArrayMap, abort_on_panic},
};

/// A reference to a node in an octree.
Expand Down Expand Up @@ -496,7 +496,9 @@ debug key check failed: tried to reference key {:?} which is not present in the
// SAFETY: as above
ptr::write(
&mut self.node_mut().value,
NodeData::Leaf( f(old_payload) ),
NodeData::Leaf(
abort_on_panic(|| f(old_payload))
),
);
}
Ok(children_payloads)
Expand Down
2 changes: 1 addition & 1 deletion src/quadtree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl<B, L> MoveFix for Node<B, L, usize> {
return;
}
}
unsafe {
/*unsafe*/ {
// SAFETY: this mismatch is assumed to never happen as a guarantee
// of key validity
unreachable_debugchecked("failed to find node in parent's child list")
Expand Down
6 changes: 4 additions & 2 deletions src/quadtree/node_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
TryRemoveChildrenError,
MakeBranchError,
traversal::algorithms,
util::ArrayMap,
util::{ArrayMap, abort_on_panic},
};

/// A reference to a node in a quadtree.
Expand Down Expand Up @@ -485,7 +485,9 @@ debug key check failed: tried to reference key {:?} which is not present in the
// SAFETY: as above
ptr::write(
&mut self.node_mut().value,
NodeData::Leaf( f(old_payload) ),
NodeData::Leaf(
abort_on_panic(|| f(old_payload))
),
);
}
Ok(children_payloads)
Expand Down
14 changes: 14 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,18 @@ pub unsafe fn unreachable_debugchecked(msg: &str) -> ! {
} else {
hint::unreachable_unchecked()
}
}

#[inline]
pub fn abort_on_panic<R>(f: impl FnOnce() -> R) -> R {
#[cfg(feature = "unwind_safety")]
{
std::panic::catch_unwind(
std::panic::AssertUnwindSafe(f)
).unwrap_or_else(|_| std::process::exit(101))
}
#[cfg(not(feature = "unwind_safety"))]
{
f()
}
}

0 comments on commit cf0906c

Please sign in to comment.