Skip to content

Commit

Permalink
Deprecate Read::initializer in favor of ptr::freeze
Browse files Browse the repository at this point in the history
Read implementations should only write into the buffer passed to them,
but have the ability to read from it. Access of uninitialized memory can
easily cause UB, so there's then a question of what a user of a reader
should do to initialize buffers.

Previously, we allowed a Read implementation to promise it wouldn't look
at the contents of the buffer, which allows the user to pass
uninitialized memory to it.

Instead, this PR adds a method to "freeze" undefined bytes into
arbitrary-but-defined bytes. This is currently done via an inline
assembly directive noting the address as an output, so LLVM no longer
knows it's uninitialized. There is a proposed "freeze" operation in LLVM
itself that would do this directly, but it hasn't been fully
implemented.

Some targets don't support inline assembly, so there we instead pass the
pointer to an extern "C" function, which is similarly opaque to LLVM.

The current approach is very low level. If we stabilize, we'll probably
want to add something like `slice.freeze()` to make this easier to use.
  • Loading branch information
sfackler committed Feb 10, 2019
1 parent c3d2490 commit 5e0fb23
Show file tree
Hide file tree
Showing 14 changed files with 128 additions and 15 deletions.
50 changes: 50 additions & 0 deletions src/libcore/ptr.rs
Expand Up @@ -946,6 +946,56 @@ pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
intrinsics::volatile_store(dst, src);
}

/// Freezes `count * size_of::<T>()` bytes of memory, converting undefined data into
/// defined-but-arbitrary data.
///
/// Uninitialized memory has undefined contents, and interation with that data
/// can easily cause undefined behavior. This function "freezes" memory
/// contents, converting uninitialized memory to initialized memory with
/// arbitrary conents so that use of it is well defined.
///
/// This function has no runtime effect; it is purely an instruction to the
/// compiler. In particular, it does not actually write anything to the memory.
///
/// # Safety
///
/// Behavior is undefined if any of the following conditions are violated:
///
/// * `dst` must be [valid] for reads.
///
/// Note that even if `T` has size `0`, the pointer must be non-NULL and properly aligned.
///
/// [valid]: ../ptr/index.html#safety
///
/// # Examples
///
/// ```ignore (io-is-in-std)
/// use std::io::{self, Read};
/// use std::mem;
/// use std::ptr;
///
/// pub fn read_le_u32<R>(reader: &mut R) -> io::Result<u32>
/// where
/// R: Read,
/// {
/// unsafe {
/// // We're passing this buffer to an arbitrary reader and aren't
/// // guaranteed they won't read from it, so freeze to avoid UB.
/// let mut buf: [u8; 4] = mem::uninitialized();
/// ptr::freeze(&mut buf, 1);
/// reader.read_exact(&mut buf)?;
///
/// Ok(u32::from_le_bytes(buf))
/// }
/// }
/// ```
#[inline]
#[unstable(feature = "ptr_freeze", issue = "0")]
pub unsafe fn freeze<T>(dst: *mut T, count: usize) {
let _ = count;
asm!("" : "=*m"(dst) : );
}

#[lang = "const_ptr"]
impl<T: ?Sized> *const T {
/// Returns `true` if the pointer is null.
Expand Down
6 changes: 5 additions & 1 deletion src/libstd/fs.rs
Expand Up @@ -9,7 +9,9 @@

use fmt;
use ffi::OsString;
use io::{self, SeekFrom, Seek, Read, Initializer, Write};
use io::{self, SeekFrom, Seek, Read, Write};
#[allow(deprecated)]
use io::Initializer;
use path::{Path, PathBuf};
use sys::fs as fs_imp;
use sys_common::{AsInnerMut, FromInner, AsInner, IntoInner};
Expand Down Expand Up @@ -600,6 +602,7 @@ impl Read for File {
}

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand All @@ -624,6 +627,7 @@ impl<'a> Read for &'a File {
}

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down
8 changes: 6 additions & 2 deletions src/libstd/io/buffered.rs
Expand Up @@ -5,8 +5,11 @@ use io::prelude::*;
use cmp;
use error;
use fmt;
use io::{self, Initializer, DEFAULT_BUF_SIZE, Error, ErrorKind, SeekFrom};
use io::{self, DEFAULT_BUF_SIZE, Error, ErrorKind, SeekFrom};
#[allow(deprecated)]
use io::Initializer;
use memchr;
use ptr;

/// The `BufReader` struct adds buffering to any reader.
///
Expand Down Expand Up @@ -92,7 +95,7 @@ impl<R: Read> BufReader<R> {
unsafe {
let mut buffer = Vec::with_capacity(cap);
buffer.set_len(cap);
inner.initializer().initialize(&mut buffer);
ptr::freeze(buffer.as_mut_ptr(), cap);
BufReader {
inner,
buf: buffer.into_boxed_slice(),
Expand Down Expand Up @@ -236,6 +239,7 @@ impl<R: Read> Read for BufReader<R> {
}

// we can't skip unconditionally because of the large buffer case in read.
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
self.inner.initializer()
}
Expand Down
5 changes: 4 additions & 1 deletion src/libstd/io/cursor.rs
Expand Up @@ -2,7 +2,9 @@ use io::prelude::*;

use core::convert::TryInto;
use cmp;
use io::{self, Initializer, SeekFrom, Error, ErrorKind};
use io::{self, SeekFrom, Error, ErrorKind};
#[allow(deprecated)]
use io::Initializer;

/// A `Cursor` wraps an in-memory buffer and provides it with a
/// [`Seek`] implementation.
Expand Down Expand Up @@ -229,6 +231,7 @@ impl<T> Read for Cursor<T> where T: AsRef<[u8]> {
}

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down
7 changes: 6 additions & 1 deletion src/libstd/io/impls.rs
@@ -1,5 +1,7 @@
use cmp;
use io::{self, SeekFrom, Read, Initializer, Write, Seek, BufRead, Error, ErrorKind};
use io::{self, SeekFrom, Read, Write, Seek, BufRead, Error, ErrorKind};
#[allow(deprecated)]
use io::Initializer;
use fmt;
use mem;

Expand All @@ -14,6 +16,7 @@ impl<'a, R: Read + ?Sized> Read for &'a mut R {
}

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
(**self).initializer()
}
Expand Down Expand Up @@ -83,6 +86,7 @@ impl<R: Read + ?Sized> Read for Box<R> {
}

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
(**self).initializer()
}
Expand Down Expand Up @@ -172,6 +176,7 @@ impl<'a> Read for &'a [u8] {
}

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down
21 changes: 19 additions & 2 deletions src/libstd/io/mod.rs
Expand Up @@ -367,7 +367,7 @@ fn read_to_end_with_reservation<R: Read + ?Sized>(r: &mut R,
g.buf.reserve(reservation_size);
let capacity = g.buf.capacity();
g.buf.set_len(capacity);
r.initializer().initialize(&mut g.buf[g.len..]);
ptr::freeze(g.buf.as_mut_ptr().add(g.len), g.buf.len() - g.len);
}
}

