Skip to content

Commit

Permalink
Auto merge of #57045 - RalfJung:kill-more-uninit, r=SimonSapin
Browse files Browse the repository at this point in the history
Kill remaining uses of mem::uninitialized in libcore, liballoc

Kill remaining uses of mem::uninitialized in libcore and liballoc, and enable a lint that will warn when uses are added again in the future.

To avoid casting raw pointers around (which is always very dangerous because it is not typechecked at all -- it doesn't even get the "same size" sanity check that `transmute` gets), I also added two new functions to `MaybeUninit`:

```rust
    /// Get a pointer to the first contained values.
    pub fn first_ptr(this: &[MaybeUninit<T>]) -> *const T {
        this as *const [MaybeUninit<T>] as *const T
    }

    /// Get a mutable pointer to the first contained values.
    pub fn first_mut_ptr(this: &mut [MaybeUninit<T>]) -> *mut T {
        this as *mut [MaybeUninit<T>] as *mut T
    }
```

I changed some of the existing code to use array-of-`MaybeUninit` instead of `MaybeUninit`-of-array, successfully removing raw pointer casts there as well.
  • Loading branch information
bors committed Jan 28, 2019
2 parents d8a0dd7 + 489a792 commit 8424c01
Show file tree
Hide file tree
Showing 12 changed files with 85 additions and 47 deletions.
11 changes: 6 additions & 5 deletions src/etc/gdb_rust_pretty_printing.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,19 +330,20 @@ def children_of_node(boxed_node, height, want_values):
leaf = node_ptr['data']
else:
leaf = node_ptr.dereference()
keys = leaf['keys']['value']['value']
keys = leaf['keys']
if want_values:
values = leaf['vals']['value']['value']
values = leaf['vals']
length = int(leaf['len'])
for i in xrange(0, length + 1):
if height > 0:
for child in children_of_node(node_ptr['edges'][i], height - 1, want_values):
child_ptr = node_ptr['edges'][i]['value']['value']
for child in children_of_node(child_ptr, height - 1, want_values):
yield child
if i < length:
if want_values:
yield (keys[i], values[i])
yield (keys[i]['value']['value'], values[i]['value']['value'])
else:
yield keys[i]
yield keys[i]['value']['value']

class RustStdBTreeSetPrinter(object):
def __init__(self, val):
Expand Down
1 change: 0 additions & 1 deletion src/liballoc/borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,6 @@ impl<'a, B: ?Sized> Hash for Cow<'a, B>
}

