From d888725fba70af4139629da0f7a17910aa66d951 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 13 Sep 2020 11:27:21 +0200 Subject: [PATCH 1/3] reduce size of test_from_iter_specialization_with_iterator_adapters test in Miri --- library/alloc/tests/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 608a3c9bdf762..5fdbef7171025 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -894,7 +894,7 @@ fn test_from_iter_partially_drained_in_place_specialization() { #[test] fn test_from_iter_specialization_with_iterator_adapters() { fn assert_in_place_trait(_: &T) {}; - let src: Vec = vec![0usize; 65535]; + let src: Vec = vec![0usize; if cfg!(miri) { 256 } else { 65535 }]; let srcptr = src.as_ptr(); let iter = src .into_iter() From c528d2419672bd4ace322ddbc000813a12c1d4c7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 13 Sep 2020 12:14:59 +0200 Subject: [PATCH 2/3] fix slice::check_range aliasing problems --- library/alloc/src/collections/vec_deque.rs | 6 +- library/alloc/src/slice.rs | 2 + library/alloc/src/string.rs | 3 +- library/alloc/src/vec.rs | 2 +- library/alloc/tests/vec.rs | 2 +- library/core/src/slice/index.rs | 75 +++++++++++++++++++- library/core/src/slice/mod.rs | 81 ++-------------------- 7 files changed, 85 insertions(+), 86 deletions(-) diff --git a/library/alloc/src/collections/vec_deque.rs b/library/alloc/src/collections/vec_deque.rs index 253a3e9f2bea9..65cfe9a9b4996 100644 --- a/library/alloc/src/collections/vec_deque.rs +++ b/library/alloc/src/collections/vec_deque.rs @@ -1089,11 +1089,7 @@ impl VecDeque { where R: RangeBounds, { - // SAFETY: This buffer is only used to check the range. It might be partially - // uninitialized, but `check_range` needs a contiguous slice. - // https://github.com/rust-lang/rust/pull/75207#discussion_r471193682 - let buffer = unsafe { slice::from_raw_parts(self.ptr(), self.len()) }; - let Range { start, end } = buffer.check_range(range); + let Range { start, end } = slice::check_range(self.len(), range); let tail = self.wrap_add(self.tail, start); let head = self.wrap_add(self.tail, end); (tail, head) diff --git a/library/alloc/src/slice.rs b/library/alloc/src/slice.rs index 677bfdd2349ec..55afdd94f4468 100644 --- a/library/alloc/src/slice.rs +++ b/library/alloc/src/slice.rs @@ -91,6 +91,8 @@ use crate::borrow::ToOwned; use crate::boxed::Box; use crate::vec::Vec; +#[unstable(feature = "slice_check_range", issue = "76393")] +pub use core::slice::check_range; #[unstable(feature = "array_chunks", issue = "74985")] pub use core::slice::ArrayChunks; #[unstable(feature = "array_chunks", issue = "74985")] diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index e1724bf3c9a90..2b0ce5ede5630 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -49,6 +49,7 @@ use core::iter::{FromIterator, FusedIterator}; use core::ops::Bound::{Excluded, Included, Unbounded}; use core::ops::{self, Add, AddAssign, Index, IndexMut, Range, RangeBounds}; use core::ptr; +use core::slice; use core::str::{lossy, pattern::Pattern}; use crate::borrow::{Cow, ToOwned}; @@ -1506,7 +1507,7 @@ impl String { // of the vector version. The data is just plain bytes. // Because the range removal happens in Drop, if the Drain iterator is leaked, // the removal will not happen. - let Range { start, end } = self.as_bytes().check_range(range); + let Range { start, end } = slice::check_range(self.len(), range); assert!(self.is_char_boundary(start)); assert!(self.is_char_boundary(end)); diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index cb4c1c20abcf6..fdd71a3211afa 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -1310,7 +1310,7 @@ impl Vec { // the hole, and the vector length is restored to the new length. // let len = self.len(); - let Range { start, end } = self.check_range(range); + let Range { start, end } = slice::check_range(len, range); unsafe { // set self.vec length's to start, to be safe in case Drain is leaked diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 5fdbef7171025..9ef5df01565c6 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -894,7 +894,7 @@ fn test_from_iter_partially_drained_in_place_specialization() { #[test] fn test_from_iter_specialization_with_iterator_adapters() { fn assert_in_place_trait(_: &T) {}; - let src: Vec = vec![0usize; if cfg!(miri) { 256 } else { 65535 }]; + let src: Vec = vec![0usize; 256]; let srcptr = src.as_ptr(); let iter = src .into_iter() diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index d67e0ae536d9f..a662d9682c51b 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -1,6 +1,6 @@ //! Indexing implementations for `[T]`. -use crate::ops; +use crate::ops::{self, Bound, Range, RangeBounds}; use crate::ptr; #[stable(feature = "rust1", since = "1.0.0")] @@ -62,6 +62,79 @@ pub(super) fn slice_end_index_overflow_fail() -> ! { panic!("attempted to index slice up to maximum usize"); } +/// Performs bounds-checking of the given range. +/// The returned [`Range`] is safe to pass to [`get_unchecked`] and [`get_unchecked_mut`] +/// for slices of the given length. +/// +/// [`get_unchecked`]: ../../std/primitive.slice.html#method.get_unchecked +/// [`get_unchecked_mut`]: ../../std/primitive.slice.html#method.get_unchecked_mut +/// +/// # Panics +/// +/// Panics if the range is out of bounds. +/// +/// # Examples +/// +/// ``` +/// #![feature(slice_check_range)] +/// use std::slice; +/// +/// let v = [10, 40, 30]; +/// assert_eq!(1..2, slice::check_range(v.len(), 1..2)); +/// assert_eq!(0..2, slice::check_range(v.len(), ..2)); +/// assert_eq!(1..3, slice::check_range(v.len(), 1..)); +/// ``` +/// +/// Panics when [`Index::index`] would panic: +/// +/// ```should_panic +/// #![feature(slice_check_range)] +/// +/// std::slice::check_range(3, 2..1); +/// ``` +/// +/// ```should_panic +/// #![feature(slice_check_range)] +/// +/// std::slice::check_range(3, 1..4); +/// ``` +/// +/// ```should_panic +/// #![feature(slice_check_range)] +/// +/// std::slice::check_range(3, 1..=usize::MAX); +/// ``` +/// +/// [`Index::index`]: crate::ops::Index::index +#[track_caller] +#[unstable(feature = "slice_check_range", issue = "76393")] +pub fn check_range>(len: usize, range: R) -> Range { + let start = match range.start_bound() { + Bound::Included(&start) => start, + Bound::Excluded(start) => { + start.checked_add(1).unwrap_or_else(|| slice_start_index_overflow_fail()) + } + Bound::Unbounded => 0, + }; + + let end = match range.end_bound() { + Bound::Included(end) => { + end.checked_add(1).unwrap_or_else(|| slice_end_index_overflow_fail()) + } + Bound::Excluded(&end) => end, + Bound::Unbounded => len, + }; + + if start > end { + slice_index_order_fail(start, end); + } + if end > len { + slice_end_index_len_fail(end, len); + } + + Range { start, end } +} + mod private_slice_index { use super::ops; #[stable(feature = "slice_get_slice", since = "1.28.0")] diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 64a707c39f076..ba3185433c8d8 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -12,7 +12,7 @@ use crate::cmp::Ordering::{self, Equal, Greater, Less}; use crate::intrinsics::assume; use crate::marker::{self, Copy}; use crate::mem; -use crate::ops::{Bound, FnMut, Range, RangeBounds}; +use crate::ops::{FnMut, Range, RangeBounds}; use crate::option::Option; use crate::option::Option::{None, Some}; use crate::ptr::{self, NonNull}; @@ -72,8 +72,8 @@ pub use sort::heapsort; #[stable(feature = "slice_get_slice", since = "1.28.0")] pub use index::SliceIndex; -use index::{slice_end_index_len_fail, slice_index_order_fail}; -use index::{slice_end_index_overflow_fail, slice_start_index_overflow_fail}; +#[unstable(feature = "slice_check_range", issue = "76393")] +pub use index::check_range; #[lang = "slice"] #[cfg(not(test))] @@ -378,79 +378,6 @@ impl [T] { unsafe { &mut *index.get_unchecked_mut(self) } } - /// Converts a range over this slice to [`Range`]. - /// - /// The returned range is safe to pass to [`get_unchecked`] and [`get_unchecked_mut`]. - /// - /// [`get_unchecked`]: #method.get_unchecked - /// [`get_unchecked_mut`]: #method.get_unchecked_mut - /// - /// # Panics - /// - /// Panics if the range is out of bounds. - /// - /// # Examples - /// - /// ``` - /// #![feature(slice_check_range)] - /// - /// let v = [10, 40, 30]; - /// assert_eq!(1..2, v.check_range(1..2)); - /// assert_eq!(0..2, v.check_range(..2)); - /// assert_eq!(1..3, v.check_range(1..)); - /// ``` - /// - /// Panics when [`Index::index`] would panic: - /// - /// ```should_panic - /// #![feature(slice_check_range)] - /// - /// [10, 40, 30].check_range(2..1); - /// ``` - /// - /// ```should_panic - /// #![feature(slice_check_range)] - /// - /// [10, 40, 30].check_range(1..4); - /// ``` - /// - /// ```should_panic - /// #![feature(slice_check_range)] - /// - /// [10, 40, 30].check_range(1..=usize::MAX); - /// ``` - /// - /// [`Index::index`]: crate::ops::Index::index - #[track_caller] - #[unstable(feature = "slice_check_range", issue = "76393")] - pub fn check_range>(&self, range: R) -> Range { - let start = match range.start_bound() { - Bound::Included(&start) => start, - Bound::Excluded(start) => { - start.checked_add(1).unwrap_or_else(|| slice_start_index_overflow_fail()) - } - Bound::Unbounded => 0, - }; - - let len = self.len(); - let end = match range.end_bound() { - Bound::Included(end) => { - end.checked_add(1).unwrap_or_else(|| slice_end_index_overflow_fail()) - } - Bound::Excluded(&end) => end, - Bound::Unbounded => len, - }; - - if start > end { - slice_index_order_fail(start, end); - } - if end > len { - slice_end_index_len_fail(end, len); - } - - Range { start, end } - } - /// Returns a raw pointer to the slice's buffer. /// /// The caller must ensure that the slice outlives the pointer this @@ -2794,7 +2721,7 @@ impl [T] { where T: Copy, { - let Range { start: src_start, end: src_end } = self.check_range(src); + let Range { start: src_start, end: src_end } = check_range(self.len(), src); let count = src_end - src_start; assert!(dest <= self.len() - count, "dest is out of bounds"); // SAFETY: the conditions for `ptr::copy` have all been checked above, From 7d67546a6ade5c1ade4b8e66266411f7aa4fae69 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 15 Sep 2020 23:46:26 +0200 Subject: [PATCH 3/3] hopefully fix rustdoc links --- library/core/src/slice/index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index a662d9682c51b..16fcb6231dc09 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -105,7 +105,7 @@ pub(super) fn slice_end_index_overflow_fail() -> ! { /// std::slice::check_range(3, 1..=usize::MAX); /// ``` /// -/// [`Index::index`]: crate::ops::Index::index +/// [`Index::index`]: ops::Index::index #[track_caller] #[unstable(feature = "slice_check_range", issue = "76393")] pub fn check_range>(len: usize, range: R) -> Range {