Expand Down Expand Up @@ -543,6 +543,8 @@ pub trait Read {
/// [`Initializer::nop()`]: ../../std/io/struct.Initializer.html#method.nop
/// [`Initializer`]: ../../std/io/struct.Initializer.html
#[unstable(feature = "read_initializer", issue = "42788")]
#[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")]
#[allow(deprecated)]
#[inline]
unsafe fn initializer(&self) -> Initializer {
Initializer::zeroing()
Expand Down Expand Up @@ -869,12 +871,22 @@ pub trait Read {

/// A type used to conditionally initialize buffers passed to `Read` methods.
#[unstable(feature = "read_initializer", issue = "42788")]
#[derive(Debug)]
#[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")]
pub struct Initializer(bool);

#[allow(deprecated)]
#[unstable(feature = "read_initializer", issue = "42788")]
impl fmt::Debug for Initializer {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt.debug_tuple("Initializer").field(&self.0).finish()
}
}

#[allow(deprecated)]
impl Initializer {
/// Returns a new `Initializer` which will zero out buffers.
#[unstable(feature = "read_initializer", issue = "42788")]
#[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")]
#[inline]
pub fn zeroing() -> Initializer {
Initializer(true)
Expand All @@ -889,20 +901,23 @@ impl Initializer {
/// the method accurately reflects the number of bytes that have been
/// written to the head of the buffer.
#[unstable(feature = "read_initializer", issue = "42788")]
#[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")]
#[inline]
pub unsafe fn nop() -> Initializer {
Initializer(false)
}

/// Indicates if a buffer should be initialized.
#[unstable(feature = "read_initializer", issue = "42788")]
#[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")]
#[inline]
pub fn should_initialize(&self) -> bool {
self.0
}

/// Initializes a buffer if necessary.
#[unstable(feature = "read_initializer", issue = "42788")]
#[rustc_deprecated(since = "1.33.0", reason = "use std::ptr::freeze instead")]
#[inline]
pub fn initialize(&self, buf: &mut [u8]) {
if self.should_initialize() {
Expand Down Expand Up @@ -1698,6 +1713,7 @@ impl<T: Read, U: Read> Read for Chain<T, U> {
self.second.read(buf)
}

#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
let initializer = self.first.initializer();
if initializer.should_initialize() {
Expand Down Expand Up @@ -1895,6 +1911,7 @@ impl<T: Read> Read for Take<T> {
Ok(n)
}

#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
self.inner.initializer()
}
Expand Down
7 changes: 6 additions & 1 deletion src/libstd/io/stdio.rs
Expand Up @@ -3,7 +3,9 @@ use io::prelude::*;
use cell::RefCell;
use fmt;
use io::lazy::Lazy;
use io::{self, Initializer, BufReader, LineWriter};
use io::{self, BufReader, LineWriter};
#[allow(deprecated)]
use io::Initializer;
use sync::{Arc, Mutex, MutexGuard};
use sys::stdio;
use sys_common::remutex::{ReentrantMutex, ReentrantMutexGuard};
Expand Down Expand Up @@ -67,6 +69,7 @@ impl Read for StdinRaw {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { self.0.read(buf) }

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down Expand Up @@ -297,6 +300,7 @@ impl Read for Stdin {
self.lock().read(buf)
}
#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand All @@ -317,6 +321,7 @@ impl<'a> Read for StdinLock<'a> {
self.inner.read(buf)
}
#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down
9 changes: 7 additions & 2 deletions src/libstd/io/util.rs
@@ -1,8 +1,11 @@
#![allow(missing_copy_implementations)]

use fmt;
use io::{self, Read, Initializer, Write, ErrorKind, BufRead};
use io::{self, Read, Write, ErrorKind, BufRead};
#[allow(deprecated)]
use io::Initializer;
use mem;
use ptr;

/// Copies the entire contents of a reader into a writer.
///
Expand Down Expand Up @@ -45,7 +48,7 @@ pub fn copy<R: ?Sized, W: ?Sized>(reader: &mut R, writer: &mut W) -> io::Result<
{
let mut buf = unsafe {
let mut buf: [u8; super::DEFAULT_BUF_SIZE] = mem::uninitialized();
reader.initializer().initialize(&mut buf);
ptr::freeze(&mut buf, 1);
buf
};

Expand Down Expand Up @@ -97,6 +100,7 @@ impl Read for Empty {
fn read(&mut self, _buf: &mut [u8]) -> io::Result<usize> { Ok(0) }

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down Expand Up @@ -153,6 +157,7 @@ impl Read for Repeat {
}

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down
1 change: 1 addition & 0 deletions src/libstd/lib.rs
Expand Up @@ -264,6 +264,7 @@
#![feature(panic_unwind)]
#![feature(prelude_import)]
#![feature(ptr_internals)]
#![feature(ptr_freeze)]
#![feature(raw)]
#![feature(hash_raw_entry)]
#![feature(rustc_attrs)]
Expand Down
6 changes: 5 additions & 1 deletion src/libstd/net/tcp.rs
@@ -1,7 +1,9 @@
use io::prelude::*;

use fmt;
use io::{self, Initializer};
use io;
#[allow(deprecated)]
use io::Initializer;
use net::{ToSocketAddrs, SocketAddr, Shutdown};
use sys_common::net as net_imp;
use sys_common::{AsInner, FromInner, IntoInner};
Expand Down Expand Up @@ -570,6 +572,7 @@ impl Read for TcpStream {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { self.0.read(buf) }

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand All @@ -584,6 +587,7 @@ impl<'a> Read for &'a TcpStream {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> { self.0.read(buf) }

#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down
6 changes: 5 additions & 1 deletion src/libstd/process.rs
Expand Up @@ -111,7 +111,9 @@ use io::prelude::*;
use ffi::OsStr;
use fmt;
use fs;
use io::{self, Initializer};
use io;
#[allow(deprecated)]
use io::Initializer;
use path::Path;
use str;
use sys::pipe::{read2, AnonPipe};
Expand Down Expand Up @@ -272,6 +274,7 @@ impl Read for ChildStdout {
self.inner.read(buf)
}
#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down Expand Up @@ -319,6 +322,7 @@ impl Read for ChildStderr {
self.inner.read(buf)
}
#[inline]
#[allow(deprecated)]
unsafe fn initializer(&self) -> Initializer {
Initializer::nop()
}
Expand Down

0 comments on commit 5e0fb23

Please sign in to comment.