From aeea992fa298a26ffb2384987e3ec3651e36b081 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sun, 19 Aug 2018 21:56:49 +0200 Subject: [PATCH 1/5] alloc: fix deprecated warnings --- src/liballoc/collections/btree/node.rs | 40 +++++++++++++------------- src/liballoc/lib.rs | 1 + 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 0315545262b6b..08bcfea980b60 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -42,7 +42,7 @@ // This implies that even an empty internal node has at least one edge. use core::marker::PhantomData; -use core::mem; +use core::mem::{self, MaybeUninit}; use core::ptr::{self, Unique, NonNull}; use core::slice; @@ -73,7 +73,7 @@ struct LeafNode { /// This node's index into the parent node's `edges` array. /// `*node.parent.edges[node.parent_idx]` should be the same thing as `node`. /// This is only guaranteed to be initialized when `parent` is nonnull. - parent_idx: u16, + parent_idx: MaybeUninit, /// The number of keys and values this node stores. /// @@ -83,8 +83,8 @@ struct LeafNode { /// The arrays storing the actual data of the node. Only the first `len` elements of each /// array are initialized and valid. - keys: [K; CAPACITY], - vals: [V; CAPACITY], + keys: MaybeUninit<[K; CAPACITY]>, + vals: MaybeUninit<[V; CAPACITY]>, } impl LeafNode { @@ -94,10 +94,10 @@ impl LeafNode { LeafNode { // As a general policy, we leave fields uninitialized if they can be, as this should // be both slightly faster and easier to track in Valgrind. - keys: mem::uninitialized(), - vals: mem::uninitialized(), + keys: MaybeUninit::uninitialized(), + vals: MaybeUninit::uninitialized(), parent: ptr::null(), - parent_idx: mem::uninitialized(), + parent_idx: MaybeUninit::uninitialized(), len: 0 } } @@ -115,10 +115,10 @@ unsafe impl Sync for LeafNode<(), ()> {} // ever take a pointer past the first key. static EMPTY_ROOT_NODE: LeafNode<(), ()> = LeafNode { parent: ptr::null(), - parent_idx: 0, + parent_idx: MaybeUninit::uninitialized(), len: 0, - keys: [(); CAPACITY], - vals: [(); CAPACITY], + keys: MaybeUninit::uninitialized(), + vals: MaybeUninit::uninitialized(), }; /// The underlying representation of internal nodes. As with `LeafNode`s, these should be hidden @@ -430,7 +430,7 @@ impl NodeRef { root: self.root, _marker: PhantomData }, - idx: self.as_leaf().parent_idx as usize, + idx: unsafe { usize::from(*self.as_leaf().parent_idx.get_ref()) }, _marker: PhantomData }) } else { @@ -567,7 +567,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { // the node, which is allowed by LLVM. unsafe { slice::from_raw_parts( - self.as_leaf().keys.as_ptr(), + self.as_leaf().keys.get_ref().as_ptr(), self.len() ) } @@ -578,7 +578,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { debug_assert!(!self.is_shared_root()); unsafe { slice::from_raw_parts( - self.as_leaf().vals.as_ptr(), + self.as_leaf().vals.get_ref().as_ptr(), self.len() ) } @@ -605,7 +605,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { } else { unsafe { slice::from_raw_parts_mut( - &mut self.as_leaf_mut().keys as *mut [K] as *mut K, + self.as_leaf_mut().keys.get_mut() as *mut [K] as *mut K, self.len() ) } @@ -616,7 +616,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { debug_assert!(!self.is_shared_root()); unsafe { slice::from_raw_parts_mut( - &mut self.as_leaf_mut().vals as *mut [V] as *mut V, + self.as_leaf_mut().vals.get_mut() as *mut [V] as *mut V, self.len() ) } @@ -1013,7 +1013,7 @@ impl<'a, K, V> Handle, K, V, marker::Internal>, marker:: let ptr = self.node.as_internal_mut() as *mut _; let mut child = self.descend(); child.as_leaf_mut().parent = ptr; - child.as_leaf_mut().parent_idx = idx; + child.as_leaf_mut().parent_idx.set(idx); } /// Unsafely asserts to the compiler some static information about whether the underlying @@ -1152,12 +1152,12 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::KV> ptr::copy_nonoverlapping( self.node.keys().as_ptr().add(self.idx + 1), - new_node.keys.as_mut_ptr(), + new_node.keys.get_mut().as_mut_ptr(), new_len ); ptr::copy_nonoverlapping( self.node.vals().as_ptr().add(self.idx + 1), - new_node.vals.as_mut_ptr(), + new_node.vals.get_mut().as_mut_ptr(), new_len ); @@ -1210,12 +1210,12 @@ impl<'a, K, V> Handle, K, V, marker::Internal>, marker:: ptr::copy_nonoverlapping( self.node.keys().as_ptr().add(self.idx + 1), - new_node.data.keys.as_mut_ptr(), + new_node.data.keys.get_mut().as_mut_ptr(), new_len ); ptr::copy_nonoverlapping( self.node.vals().as_ptr().add(self.idx + 1), - new_node.data.vals.as_mut_ptr(), + new_node.data.vals.get_mut().as_mut_ptr(), new_len ); ptr::copy_nonoverlapping( diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 78d1958b8fb37..987572e6b74a9 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -119,6 +119,7 @@ #![feature(rustc_const_unstable)] #![feature(const_vec_new)] #![feature(slice_partition_dedup)] +#![feature(maybe_uninit)] // Allow testing this library From 6644e18e30b69bbbc98526fcaad9acd4c278e216 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 23 Aug 2018 16:52:05 +0200 Subject: [PATCH 2/5] address RalfJung's comment --- src/liballoc/collections/btree/node.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 08bcfea980b60..c86278fc5ddaa 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -567,7 +567,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { // the node, which is allowed by LLVM. unsafe { slice::from_raw_parts( - self.as_leaf().keys.get_ref().as_ptr(), + self.as_leaf().keys.as_ptr() as *const K, self.len() ) } @@ -578,7 +578,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { debug_assert!(!self.is_shared_root()); unsafe { slice::from_raw_parts( - self.as_leaf().vals.get_ref().as_ptr(), + self.as_leaf().vals.as_ptr() as *const V, self.len() ) } @@ -1152,12 +1152,12 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::KV> ptr::copy_nonoverlapping( self.node.keys().as_ptr().add(self.idx + 1), - new_node.keys.get_mut().as_mut_ptr(), + new_node.keys.as_mut_ptr() as *mut K, new_len ); ptr::copy_nonoverlapping( self.node.vals().as_ptr().add(self.idx + 1), - new_node.vals.get_mut().as_mut_ptr(), + new_node.vals.as_mut_ptr() as *mut V, new_len ); @@ -1210,12 +1210,12 @@ impl<'a, K, V> Handle, K, V, marker::Internal>, marker:: ptr::copy_nonoverlapping( self.node.keys().as_ptr().add(self.idx + 1), - new_node.data.keys.get_mut().as_mut_ptr(), + new_node.data.keys.as_mut_ptr() as *mut K, new_len ); ptr::copy_nonoverlapping( self.node.vals().as_ptr().add(self.idx + 1), - new_node.data.vals.get_mut().as_mut_ptr(), + new_node.data.vals.as_mut_ptr() as *mut V, new_len ); ptr::copy_nonoverlapping( From 4d8310cef2e1ae49f2b0b1faf05d84b72b907d31 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 1 Sep 2018 19:58:04 +0200 Subject: [PATCH 3/5] gdb_rust_pretty_printing: adapt to the changes in the layout of btree::LeafNode --- src/etc/gdb_rust_pretty_printing.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/etc/gdb_rust_pretty_printing.py b/src/etc/gdb_rust_pretty_printing.py index 216915dba5fe7..686c2c447f6ce 100755 --- a/src/etc/gdb_rust_pretty_printing.py +++ b/src/etc/gdb_rust_pretty_printing.py @@ -322,8 +322,10 @@ def to_string(self): def children(self): (length, data_ptr) = \ rustpp.extract_length_and_ptr_from_std_btreeset(self.__val) - val = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(3) - gdb_ptr = val.get_wrapped_value() + maybe_uninit_keys = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(3) + manually_drop_keys = maybe_uninit_keys.get_child_at_index(1) + keys = manually_drop_keys.get_child_at_index(0) + gdb_ptr = keys.get_wrapped_value() for index in xrange(length): yield (str(index), gdb_ptr[index]) @@ -345,9 +347,13 @@ def to_string(self): def children(self): (length, data_ptr) = \ rustpp.extract_length_and_ptr_from_std_btreemap(self.__val) - keys = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(3) + maybe_uninit_keys = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(3) + manually_drop_keys = maybe_uninit_keys.get_child_at_index(1) + keys = manually_drop_keys.get_child_at_index(0) keys_ptr = keys.get_wrapped_value() - vals = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(4) + maybe_uninit_vals = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(4) + manually_drop_vals = maybe_uninit_vals.get_child_at_index(1) + vals = manually_drop_vals.get_child_at_index(0) vals_ptr = vals.get_wrapped_value() for index in xrange(length): yield (str(index), keys_ptr[index]) From faa733dd18e5bf5b24cba387703b423489b8b09c Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 1 Sep 2018 21:44:35 +0200 Subject: [PATCH 4/5] fix tidy --- src/etc/gdb_rust_pretty_printing.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/etc/gdb_rust_pretty_printing.py b/src/etc/gdb_rust_pretty_printing.py index 686c2c447f6ce..b1252f386df36 100755 --- a/src/etc/gdb_rust_pretty_printing.py +++ b/src/etc/gdb_rust_pretty_printing.py @@ -322,7 +322,8 @@ def to_string(self): def children(self): (length, data_ptr) = \ rustpp.extract_length_and_ptr_from_std_btreeset(self.__val) - maybe_uninit_keys = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(3) + leaf_node = GdbValue(data_ptr.get_wrapped_value().dereference()) + maybe_uninit_keys = leaf_node.get_child_at_index(3) manually_drop_keys = maybe_uninit_keys.get_child_at_index(1) keys = manually_drop_keys.get_child_at_index(0) gdb_ptr = keys.get_wrapped_value() @@ -347,11 +348,12 @@ def to_string(self): def children(self): (length, data_ptr) = \ rustpp.extract_length_and_ptr_from_std_btreemap(self.__val) - maybe_uninit_keys = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(3) + leaf_node = GdbValue(data_ptr.get_wrapped_value().dereference()) + maybe_uninit_keys = leaf_node.get_child_at_index(3) manually_drop_keys = maybe_uninit_keys.get_child_at_index(1) keys = manually_drop_keys.get_child_at_index(0) keys_ptr = keys.get_wrapped_value() - maybe_uninit_vals = GdbValue(data_ptr.get_wrapped_value().dereference()).get_child_at_index(4) + maybe_uninit_vals = leaf_node.get_child_at_index(4) manually_drop_vals = maybe_uninit_vals.get_child_at_index(1) vals = manually_drop_vals.get_child_at_index(0) vals_ptr = vals.get_wrapped_value() From e4434be6b7caa1261fa1500d321c20a59f9953b1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 10 Oct 2018 09:16:05 +0200 Subject: [PATCH 5/5] remove a now outdated comment --- src/liballoc/collections/btree/node.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index c86278fc5ddaa..deca9591fbd5c 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -58,9 +58,6 @@ pub const CAPACITY: usize = 2 * B - 1; /// these should always be put behind pointers, and specifically behind `BoxedNode` in the owned /// case. /// -/// See also rust-lang/rfcs#197, which would make this structure significantly more safe by -/// avoiding accidentally dropping unused and uninitialized keys and values. -/// /// We put the metadata first so that its position is the same for every `K` and `V`, in order /// to statically allocate a single dummy node to avoid allocations. This struct is `repr(C)` to /// prevent them from being reordered.