From 5719f57fb1628e33765393d886f1638bfa0da059 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Oct 2019 12:02:35 +0200 Subject: [PATCH 1/4] miri add write_bytes method to Memory doing bounds-checks and supporting iterators --- src/librustc/mir/interpret/allocation.rs | 9 +++++++-- src/librustc_mir/interpret/memory.rs | 21 ++++++++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 15e6cb6bcab..3bcde8defdf 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -346,11 +346,16 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { &mut self, cx: &impl HasDataLayout, ptr: Pointer, - src: &[u8], + src: impl IntoIterator, ) -> InterpResult<'tcx> { + let mut src = src.into_iter(); let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64))?; - bytes.clone_from_slice(src); + // `zip` would stop when the first iterator ends; we want to definitely + // cover all of `bytes`. + for dest in bytes { + *dest = src.next().expect("iterator was shorter than it said it would be"); + } Ok(()) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 924474c5317..0b65e9742b6 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -7,7 +7,7 @@ //! short-circuiting the empty case! use std::collections::VecDeque; -use std::ptr; +use std::{ptr, iter}; use std::borrow::Cow; use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt}; @@ -785,6 +785,25 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { self.get(ptr.alloc_id)?.read_c_str(self, ptr) } + /// Writes the given stream of bytes into memory. + /// + /// Performs appropriate bounds checks. + pub fn write_bytes( + &mut self, + ptr: Scalar, + src: impl IntoIterator, + ) -> InterpResult<'tcx> + { + let src = src.into_iter(); + let size = Size::from_bytes(src.len() as u64); + let ptr = match self.check_ptr_access(ptr, size, Align::from_bytes(1).unwrap())? { + Some(ptr) => ptr, + None => return Ok(()), // zero-sized access + }; + let tcx = self.tcx.tcx; + self.get_mut(ptr.alloc_id)?.write_bytes(&tcx, ptr, src) + } + /// Expects the caller to have checked bounds and alignment. pub fn copy( &mut self, From 50ddcbb2f5004da5b2d805c079e7c9699b6b7bea Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Oct 2019 12:06:03 +0200 Subject: [PATCH 2/4] also check the iterator is not too long --- src/librustc/lib.rs | 1 + src/librustc/mir/interpret/allocation.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index 8943fc342c0..3b0ac9ada8f 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -43,6 +43,7 @@ #![feature(nll)] #![feature(non_exhaustive)] #![feature(optin_builtin_traits)] +#![feature(option_expect_none)] #![feature(range_is_empty)] #![feature(slice_patterns)] #![feature(specialization)] diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 3bcde8defdf..87e9b91a86c 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -356,6 +356,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { for dest in bytes { *dest = src.next().expect("iterator was shorter than it said it would be"); } + src.next().expect_none("iterator was longer than it said it would be"); Ok(()) } From d4b365429ddaeb18b42ff47456bd9865fd1d732e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Oct 2019 14:57:21 +0200 Subject: [PATCH 3/4] points the user away from the Allocation type and towards the Memory type --- src/librustc/mir/interpret/allocation.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 87e9b91a86c..33c53323d96 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -245,6 +245,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// as a slice. /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to use the `PlaceTy` and `OperandTy`-based methods + /// on `InterpCx` instead. #[inline] pub fn get_bytes( &self, @@ -275,6 +277,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// so be sure to actually put data there! /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to use the `PlaceTy` and `OperandTy`-based methods + /// on `InterpCx` instead. pub fn get_bytes_mut( &mut self, cx: &impl HasDataLayout, @@ -297,6 +301,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Reads bytes until a `0` is encountered. Will error if the end of the allocation is reached /// before a `0` is found. + /// + /// Most likely, you want to call `Memory::read_c_str` instead of this method. pub fn read_c_str( &self, cx: &impl HasDataLayout, @@ -342,6 +348,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Writes `src` to the memory starting at `ptr.offset`. /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to call `Memory::write_bytes` instead of this method. pub fn write_bytes( &mut self, cx: &impl HasDataLayout, @@ -363,6 +370,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Sets `count` bytes starting at `ptr.offset` with `val`. Basically `memset`. /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to call `Memory::write_bytes` instead of this method. pub fn write_repeat( &mut self, cx: &impl HasDataLayout, @@ -386,6 +394,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// pointers being valid for ZSTs. /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to call `InterpCx::read_scalar` instead of this method. pub fn read_scalar( &self, cx: &impl HasDataLayout, @@ -424,6 +433,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Reads a pointer-sized scalar. /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to call `InterpCx::read_scalar` instead of this method. pub fn read_ptr_sized( &self, cx: &impl HasDataLayout, @@ -441,6 +451,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// pointers being valid for ZSTs. /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to call `InterpCx::write_scalar` instead of this method. pub fn write_scalar( &mut self, cx: &impl HasDataLayout, @@ -483,6 +494,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Writes a pointer-sized scalar. /// /// It is the caller's responsibility to check bounds and alignment beforehand. + /// Most likely, you want to call `InterpCx::write_scalar` instead of this method. pub fn write_ptr_sized( &mut self, cx: &impl HasDataLayout, From f6d70b42b8d6d299fe5621ac9170f69c511c8ddc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 21 Oct 2019 11:08:37 +0200 Subject: [PATCH 4/4] remove write_repeat; it is subsumed by the new write_bytes --- src/librustc/mir/interpret/allocation.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 33c53323d96..796d293e2c6 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -367,25 +367,6 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { Ok(()) } - /// Sets `count` bytes starting at `ptr.offset` with `val`. Basically `memset`. - /// - /// It is the caller's responsibility to check bounds and alignment beforehand. - /// Most likely, you want to call `Memory::write_bytes` instead of this method. - pub fn write_repeat( - &mut self, - cx: &impl HasDataLayout, - ptr: Pointer, - val: u8, - count: Size - ) -> InterpResult<'tcx> - { - let bytes = self.get_bytes_mut(cx, ptr, count)?; - for b in bytes { - *b = val; - } - Ok(()) - } - /// Reads a *non-ZST* scalar. /// /// ZSTs can't be read for two reasons: