From 52efe55d0425d963ee949be96e36d584a603a455 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Mon, 11 May 2015 00:55:21 -0700 Subject: [PATCH] Handle overflow properly in core::slice core::slice was originally written to tolerate overflow (notably, with slices of zero-sized elements), but it was never updated to use wrapping arithmetic when overflow traps were added. Also correctly handle the case of calling .nth() on an Iter with a zero-sized element type. The iterator was assuming that the pointer value of the returned reference was meaningful, but that's not true for zero-sized elements. Fixes #25016. --- src/libcore/slice.rs | 51 +++++-------------- .../run-pass/slice-of-zero-size-elements.rs | 34 +++++++++++++ 2 files changed, 46 insertions(+), 39 deletions(-) create mode 100644 src/test/run-pass/slice-of-zero-size-elements.rs diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index 56dee3034870c..f9aa83d241a75 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -140,7 +140,7 @@ impl SliceExt for [T] { assume(!p.is_null()); if mem::size_of::() == 0 { Iter {ptr: p, - end: (p as usize + self.len()) as *const T, + end: ((p as usize).wrapping_add(self.len())) as *const T, _marker: marker::PhantomData} } else { Iter {ptr: p, @@ -277,7 +277,7 @@ impl SliceExt for [T] { assume(!p.is_null()); if mem::size_of::() == 0 { IterMut {ptr: p, - end: (p as usize + self.len()) as *mut T, + end: ((p as usize).wrapping_add(self.len())) as *mut T, _marker: marker::PhantomData} } else { IterMut {ptr: p, @@ -632,35 +632,17 @@ fn size_from_ptr(_: *const T) -> usize { // Use macros to be generic over const/mut -// -// They require non-negative `$by` because otherwise the expression -// `(ptr as usize + $by)` would interpret `-1` as `usize::MAX` (and -// thus trigger a panic when overflow checks are on). - -// Use this to do `$ptr + $by`, where `$by` is non-negative. -macro_rules! slice_add_offset { +macro_rules! slice_offset { ($ptr:expr, $by:expr) => {{ let ptr = $ptr; if size_from_ptr(ptr) == 0 { - transmute(ptr as usize + $by) + transmute((ptr as isize).wrapping_add($by)) } else { ptr.offset($by) } }}; } -// Use this to do `$ptr - $by`, where `$by` is non-negative. -macro_rules! slice_sub_offset { - ($ptr:expr, $by:expr) => {{ - let ptr = $ptr; - if size_from_ptr(ptr) == 0 { - transmute(ptr as usize - $by) - } else { - ptr.offset(-$by) - } - }}; -} - macro_rules! slice_ref { ($ptr:expr) => {{ let ptr = $ptr; @@ -684,13 +666,11 @@ macro_rules! iterator { fn next(&mut self) -> Option<$elem> { // could be implemented with slices, but this avoids bounds checks unsafe { - ::intrinsics::assume(!self.ptr.is_null()); - ::intrinsics::assume(!self.end.is_null()); if self.ptr == self.end { None } else { let old = self.ptr; - self.ptr = slice_add_offset!(self.ptr, 1); + self.ptr = slice_offset!(self.ptr, 1); Some(slice_ref!(old)) } } @@ -698,7 +678,7 @@ macro_rules! iterator { #[inline] fn size_hint(&self) -> (usize, Option) { - let diff = (self.end as usize) - (self.ptr as usize); + let diff = (self.end as usize).wrapping_sub(self.ptr as usize); let size = mem::size_of::(); let exact = diff / (if size == 0 {1} else {size}); (exact, Some(exact)) @@ -727,12 +707,10 @@ macro_rules! iterator { fn next_back(&mut self) -> Option<$elem> { // could be implemented with slices, but this avoids bounds checks unsafe { - ::intrinsics::assume(!self.ptr.is_null()); - ::intrinsics::assume(!self.end.is_null()); if self.end == self.ptr { None } else { - self.end = slice_sub_offset!(self.end, 1); + self.end = slice_offset!(self.end, -1); Some(slice_ref!(self.end)) } } @@ -743,7 +721,7 @@ macro_rules! iterator { macro_rules! make_slice { ($t: ty => $result: ty: $start: expr, $end: expr) => {{ - let diff = $end as usize - $start as usize; + let diff = ($end as usize).wrapping_sub($start as usize); let len = if mem::size_of::() == 0 { diff } else { @@ -757,7 +735,7 @@ macro_rules! make_slice { macro_rules! make_mut_slice { ($t: ty => $result: ty: $start: expr, $end: expr) => {{ - let diff = $end as usize - $start as usize; + let diff = ($end as usize).wrapping_sub($start as usize); let len = if mem::size_of::() == 0 { diff } else { @@ -794,7 +772,7 @@ impl<'a, T> Iter<'a, T> { fn iter_nth(&mut self, n: usize) -> Option<&'a T> { match self.as_slice().get(n) { Some(elem_ref) => unsafe { - self.ptr = slice_add_offset!(elem_ref as *const _, 1); + self.ptr = slice_offset!(self.ptr, (n as isize).wrapping_add(1)); Some(slice_ref!(elem_ref)) }, None => { @@ -827,12 +805,7 @@ impl<'a, T> RandomAccessIterator for Iter<'a, T> { fn idx(&mut self, index: usize) -> Option<&'a T> { unsafe { if index < self.indexable() { - if mem::size_of::() == 0 { - // Use a non-null pointer value - Some(&mut *(1 as *mut _)) - } else { - Some(transmute(self.ptr.offset(index as isize))) - } + Some(slice_ref!(self.ptr.offset(index as isize))) } else { None } @@ -867,7 +840,7 @@ impl<'a, T> IterMut<'a, T> { fn iter_nth(&mut self, n: usize) -> Option<&'a mut T> { match make_mut_slice!(T => &'a mut [T]: self.ptr, self.end).get_mut(n) { Some(elem_ref) => unsafe { - self.ptr = slice_add_offset!(elem_ref as *mut _, 1); + self.ptr = slice_offset!(self.ptr, (n as isize).wrapping_add(1)); Some(slice_ref!(elem_ref)) }, None => { diff --git a/src/test/run-pass/slice-of-zero-size-elements.rs b/src/test/run-pass/slice-of-zero-size-elements.rs new file mode 100644 index 0000000000000..98b668031a894 --- /dev/null +++ b/src/test/run-pass/slice-of-zero-size-elements.rs @@ -0,0 +1,34 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -C debug-assertions + +use std::slice; + +pub fn main() { + // In a slice of zero-size elements the pointer is meaningless. + // Ensure iteration still works even if the pointer is at the end of the address space. + let slice: &[()] = unsafe { slice::from_raw_parts(-5isize as *const (), 10) }; + assert_eq!(slice.len(), 10); + assert_eq!(slice.iter().count(), 10); + + // .nth() on the iterator should also behave correctly + let mut it = slice.iter(); + assert!(it.nth(5).is_some()); + assert_eq!(it.count(), 4); + + let slice: &mut [()] = unsafe { slice::from_raw_parts_mut(-5isize as *mut (), 10) }; + assert_eq!(slice.len(), 10); + assert_eq!(slice.iter_mut().count(), 10); + + let mut it = slice.iter_mut(); + assert!(it.nth(5).is_some()); + assert_eq!(it.count(), 4); +}