diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 6383fea862425..f1064e702c5e5 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -575,7 +575,7 @@ impl Box { #[inline(always)] pub const fn pin_in(x: T, alloc: A) -> Pin where - A: 'static + ~const Allocator + ~const Destruct, + A: ~const Allocator + ~const PinSafeAllocator + ~const Destruct, { Self::into_pin(Self::new_in(x, alloc)) } diff --git a/library/alloc/src/collections/btree/append.rs b/library/alloc/src/collections/btree/append.rs index b6989afb6255d..c3230add01137 100644 --- a/library/alloc/src/collections/btree/append.rs +++ b/library/alloc/src/collections/btree/append.rs @@ -15,7 +15,11 @@ impl Root { /// a `BTreeMap`, both iterators should produce keys in strictly ascending /// order, each greater than all keys in the tree, including any keys /// already in the tree upon entry. - pub fn append_from_sorted_iters( + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + pub unsafe fn append_from_sorted_iters( &mut self, left: I, right: I, @@ -29,14 +33,25 @@ impl Root { let iter = MergeIter(MergeIterInner::new(left, right)); // Meanwhile, we build a tree from the sorted sequence in linear time. - self.bulk_push(iter, length, alloc) + + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + unsafe { self.bulk_push(iter, length, alloc) } } /// Pushes all key-value pairs to the end of the tree, incrementing a /// `length` variable along the way. The latter makes it easier for the /// caller to avoid a leak when the iterator panicks. - pub fn bulk_push(&mut self, iter: I, length: &mut usize, alloc: A) - where + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + pub unsafe fn bulk_push( + &mut self, + iter: I, + length: &mut usize, + alloc: A, + ) where I: Iterator, { let mut cur_node = self.borrow_mut().last_leaf_edge().into_node(); @@ -64,7 +79,10 @@ impl Root { } Err(_) => { // We are at the top, create a new root node and push there. - open_node = self.push_internal_level(alloc.clone()); + + // SAFETY: The caller has guaranteed that `alloc` is the allocator for + // the owning `BTreeMap`. + open_node = unsafe { self.push_internal_level(alloc.clone()) }; break; } } @@ -72,9 +90,13 @@ impl Root { // Push key-value pair and new right subtree. let tree_height = open_node.height() - 1; - let mut right_tree = Root::new(alloc.clone()); + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + let mut right_tree = unsafe { Root::new(alloc.clone()) }; for _ in 0..tree_height { - right_tree.push_internal_level(alloc.clone()); + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the + // owning `BTreeMap`. + unsafe { right_tree.push_internal_level(alloc.clone()) }; } open_node.push(key, value, right_tree); diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index cacbd54b6c246..ca8108f3d7562 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -216,7 +216,9 @@ impl Clone for BTreeMap { match node.force() { Leaf(leaf) => { let mut out_tree = BTreeMap { - root: Some(Root::new(alloc.clone())), + // SAFETY: `alloc` is the allocator for both the original and the cloned + // `BTreeMap`. + root: unsafe { Some(Root::new(alloc.clone())) }, length: 0, alloc: ManuallyDrop::new(alloc), _marker: PhantomData, @@ -247,7 +249,9 @@ impl Clone for BTreeMap { { let out_root = out_tree.root.as_mut().unwrap(); - let mut out_node = out_root.push_internal_level(alloc.clone()); + // SAFETY: `alloc` is the allocator for both the original and the cloned + // `BTreeMap`. + let mut out_node = unsafe { out_root.push_internal_level(alloc.clone()) }; let mut in_edge = internal.first_edge(); while let Ok(kv) = in_edge.right_kv() { let (k, v) = kv.into_kv(); @@ -269,7 +273,9 @@ impl Clone for BTreeMap { out_node.push( k, v, - subroot.unwrap_or_else(|| Root::new(alloc.clone())), + // SAFETY: `alloc` is the allocator for both the original and cloned + // `BTreeMap`. + subroot.unwrap_or_else(|| unsafe { Root::new(alloc.clone()) }), ); out_tree.length += 1 + sublength; } @@ -323,8 +329,9 @@ where fn replace(&mut self, key: K) -> Option { let (map, dormant_map) = DormantMutRef::new(self); + // SAFETY: `alloc` is the allocator for the `BTreeMap`. let root_node = - map.root.get_or_insert_with(|| Root::new((*map.alloc).clone())).borrow_mut(); + map.root.get_or_insert_with(|| unsafe { Root::new((*map.alloc).clone()) }).borrow_mut(); match root_node.search_tree::(&key) { Found(mut kv) => Some(mem::replace(kv.key_mut(), key)), GoDown(handle) => { @@ -1144,13 +1151,16 @@ impl BTreeMap { let self_iter = mem::replace(self, Self::new_in((*self.alloc).clone())).into_iter(); let other_iter = mem::replace(other, Self::new_in((*self.alloc).clone())).into_iter(); - let root = self.root.get_or_insert_with(|| Root::new((*self.alloc).clone())); - root.append_from_sorted_iters( - self_iter, - other_iter, - &mut self.length, - (*self.alloc).clone(), - ) + let root = self.root.get_or_insert_with(|| unsafe { Root::new((*self.alloc).clone()) }); + // SAFETY: `self.alloc` is the allocator for the `BTreeMap`. + unsafe { + root.append_from_sorted_iters( + self_iter, + other_iter, + &mut self.length, + (*self.alloc).clone(), + ) + } } /// Constructs a double-ended iterator over a sub-range of elements in the map. @@ -1464,9 +1474,13 @@ impl BTreeMap { K: Ord, I: IntoIterator, { - let mut root = Root::new(alloc.clone()); + // SAFETY: `alloc` is the allocator for the returned `BTreeMap`. + let mut root = unsafe { Root::new(alloc.clone()) }; let mut length = 0; - root.bulk_push(DedupSortedIter::new(iter.into_iter()), &mut length, alloc.clone()); + // SAFETY: `alloc` is the allocator for the returned `BTreeMap`. + unsafe { + root.bulk_push(DedupSortedIter::new(iter.into_iter()), &mut length, alloc.clone()); + } BTreeMap { root: Some(root), length, alloc: ManuallyDrop::new(alloc), _marker: PhantomData } } } diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index b6eecf9b0e952..f661095d378b9 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -342,30 +342,39 @@ impl<'a, K: Ord, V, A: Allocator + Clone> VacantEntry<'a, K, V, A> { None => { // SAFETY: There is no tree yet so no reference to it exists. let map = unsafe { self.dormant_map.awaken() }; - let mut root = NodeRef::new_leaf(self.alloc.clone()); + // SAFETY: `self.alloc` is the allocator for the owning `BTreeMap`. + let mut root = unsafe { NodeRef::new_leaf(self.alloc.clone()) }; let val_ptr = root.borrow_mut().push(self.key, value) as *mut V; map.root = Some(root.forget_type()); map.length = 1; val_ptr } - Some(handle) => match handle.insert_recursing(self.key, value, self.alloc.clone()) { - (None, val_ptr) => { - // SAFETY: We have consumed self.handle. - let map = unsafe { self.dormant_map.awaken() }; - map.length += 1; - val_ptr + Some(handle) => { + // SAFETY: `self.alloc` is the allocator for the owning `BTreeMap`. + let insert_result = + unsafe { handle.insert_recursing(self.key, value, self.alloc.clone()) }; + match insert_result { + (None, val_ptr) => { + // SAFETY: We have consumed self.handle. + let map = unsafe { self.dormant_map.awaken() }; + map.length += 1; + val_ptr + } + (Some(ins), val_ptr) => { + drop(ins.left); + // SAFETY: We have consumed self.handle and dropped the + // remaining reference to the tree, ins.left. + let map = unsafe { self.dormant_map.awaken() }; + let root = map.root.as_mut().unwrap(); // same as ins.left + // SAFETY: `self.alloc` is the allocator for the owning `BTreeMap`. + unsafe { + root.push_internal_level(self.alloc).push(ins.kv.0, ins.kv.1, ins.right) + }; + map.length += 1; + val_ptr + } } - (Some(ins), val_ptr) => { - drop(ins.left); - // SAFETY: We have consumed self.handle and dropped the - // remaining reference to the tree, ins.left. - let map = unsafe { self.dormant_map.awaken() }; - let root = map.root.as_mut().unwrap(); // same as ins.left - root.push_internal_level(self.alloc).push(ins.kv.0, ins.kv.1, ins.right); - map.length += 1; - val_ptr - } - }, + } }; // Now that we have finished growing the tree using borrowed references, // dereference the pointer to a part of it, that we picked up along the way. diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index 4c372b1d60ac4..7636268c0380d 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -116,7 +116,14 @@ impl BTreeMap { { let iter = mem::take(self).into_iter(); if !iter.is_empty() { - self.root.insert(Root::new(*self.alloc)).bulk_push(iter, &mut self.length, *self.alloc); + // SAFETY: `self.alloc` is the allocator for this `BTreeMap`. + unsafe { + self.root.insert(Root::new(*self.alloc)).bulk_push( + iter, + &mut self.length, + *self.alloc, + ); + } } } } diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index d831161bcb686..217718ab525d4 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -213,31 +213,61 @@ unsafe impl Send for NodeRef unsafe impl Send for NodeRef {} impl NodeRef { - pub fn new_leaf(alloc: A) -> Self { - Self::from_new_leaf(LeafNode::new(alloc)) + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + pub unsafe fn new_leaf(alloc: A) -> Self { + // SAFETY: The caller has guaranteed that the allocator of the provided `Box` is the + // allocator for the owning `BTreeMap`. + unsafe { Self::from_new_leaf(LeafNode::new(alloc)) } } - fn from_new_leaf(leaf: Box, A>) -> Self { - NodeRef { height: 0, node: NonNull::from(Box::leak(leaf)), _marker: PhantomData } + /// # Safety + /// + /// The allocator of the `Box` must be the allocator for the owning `BTreeMap`. + unsafe fn from_new_leaf(leaf: Box, A>) -> Self { + // We're dropping the `alloc` part of the box here, but our safety condition guarantees that + // a clone of that allocator will outlive the returned `NodeRef` in the owning `BTreeMap`. + // This prevents the memory of the box from being invalidated. + let ptr = Box::into_raw(leaf); + // SAFETY: The pointer returned from `Box::into_raw` is guaranteed to be non-null. + let node = unsafe { NonNull::new_unchecked(ptr) }; + NodeRef { height: 0, node, _marker: PhantomData } } } impl NodeRef { - fn new_internal(child: Root, alloc: A) -> Self { + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + unsafe fn new_internal(child: Root, alloc: A) -> Self { let mut new_node = unsafe { InternalNode::new(alloc) }; new_node.edges[0].write(child.node); + + // SAFETY: + // - `child.height + 1` is always nonzero. + // - The caller has guaranteed that the allocator of the provided `Box` is the allocator for + // the owning `BTreeMap`. unsafe { NodeRef::from_new_internal(new_node, child.height + 1) } } /// # Safety - /// `height` must not be zero. + /// + /// - `height` must not be zero. + /// - The allocator of the `Box` must be the allocator for the owning `BTreeMap`. unsafe fn from_new_internal( internal: Box, A>, height: usize, ) -> Self { debug_assert!(height > 0); - let node = NonNull::from(Box::leak(internal)).cast(); - let mut this = NodeRef { height, node, _marker: PhantomData }; + // We're dropping the `alloc` part of the box here, but our safety condition guarantees that + // a clone of that allocator will outlive the returned `NodeRef` in the owning `BTreeMap`. + // This prevents the memory of the box from being invalidated. + let ptr = Box::into_raw(internal); + // SAFETY: The pointer returned from `Box::into_raw` is guaranteed to be non-null. + let node = unsafe { NonNull::new_unchecked(ptr) }; + + let mut this = NodeRef { height, node: node.cast(), _marker: PhantomData }; this.borrow_mut().correct_all_childrens_parent_links(); this } @@ -559,18 +589,32 @@ impl NodeRef { impl NodeRef { /// Returns a new owned tree, with its own root node that is initially empty. - pub fn new(alloc: A) -> Self { - NodeRef::new_leaf(alloc).forget_type() + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + pub unsafe fn new(alloc: A) -> Self { + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + unsafe { NodeRef::new_leaf(alloc).forget_type() } } /// Adds a new internal node with a single edge pointing to the previous root node, /// make that new node the root node, and return it. This increases the height by 1 /// and is the opposite of `pop_internal_level`. - pub fn push_internal_level( + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + pub unsafe fn push_internal_level( &mut self, alloc: A, ) -> NodeRef, K, V, marker::Internal> { - super::mem::take_mut(self, |old_root| NodeRef::new_internal(old_root, alloc).forget_type()); + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + super::mem::take_mut(self, |old_root| unsafe { + NodeRef::new_internal(old_root, alloc).forget_type() + }); // `self.borrow_mut()`, except that we just forgot we're internal now: NodeRef { height: self.height, node: self.node, _marker: PhantomData } @@ -869,7 +913,11 @@ impl<'a, K: 'a, V: 'a> Handle, K, V, marker::Leaf>, mark /// this edge. This method splits the node if there isn't enough room. /// /// The returned pointer points to the inserted value. - fn insert( + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + unsafe fn insert( mut self, key: K, val: V, @@ -881,7 +929,9 @@ impl<'a, K: 'a, V: 'a> Handle, K, V, marker::Leaf>, mark } else { let (middle_kv_idx, insertion) = splitpoint(self.idx); let middle = unsafe { Handle::new_kv(self.node, middle_kv_idx) }; - let mut result = middle.split(alloc); + // SAFETY: The caller has guaranteed that `alloc` is the allocator of the owning + // `BTreeMap`. + let mut result = unsafe { middle.split(alloc) }; let mut insertion_edge = match insertion { LeftOrRight::Left(insert_idx) => unsafe { Handle::new_edge(result.left.reborrow_mut(), insert_idx) @@ -968,13 +1018,19 @@ impl<'a, K: 'a, V: 'a> Handle, K, V, marker::Leaf>, mark /// If the returned result is some `SplitResult`, the `left` field will be the root node. /// The returned pointer points to the inserted value, which in the case of `SplitResult` /// is in the `left` or `right` tree. - pub fn insert_recursing( + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + pub unsafe fn insert_recursing( self, key: K, value: V, alloc: A, ) -> (Option>, *mut V) { - let (mut split, val_ptr) = match self.insert(key, value, alloc.clone()) { + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + let (mut split, val_ptr) = match unsafe { self.insert(key, value, alloc.clone()) } { (None, val_ptr) => return (None, val_ptr), (Some(split), val_ptr) => (split.forget_node_type(), val_ptr), }; @@ -1128,12 +1184,21 @@ impl<'a, K: 'a, V: 'a> Handle, K, V, marker::Leaf>, mark /// - The key and value pointed to by this handle are extracted. /// - All the key-value pairs to the right of this handle are put into a newly /// allocated node. - pub fn split(mut self, alloc: A) -> SplitResult<'a, K, V, marker::Leaf> { + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. + pub unsafe fn split( + mut self, + alloc: A, + ) -> SplitResult<'a, K, V, marker::Leaf> { let mut new_node = LeafNode::new(alloc); let kv = self.split_leaf_data(&mut new_node); - let right = NodeRef::from_new_leaf(new_node); + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + let right = unsafe { NodeRef::from_new_leaf(new_node) }; SplitResult { left: self.node, kv, right } } diff --git a/library/alloc/src/collections/btree/node/tests.rs b/library/alloc/src/collections/btree/node/tests.rs index aadb0dc9c40d9..e79bc77359c1e 100644 --- a/library/alloc/src/collections/btree/node/tests.rs +++ b/library/alloc/src/collections/btree/node/tests.rs @@ -68,10 +68,13 @@ fn test_splitpoint() { #[test] fn test_partial_eq() { - let mut root1 = NodeRef::new_leaf(Global); + // SAFETY: `Global` is the allocator for the `BTreeMap` we're testing. + let mut root1 = unsafe { NodeRef::new_leaf(Global) }; root1.borrow_mut().push(1, ()); - let mut root1 = NodeRef::new_internal(root1.forget_type(), Global).forget_type(); - let root2 = Root::new(Global); + // SAFETY: `Global` is the allocator for the `BTreeMap` we're testing. + let mut root1 = unsafe { NodeRef::new_internal(root1.forget_type(), Global).forget_type() }; + // SAFETY: `Global` is the allocator for the `BTreeMap` we're testing. + let root2 = unsafe { Root::new(Global) }; root1.reborrow().assert_back_pointers(); root2.reborrow().assert_back_pointers(); diff --git a/library/alloc/src/collections/btree/split.rs b/library/alloc/src/collections/btree/split.rs index 638dc98fc3e41..dd9f29521b745 100644 --- a/library/alloc/src/collections/btree/split.rs +++ b/library/alloc/src/collections/btree/split.rs @@ -63,10 +63,20 @@ impl Root { } /// Creates a tree consisting of empty nodes. + /// + /// # Safety + /// + /// `alloc` must be the allocator for the owning `BTreeMap`. fn new_pillar(height: usize, alloc: A) -> Self { - let mut root = Root::new(alloc.clone()); + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + let mut root = unsafe { Root::new(alloc.clone()) }; for _ in 0..height { - root.push_internal_level(alloc.clone()); + // SAFETY: The caller has guaranteed that `alloc` is the allocator for the owning + // `BTreeMap`. + unsafe { + root.push_internal_level(alloc.clone()); + } } root } diff --git a/src/test/ui/box/leak-alloc.stderr b/src/test/ui/box/leak-alloc.stderr index dc2a0ea23426d..83bf0d60cc185 100644 --- a/src/test/ui/box/leak-alloc.stderr +++ b/src/test/ui/box/leak-alloc.stderr @@ -2,10 +2,10 @@ error[E0597]: `alloc` does not live long enough --> $DIR/leak-alloc.rs:24:33 | LL | let boxed = Box::new_in(10, alloc.by_ref()); - | ^^^^^^^^^^^^^^ - | | - | borrowed value does not live long enough - | argument requires that `alloc` is borrowed for `'static` + | ----------------^^^^^^^^^^^^^^- + | | | + | | borrowed value does not live long enough + | argument requires that `alloc` is borrowed for `'static` ... LL | } | - `alloc` dropped here while still borrowed @@ -14,10 +14,10 @@ error[E0505]: cannot move out of `alloc` because it is borrowed --> $DIR/leak-alloc.rs:27:10 | LL | let boxed = Box::new_in(10, alloc.by_ref()); - | -------------- - | | - | borrow of `alloc` occurs here - | argument requires that `alloc` is borrowed for `'static` + | ------------------------------- + | | | + | | borrow of `alloc` occurs here + | argument requires that `alloc` is borrowed for `'static` ... LL | drop(alloc); | ^^^^^ move out of `alloc` occurs here diff --git a/src/test/ui/box/pin-safe-alloc.rs b/src/test/ui/box/pin-safe-alloc.rs index 699be28df2abc..91fdc799ddae0 100644 --- a/src/test/ui/box/pin-safe-alloc.rs +++ b/src/test/ui/box/pin-safe-alloc.rs @@ -1,6 +1,5 @@ // run-pass #![feature(allocator_api)] -#![feature(box_into_pin)] use std::alloc::{AllocError, Allocator, Layout, PinSafeAllocator, System}; use std::ptr::NonNull; diff --git a/src/test/ui/box/pin-unsafe-alloc.rs b/src/test/ui/box/pin-unsafe-alloc.rs index 49485f4763443..ffccf474c7c2c 100644 --- a/src/test/ui/box/pin-unsafe-alloc.rs +++ b/src/test/ui/box/pin-unsafe-alloc.rs @@ -1,5 +1,4 @@ #![feature(allocator_api)] -#![feature(box_into_pin)] use std::alloc::{AllocError, Allocator, Layout, System}; use std::ptr::NonNull; diff --git a/src/test/ui/box/pin-unsafe-alloc.stderr b/src/test/ui/box/pin-unsafe-alloc.stderr index 09996df418975..40ffb18819a8c 100644 --- a/src/test/ui/box/pin-unsafe-alloc.stderr +++ b/src/test/ui/box/pin-unsafe-alloc.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `Alloc: PinSafeAllocator` is not satisfied - --> $DIR/pin-unsafe-alloc.rs:32:32 + --> $DIR/pin-unsafe-alloc.rs:31:32 | LL | let _ = Box::pin_in(value, alloc); | ----------- ^^^^^ expected an implementor of trait `PinSafeAllocator` @@ -9,7 +9,7 @@ LL | let _ = Box::pin_in(value, alloc); note: required by a bound in `Box::::pin_in` --> $SRC_DIR/alloc/src/boxed.rs:LL:COL | -LL | A: ~const Allocator + ~const PinSafeAllocator + ~const Drop + ~const Destruct, +LL | A: ~const Allocator + ~const PinSafeAllocator + ~const Destruct, | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Box::::pin_in` help: consider borrowing here |