#[stable(feature = "rust1", since = "1.0.0")]
#[allow(deprecated)]
impl<'a, T: ?Sized + ToOwned> AsRef<T> for Cow<'a, T> {
fn as_ref(&self) -> &T {
self
Expand Down
36 changes: 20 additions & 16 deletions src/liballoc/collections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ struct LeafNode<K, V> {

/// The arrays storing the actual data of the node. Only the first `len` elements of each
/// array are initialized and valid.
keys: MaybeUninit<[K; CAPACITY]>,
vals: MaybeUninit<[V; CAPACITY]>,
keys: [MaybeUninit<K>; CAPACITY],
vals: [MaybeUninit<V>; CAPACITY],
}

impl<K, V> LeafNode<K, V> {
Expand All @@ -106,8 +106,8 @@ impl<K, V> LeafNode<K, V> {
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: MaybeUninit::uninitialized(),
vals: MaybeUninit::uninitialized(),
keys: uninitialized_array![_; CAPACITY],
vals: uninitialized_array![_; CAPACITY],
parent: ptr::null(),
parent_idx: MaybeUninit::uninitialized(),
len: 0
Expand Down Expand Up @@ -145,7 +145,7 @@ struct InternalNode<K, V> {

/// The pointers to the children of this node. `len + 1` of these are considered
/// initialized and valid.
edges: [BoxedNode<K, V>; 2 * B],
edges: [MaybeUninit<BoxedNode<K, V>>; 2 * B],
}

impl<K, V> InternalNode<K, V> {
Expand All @@ -159,7 +159,7 @@ impl<K, V> InternalNode<K, V> {
unsafe fn new() -> Self {
InternalNode {
data: LeafNode::new(),
edges: mem::uninitialized()
edges: uninitialized_array![_; 2*B],
}
}
}
Expand Down Expand Up @@ -261,7 +261,7 @@ impl<K, V> Root<K, V> {
-> NodeRef<marker::Mut, K, V, marker::Internal> {
debug_assert!(!self.is_shared_root());
let mut new_node = Box::new(unsafe { InternalNode::new() });
new_node.edges[0] = unsafe { BoxedNode::from_ptr(self.node.as_ptr()) };
new_node.edges[0].set(unsafe { BoxedNode::from_ptr(self.node.as_ptr()) });

self.node = BoxedNode::from_internal(new_node);
self.height += 1;
Expand Down Expand Up @@ -623,7 +623,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Immut<'a>, K, V, Type> {
// We cannot be the root, so `as_leaf` is okay
unsafe {
slice::from_raw_parts(
self.as_leaf().vals.as_ptr() as *const V,
MaybeUninit::first_ptr(&self.as_leaf().vals),
self.len()
)
}
Expand All @@ -650,7 +650,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
} else {
unsafe {
slice::from_raw_parts_mut(
(*self.as_leaf_mut()).keys.as_mut_ptr() as *mut K,
MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys),
self.len()
)
}
Expand All @@ -661,7 +661,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
debug_assert!(!self.is_shared_root());
unsafe {
slice::from_raw_parts_mut(
(*self.as_leaf_mut()).vals.as_mut_ptr() as *mut V,
MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals),
self.len()
)
}
Expand Down Expand Up @@ -718,7 +718,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
unsafe {
ptr::write(self.keys_mut().get_unchecked_mut(idx), key);
ptr::write(self.vals_mut().get_unchecked_mut(idx), val);
ptr::write(self.as_internal_mut().edges.get_unchecked_mut(idx + 1), edge.node);
self.as_internal_mut().edges.get_unchecked_mut(idx + 1).set(edge.node);

(*self.as_leaf_mut()).len += 1;

Expand Down Expand Up @@ -749,7 +749,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
slice_insert(self.vals_mut(), 0, val);
slice_insert(
slice::from_raw_parts_mut(
self.as_internal_mut().edges.as_mut_ptr(),
MaybeUninit::first_ptr_mut(&mut self.as_internal_mut().edges),
self.len()+1
),
0,
Expand Down Expand Up @@ -778,7 +778,9 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
let edge = match self.reborrow_mut().force() {
ForceResult::Leaf(_) => None,
ForceResult::Internal(internal) => {
let edge = ptr::read(internal.as_internal().edges.get_unchecked(idx + 1));
let edge = ptr::read(
internal.as_internal().edges.get_unchecked(idx + 1).as_ptr()
);
let mut new_root = Root { node: edge, height: internal.height - 1 };
(*new_root.as_mut().as_leaf_mut()).parent = ptr::null();
Some(new_root)
Expand Down Expand Up @@ -806,7 +808,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
ForceResult::Internal(mut internal) => {
let edge = slice_remove(
slice::from_raw_parts_mut(
internal.as_internal_mut().edges.as_mut_ptr(),
MaybeUninit::first_ptr_mut(&mut internal.as_internal_mut().edges),
old_len+1
),
0
Expand Down Expand Up @@ -1085,7 +1087,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::

slice_insert(
slice::from_raw_parts_mut(
self.node.as_internal_mut().edges.as_mut_ptr(),
MaybeUninit::first_ptr_mut(&mut self.node.as_internal_mut().edges),
self.node.len()
),
self.idx + 1,
Expand Down Expand Up @@ -1140,7 +1142,9 @@ impl<BorrowType, K, V>
pub fn descend(self) -> NodeRef<BorrowType, K, V, marker::LeafOrInternal> {
NodeRef {
height: self.node.height - 1,
node: unsafe { self.node.as_internal().edges.get_unchecked(self.idx).as_ptr() },
node: unsafe {
self.node.as_internal().edges.get_unchecked(self.idx).get_ref().as_ptr()
},
root: self.node.root,
_marker: PhantomData
}
Expand Down
5 changes: 3 additions & 2 deletions src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@
#![no_std]
#![needs_allocator]

#![deny(intra_doc_link_resolution_failure)]
#![deny(missing_debug_implementations)]
#![warn(deprecated_in_future)]
#![warn(intra_doc_link_resolution_failure)]
#![warn(missing_debug_implementations)]

#![cfg_attr(not(test), feature(fn_traits))]
#![cfg_attr(not(test), feature(generator_trait))]
Expand Down
2 changes: 0 additions & 2 deletions src/liballoc/rc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(deprecated)]

//! Single-threaded reference-counting pointers. 'Rc' stands for 'Reference
//! Counted'.
//!
Expand Down
2 changes: 1 addition & 1 deletion src/libcore/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2048,7 +2048,7 @@ macro_rules! tuple {
( $($name:ident,)+ ) => (
#[stable(feature = "rust1", since = "1.0.0")]
impl<$($name:Debug),*> Debug for ($($name,)*) where last_type!($($name,)+): ?Sized {
#[allow(non_snake_case, unused_assignments, deprecated)]
#[allow(non_snake_case, unused_assignments)]
fn fmt(&self, f: &mut Formatter) -> Result {
let mut builder = f.debug_tuple("");
let ($(ref $name,)*) = *self;
Expand Down
20 changes: 11 additions & 9 deletions src/libcore/fmt/num.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
//! Integer and floating-point number formatting

#![allow(deprecated)]


use fmt;
use ops::{Div, Rem, Sub};
use str;
use slice;
use ptr;
use mem;
use mem::MaybeUninit;

#[doc(hidden)]
trait Int: PartialEq + PartialOrd + Div<Output=Self> + Rem<Output=Self> +
Expand Down Expand Up @@ -53,7 +51,7 @@ trait GenericRadix {
// characters for a base 2 number.
let zero = T::zero();
let is_nonnegative = x >= zero;
let mut buf: [u8; 128] = unsafe { mem::uninitialized() };
let mut buf = uninitialized_array![u8; 128];
let mut curr = buf.len();
let base = T::from_u8(Self::BASE);
if is_nonnegative {
Expand All @@ -62,7 +60,7 @@ trait GenericRadix {
for byte in buf.iter_mut().rev() {
let n = x % base; // Get the current place value.
x = x / base; // Deaccumulate the number.
*byte = Self::digit(n.to_u8()); // Store the digit in the buffer.
byte.set(Self::digit(n.to_u8())); // Store the digit in the buffer.
curr -= 1;
if x == zero {
// No more digits left to accumulate.
Expand All @@ -74,15 +72,19 @@ trait GenericRadix {
for byte in buf.iter_mut().rev() {
let n = zero - (x % base); // Get the current place value.
x = x / base; // Deaccumulate the number.
*byte = Self::digit(n.to_u8()); // Store the digit in the buffer.
byte.set(Self::digit(n.to_u8())); // Store the digit in the buffer.
curr -= 1;
if x == zero {
// No more digits left to accumulate.
break
};
}
}
let buf = unsafe { str::from_utf8_unchecked(&buf[curr..]) };
let buf = &buf[curr..];
let buf = unsafe { str::from_utf8_unchecked(slice::from_raw_parts(
MaybeUninit::first_ptr(buf),
buf.len()
)) };
f.pad_integral(is_nonnegative, Self::PREFIX, buf)
}
}
Expand Down Expand Up @@ -196,9 +198,9 @@ macro_rules! impl_Display {
// convert the negative num to positive by summing 1 to it's 2 complement
(!self.$conv_fn()).wrapping_add(1)
};
let mut buf: [u8; 39] = unsafe { mem::uninitialized() };
let mut buf = uninitialized_array![u8; 39];
let mut curr = buf.len() as isize;
let buf_ptr = buf.as_mut_ptr();
let buf_ptr = MaybeUninit::first_ptr_mut(&mut buf);
let lut_ptr = DEC_DIGITS_LUT.as_ptr();

unsafe {
Expand Down
2 changes: 1 addition & 1 deletion src/libcore/hash/sip.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! An implementation of SipHash.

#![allow(deprecated)]
#![allow(deprecated)] // the types in this module are deprecated

use marker::PhantomData;
use ptr;
Expand Down
10 changes: 6 additions & 4 deletions src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,12 @@
issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/",
test(no_crate_inject, attr(deny(warnings))),
test(attr(allow(dead_code, deprecated, unused_variables, unused_mut))))]

#![no_core]
#![deny(missing_docs)]
#![deny(intra_doc_link_resolution_failure)]
#![deny(missing_debug_implementations)]

#![warn(deprecated_in_future)]
#![warn(missing_docs)]
#![warn(intra_doc_link_resolution_failure)]
#![warn(missing_debug_implementations)]

#![feature(allow_internal_unstable)]
#![feature(arbitrary_self_types)]
Expand Down Expand Up @@ -122,6 +123,7 @@
#![feature(structural_match)]
#![feature(abi_unadjusted)]
#![feature(adx_target_feature)]
#![feature(maybe_uninit)]

#[prelude_import]
#[allow(unused)]
Expand Down
17 changes: 17 additions & 0 deletions src/libcore/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,23 @@ macro_rules! unimplemented {
($($arg:tt)+) => (panic!("not yet implemented: {}", format_args!($($arg)*)));
}

/// A macro to create an array of [`MaybeUninit`]
///
/// This macro constructs and uninitialized array of the type `[MaybeUninit<K>; N]`.
///
/// [`MaybeUninit`]: mem/union.MaybeUninit.html
#[macro_export]
#[unstable(feature = "maybe_uninit", issue = "53491")]
macro_rules! uninitialized_array {
// This `into_inner` is safe because an array of `MaybeUninit` does not
// require initialization.
// FIXME(#49147): Could be replaced by an array initializer, once those can
// be any const expression.
($t:ty; $size:expr) => (unsafe {
MaybeUninit::<[MaybeUninit<$t>; $size]>::uninitialized().into_inner()
});
}

/// Built-in macros to the compiler itself.
///
/// These macros do not have any corresponding definition with a `macro_rules!`
Expand Down
14 changes: 14 additions & 0 deletions src/libcore/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1148,4 +1148,18 @@ impl<T> MaybeUninit<T> {
pub fn as_mut_ptr(&mut self) -> *mut T {
unsafe { &mut *self.value as *mut T }
}

/// Get a pointer to the first element of the array.
#[unstable(feature = "maybe_uninit", issue = "53491")]
#[inline(always)]
pub fn first_ptr(this: &[MaybeUninit<T>]) -> *const T {
this as *const [MaybeUninit<T>] as *const T
}

/// Get a mutable pointer to the first element of the array.
#[unstable(feature = "maybe_uninit", issue = "53491")]
#[inline(always)]
pub fn first_ptr_mut(this: &mut [MaybeUninit<T>]) -> *mut T {
this as *mut [MaybeUninit<T>] as *mut T
}
}
12 changes: 6 additions & 6 deletions src/libcore/slice/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,14 @@ fn partition_in_blocks<T, F>(v: &mut [T], pivot: &T, is_less: &mut F) -> usize
let mut block_l = BLOCK;
let mut start_l = ptr::null_mut();
let mut end_l = ptr::null_mut();
let mut offsets_l = MaybeUninit::<[u8; BLOCK]>::uninitialized();
let mut offsets_l: [MaybeUninit<u8>; BLOCK] = uninitialized_array![u8; BLOCK];

// The current block on the right side (from `r.sub(block_r)` to `r`).
let mut r = unsafe { l.add(v.len()) };
let mut block_r = BLOCK;
let mut start_r = ptr::null_mut();
let mut end_r = ptr::null_mut();
let mut offsets_r = MaybeUninit::<[u8; BLOCK]>::uninitialized();
let mut offsets_r: [MaybeUninit<u8>; BLOCK] = uninitialized_array![u8; BLOCK];

// FIXME: When we get VLAs, try creating one array of length `min(v.len(), 2 * BLOCK)` rather
// than two fixed-size arrays of length `BLOCK`. VLAs might be more cache-efficient.
Expand Down Expand Up @@ -262,8 +262,8 @@ fn partition_in_blocks<T, F>(v: &mut [T], pivot: &T, is_less: &mut F) -> usize

if start_l == end_l {
// Trace `block_l` elements from the left side.
start_l = offsets_l.as_mut_ptr() as *mut u8;
end_l = offsets_l.as_mut_ptr() as *mut u8;
start_l = MaybeUninit::first_ptr_mut(&mut offsets_l);
end_l = MaybeUninit::first_ptr_mut(&mut offsets_l);
let mut elem = l;

for i in 0..block_l {
Expand All @@ -278,8 +278,8 @@ fn partition_in_blocks<T, F>(v: &mut [T], pivot: &T, is_less: &mut F) -> usize

if start_r == end_r {
// Trace `block_r` elements from the right side.
start_r = offsets_r.as_mut_ptr() as *mut u8;
end_r = offsets_r.as_mut_ptr() as *mut u8;
start_r = MaybeUninit::first_ptr_mut(&mut offsets_r);
end_r = MaybeUninit::first_ptr_mut(&mut offsets_r);
let mut elem = r;

for i in 0..block_r {
Expand Down

0 comments on commit 8424c01

Please sign in to comment.