From 5e0fb23076b9e9d8bc9585cd2bb4a056e3b23261 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sun, 10 Feb 2019 14:07:58 -0800 Subject: [PATCH 01/11] Deprecate Read::initializer in favor of ptr::freeze 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. --- src/libcore/ptr.rs | 50 +++++++++++++++++++++++++++++++++ src/libstd/fs.rs | 6 +++- src/libstd/io/buffered.rs | 8 ++++-- src/libstd/io/cursor.rs | 5 +++- src/libstd/io/impls.rs | 7 ++++- src/libstd/io/mod.rs | 21 ++++++++++++-- src/libstd/io/stdio.rs | 7 ++++- src/libstd/io/util.rs | 9 ++++-- src/libstd/lib.rs | 1 + src/libstd/net/tcp.rs | 6 +++- src/libstd/process.rs | 6 +++- src/libstd/sys/redox/ext/net.rs | 6 +++- src/libstd/sys/unix/ext/net.rs | 6 +++- src/libstd/sys/unix/fd.rs | 5 +++- 14 files changed, 128 insertions(+), 15 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 02eef07afd7ab..6d0a5bc3d0201 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -946,6 +946,56 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { intrinsics::volatile_store(dst, src); } +/// Freezes `count * size_of::()` 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(reader: &mut R) -> io::Result +/// 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(dst: *mut T, count: usize) { + let _ = count; + asm!("" : "=*m"(dst) : ); +} + #[lang = "const_ptr"] impl *const T { /// Returns `true` if the pointer is null. diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index 3538816c1124c..a7a442373e584 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -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}; @@ -600,6 +602,7 @@ impl Read for File { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -624,6 +627,7 @@ impl<'a> Read for &'a File { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 056aa7c0c4263..b557ba5907d5c 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -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. /// @@ -92,7 +95,7 @@ impl BufReader { 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(), @@ -236,6 +239,7 @@ impl Read for BufReader { } // we can't skip unconditionally because of the large buffer case in read. + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { self.inner.initializer() } diff --git a/src/libstd/io/cursor.rs b/src/libstd/io/cursor.rs index b205f7888389f..d1923c35fd471 100644 --- a/src/libstd/io/cursor.rs +++ b/src/libstd/io/cursor.rs @@ -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. @@ -229,6 +231,7 @@ impl Read for Cursor where T: AsRef<[u8]> { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/impls.rs b/src/libstd/io/impls.rs index ec75a87aec34f..9bb0ebd116156 100644 --- a/src/libstd/io/impls.rs +++ b/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; @@ -14,6 +16,7 @@ impl<'a, R: Read + ?Sized> Read for &'a mut R { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { (**self).initializer() } @@ -83,6 +86,7 @@ impl Read for Box { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { (**self).initializer() } @@ -172,6 +176,7 @@ impl<'a> Read for &'a [u8] { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index 28a6fbd48cf09..81956f0163624 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -367,7 +367,7 @@ fn read_to_end_with_reservation(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); } } @@ -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() @@ -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) @@ -889,6 +901,7 @@ 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) @@ -896,6 +909,7 @@ impl Initializer { /// 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 @@ -903,6 +917,7 @@ impl Initializer { /// 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() { @@ -1698,6 +1713,7 @@ impl Read for Chain { self.second.read(buf) } + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { let initializer = self.first.initializer(); if initializer.should_initialize() { @@ -1895,6 +1911,7 @@ impl Read for Take { Ok(n) } + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { self.inner.initializer() } diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 4068c0f9c7de5..e0d92fb7f3ed8 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -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}; @@ -67,6 +69,7 @@ impl Read for StdinRaw { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -297,6 +300,7 @@ impl Read for Stdin { self.lock().read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -317,6 +321,7 @@ impl<'a> Read for StdinLock<'a> { self.inner.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/io/util.rs b/src/libstd/io/util.rs index 8df961a9add6b..54ddb767a1fe3 100644 --- a/src/libstd/io/util.rs +++ b/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. /// @@ -45,7 +48,7 @@ pub fn copy(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 }; @@ -97,6 +100,7 @@ impl Read for Empty { fn read(&mut self, _buf: &mut [u8]) -> io::Result { Ok(0) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -153,6 +157,7 @@ impl Read for Repeat { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 8ecba3ecd68fd..ca65e79e33a37 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -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)] diff --git a/src/libstd/net/tcp.rs b/src/libstd/net/tcp.rs index 86ecb10edf2f9..a78a1fb6cba4a 100644 --- a/src/libstd/net/tcp.rs +++ b/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}; @@ -570,6 +572,7 @@ impl Read for TcpStream { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -584,6 +587,7 @@ impl<'a> Read for &'a TcpStream { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/process.rs b/src/libstd/process.rs index 1263ef82e4872..1dfec6cc6d4cb 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -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}; @@ -272,6 +274,7 @@ impl Read for ChildStdout { self.inner.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -319,6 +322,7 @@ impl Read for ChildStderr { self.inner.read(buf) } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/sys/redox/ext/net.rs b/src/libstd/sys/redox/ext/net.rs index 76c68829b7f1b..f2664ffc54f86 100644 --- a/src/libstd/sys/redox/ext/net.rs +++ b/src/libstd/sys/redox/ext/net.rs @@ -3,7 +3,9 @@ //! Unix-specific networking functionality use fmt; -use io::{self, Error, ErrorKind, Initializer}; +use io::{self, Error, ErrorKind}; +#[allow(deprecated)] +use io::Initializer; use net::Shutdown; use os::unix::io::{RawFd, AsRawFd, FromRawFd, IntoRawFd}; use path::Path; @@ -410,6 +412,7 @@ impl io::Read for UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -422,6 +425,7 @@ impl<'a> io::Read for &'a UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/sys/unix/ext/net.rs b/src/libstd/sys/unix/ext/net.rs index a3ae5943f6038..c9798b0062a0f 100644 --- a/src/libstd/sys/unix/ext/net.rs +++ b/src/libstd/sys/unix/ext/net.rs @@ -18,7 +18,9 @@ mod libc { use ascii; use ffi::OsStr; use fmt; -use io::{self, Initializer}; +use io; +#[allow(deprecated)] +use io::Initializer; use mem; use net::{self, Shutdown}; use os::unix::ffi::OsStrExt; @@ -552,6 +554,7 @@ impl io::Read for UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } @@ -564,6 +567,7 @@ impl<'a> io::Read for &'a UnixStream { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index 2cbd9536f4da7..e287b50e08d65 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -1,7 +1,9 @@ #![unstable(reason = "not public", issue = "0", feature = "fd")] use cmp; -use io::{self, Read, Initializer}; +use io::{self, Read}; +#[allow(deprecated)] +use io::Initializer; use libc::{self, c_int, c_void, ssize_t}; use mem; use sync::atomic::{AtomicBool, Ordering}; @@ -262,6 +264,7 @@ impl<'a> Read for &'a FileDesc { } #[inline] + #[allow(deprecated)] unsafe fn initializer(&self) -> Initializer { Initializer::nop() } From c21e79e3004e36477d14beabad7f90a21863be0e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 11 Feb 2019 16:28:02 +0100 Subject: [PATCH 02/11] Update src/libcore/ptr.rs Co-Authored-By: sfackler --- src/libcore/ptr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 6d0a5bc3d0201..565b3d3d5f9ae 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -952,7 +952,7 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { /// 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. +/// arbitrary contents 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. From e4d316ecd04b7dfd9bd3fc00919bf73ffc8503ee Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Mon, 11 Feb 2019 07:34:33 -0800 Subject: [PATCH 03/11] Doc cleanup --- src/libcore/ptr.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 565b3d3d5f9ae..f5ff4519b3861 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -947,7 +947,7 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { } /// Freezes `count * size_of::()` bytes of memory, converting undefined data into -/// defined-but-arbitrary data. +/// arbitrary but fixed data. /// /// Uninitialized memory has undefined contents, and interation with that data /// can easily cause undefined behavior. This function "freezes" memory @@ -961,7 +961,9 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { /// /// Behavior is undefined if any of the following conditions are violated: /// -/// * `dst` must be [valid] for reads. +/// * `dst` must be [valid] for writes. +/// +/// * Every bit representation of `T` must be a valid value. /// /// Note that even if `T` has size `0`, the pointer must be non-NULL and properly aligned. /// From 73133eef27316c40599fac2b40595a4b880a8329 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Tue, 12 Feb 2019 21:07:40 -0800 Subject: [PATCH 04/11] Convert freeze to an intrinsic and tweak docs --- src/libcore/intrinsics.rs | 3 +++ src/libcore/ptr.rs | 29 +++++++++++++++++++------- src/librustc_codegen_llvm/intrinsic.rs | 20 +++++++++++++++++- src/librustc_typeck/check/intrinsic.rs | 12 +++++++++++ 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index e927ed40d7fb7..4b8702d0052f1 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1523,4 +1523,7 @@ extern "rust-intrinsic" { /// Emits a `!nontemporal` store according to LLVM (see their docs). /// Probably will never become stable. pub fn nontemporal_store(ptr: *mut T, val: T); + + #[cfg(not(stage0))] + pub fn freeze(ptr: *mut T, count: usize); } diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index f5ff4519b3861..4c1d93a7c6cb9 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -946,16 +946,23 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { intrinsics::volatile_store(dst, src); } -/// Freezes `count * size_of::()` bytes of memory, converting undefined data into +/// Freezes `count * size_of::()` bytes of memory, converting uninitialized data into /// arbitrary but fixed 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 contents so that use of it is well defined. +/// Uninitialized memory has undefined contents, and interation with those contents +/// can easily cause undefined behavior. Freezing the memory avoids those issues by +/// converting the memory to an initialized state without actually needing to write to +/// all of it. This function has no effect on memory which is already initialized. /// -/// 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. +/// While this function does not actually physically write to memory, it acts as if it +/// does. For example, calling this method on data which is concurrently accessible +/// elsewhere is undefined behavior just as it would be to `ptr::write`. +/// +/// # Warning +/// +/// Take care when using this function as the uninitialized memory being frozen can +/// contain bits and pieces of previously discarded data, including sensitive +/// information such as cryptographic keys. /// /// # Safety /// @@ -993,6 +1000,14 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { /// ``` #[inline] #[unstable(feature = "ptr_freeze", issue = "0")] +#[cfg(not(stage0))] +pub unsafe fn freeze(dst: *mut T, count: usize) { + intrinsics::freeze(dst, count) +} +/// +#[inline] +#[unstable(feature = "ptr_freeze", issue = "0")] +#[cfg(stage0)] pub unsafe fn freeze(dst: *mut T, count: usize) { let _ = count; asm!("" : "=*m"(dst) : ); diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 58b466dbe6faa..1b29161d01873 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -16,7 +16,8 @@ use rustc::ty::{self, Ty}; use rustc::ty::layout::{self, LayoutOf, HasTyCtxt, Primitive}; use rustc_codegen_ssa::common::{IntPredicate, TypeKind}; use rustc::hir; -use syntax::ast::{self, FloatTy}; +use std::ffi::CStr; +use syntax::ast::{self, FloatTy, AsmDialect}; use syntax::symbol::Symbol; use builder::Builder; use value::Value; @@ -695,6 +696,23 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { return; } + "freeze" => { + let dst = args[0].immediate(); + let r = self.inline_asm_call( + CStr::from_bytes_with_nul(b"\0").unwrap(), + CStr::from_bytes_with_nul(b"r,~{memory}\0").unwrap(), + &[dst], + self.type_void(), + true, + false, + AsmDialect::Att, + ); + if r.is_none() { + bug!("broken freeze ASM"); + } + return; + } + _ => bug!("unknown intrinsic '{}'", name), }; diff --git a/src/librustc_typeck/check/intrinsic.rs b/src/librustc_typeck/check/intrinsic.rs index 82d4300d99687..748eae02824cd 100644 --- a/src/librustc_typeck/check/intrinsic.rs +++ b/src/librustc_typeck/check/intrinsic.rs @@ -379,6 +379,18 @@ pub fn check_intrinsic_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, (1, vec![ tcx.mk_mut_ptr(param(0)), param(0) ], tcx.mk_unit()) } + "freeze" => { + (1, + vec![ + tcx.mk_ptr(ty::TypeAndMut { + ty: param(0), + mutbl: hir::MutMutable + }), + tcx.types.usize, + ], + tcx.mk_unit()) + } + ref other => { struct_span_err!(tcx.sess, it.span, E0093, "unrecognized intrinsic function: `{}`", From 1671e904fb1de143b7717f37a84c45a407e763a3 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Tue, 12 Feb 2019 21:11:05 -0800 Subject: [PATCH 05/11] Tweak example --- src/libcore/ptr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 4c1d93a7c6cb9..7df3912ef2eb9 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -991,7 +991,7 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { /// // 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); +/// ptr::freeze(buf.as_mut_ptr(), buf.len()); /// reader.read_exact(&mut buf)?; /// /// Ok(u32::from_le_bytes(buf)) From 05299215219b186850adec6539bb0b1dd683408b Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Tue, 12 Feb 2019 21:38:09 -0800 Subject: [PATCH 06/11] Add a basic codegen test for freeze --- src/libcore/ptr.rs | 2 +- src/test/codegen/freeze.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 src/test/codegen/freeze.rs diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 7df3912ef2eb9..f6f1ee7193854 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -1010,7 +1010,7 @@ pub unsafe fn freeze(dst: *mut T, count: usize) { #[cfg(stage0)] pub unsafe fn freeze(dst: *mut T, count: usize) { let _ = count; - asm!("" : "=*m"(dst) : ); + asm!("" :: "r"(dst) : "memory" : "volatile"); } #[lang = "const_ptr"] diff --git a/src/test/codegen/freeze.rs b/src/test/codegen/freeze.rs new file mode 100644 index 0000000000000..f82380215f209 --- /dev/null +++ b/src/test/codegen/freeze.rs @@ -0,0 +1,18 @@ +// compile-flags: -O +#![crate_type="lib"] +#![feature(ptr_freeze)] + +use std::ptr; +use std::mem; + +// freeze should prevent reads of uninitialized memory from being UB +#[no_mangle] +pub fn read_uninitialized() -> u8 { + // CHECK-LABEL: @read_uninitialized + // CHECK-NOT: undef + unsafe { + let mut v: u8 = mem::uninitialized(); + ptr::freeze(&mut v, 1); + v + } +} From 695feb12ba2fabc61f0cf2c6d8b7f0b8a67f2e99 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Wed, 13 Feb 2019 09:53:00 -0800 Subject: [PATCH 07/11] Update src/libcore/ptr.rs Co-Authored-By: sfackler --- src/libcore/ptr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index f6f1ee7193854..6eb0589bdd9cd 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -949,7 +949,7 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { /// Freezes `count * size_of::()` bytes of memory, converting uninitialized data into /// arbitrary but fixed data. /// -/// Uninitialized memory has undefined contents, and interation with those contents +/// Uninitialized memory has undefined contents, and interaction with those contents /// can easily cause undefined behavior. Freezing the memory avoids those issues by /// converting the memory to an initialized state without actually needing to write to /// all of it. This function has no effect on memory which is already initialized. From 9d4f07eb549434d55b9d0bfb4f95f0004045a303 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Wed, 13 Feb 2019 10:15:37 -0800 Subject: [PATCH 08/11] Update src/libcore/ptr.rs Co-Authored-By: sfackler --- src/libcore/ptr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 6eb0589bdd9cd..0ca49926bc5d8 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -956,7 +956,7 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { /// /// While this function does not actually physically write to memory, it acts as if it /// does. For example, calling this method on data which is concurrently accessible -/// elsewhere is undefined behavior just as it would be to `ptr::write`. +/// elsewhere is undefined behavior just as it would be to use `ptr::write`. /// /// # Warning /// From 055bcb66182871a5e095e3aef59b96aca39af27d Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Wed, 13 Feb 2019 17:48:17 -0800 Subject: [PATCH 09/11] Little doc mention for freeze intrinsic --- src/libcore/intrinsics.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 4b8702d0052f1..7012a3de54254 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1524,6 +1524,7 @@ extern "rust-intrinsic" { /// Probably will never become stable. pub fn nontemporal_store(ptr: *mut T, val: T); + /// Freezes undefined data. Exposed as ptr::freeze. #[cfg(not(stage0))] pub fn freeze(ptr: *mut T, count: usize); } From 78e2233f87363da30c2e6087eb41c4fbc834d58c Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Wed, 13 Feb 2019 17:51:24 -0800 Subject: [PATCH 10/11] Clean up array freeze args --- src/libstd/io/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/io/util.rs b/src/libstd/io/util.rs index 54ddb767a1fe3..8345ae4bf0547 100644 --- a/src/libstd/io/util.rs +++ b/src/libstd/io/util.rs @@ -48,7 +48,7 @@ pub fn copy(reader: &mut R, writer: &mut W) -> io::Result< { let mut buf = unsafe { let mut buf: [u8; super::DEFAULT_BUF_SIZE] = mem::uninitialized(); - ptr::freeze(&mut buf, 1); + ptr::freeze(buf.as_mut_ptr(), buf.len()); buf }; From ec682d15bc3dc68dc441f5bd785e54ebcf55cddd Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 13 Feb 2019 19:17:05 -0800 Subject: [PATCH 11/11] Update src/libcore/intrinsics.rs Co-Authored-By: sfackler --- src/libcore/intrinsics.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 7012a3de54254..46b47e9248a2d 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1524,7 +1524,9 @@ extern "rust-intrinsic" { /// Probably will never become stable. pub fn nontemporal_store(ptr: *mut T, val: T); - /// Freezes undefined data. Exposed as ptr::freeze. + /// Freezes undefined data. + /// + /// Exposed as [`std::ptr::freeze`](../../std/ptr/fn.freeze.html). #[cfg(not(stage0))] pub fn freeze(ptr: *mut T, count: usize); }