Skip to content

Commit 2a5a320

Browse files
borsbors
committed
Auto merge of #25300 - kballard:core-slice-overflow, r=Gankro
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.
2 parents 0ad2026 + f2614f5 commit 2a5a320

File tree

2 files changed

+105
-64
lines changed

2 files changed

+105
-64
lines changed

‎src/libcore/slice.rs‎

Lines changed: 45 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ impl<T> SliceExt for [T] {
140140
assume(!p.is_null());
141141
if mem::size_of::<T>() == 0 {
142142
Iter {ptr: p,
143-
end: (p as usize + self.len()) as *const T,
143+
end: ((p as usize).wrapping_add(self.len())) as *const T,
144144
_marker: marker::PhantomData}
145145
} else {
146146
Iter {ptr: p,
@@ -277,7 +277,7 @@ impl<T> SliceExt for [T] {
277277
assume(!p.is_null());
278278
if mem::size_of::<T>() == 0 {
279279
IterMut {ptr: p,
280-
end: (p as usize + self.len()) as *mut T,
280+
end: ((p as usize).wrapping_add(self.len())) as *mut T,
281281
_marker: marker::PhantomData}
282282
} else {
283283
IterMut {ptr: p,
@@ -632,35 +632,17 @@ fn size_from_ptr<T>(_: *const T) -> usize {
632632

633633

634634
// Use macros to be generic over const/mut
635-
//
636-
// They require non-negative `$by` because otherwise the expression
637-
// `(ptr as usize + $by)` would interpret `-1` as `usize::MAX` (and
638-
// thus trigger a panic when overflow checks are on).
639-
640-
// Use this to do `$ptr + $by`, where `$by` is non-negative.
641-
macro_rules! slice_add_offset {
635+
macro_rules! slice_offset {
642636
($ptr:expr, $by:expr) => {{
643637
let ptr = $ptr;
644638
if size_from_ptr(ptr) == 0 {
645-
transmute(ptr as usize + $by)
639+
transmute((ptr as isize).wrapping_add($by))
646640
} else {
647641
ptr.offset($by)
648642
}
649643
}};
650644
}
651645

652-
// Use this to do `$ptr - $by`, where `$by` is non-negative.
653-
macro_rules! slice_sub_offset {
654-
($ptr:expr, $by:expr) => {{
655-
let ptr = $ptr;
656-
if size_from_ptr(ptr) == 0 {
657-
transmute(ptr as usize - $by)
658-
} else {
659-
ptr.offset(-$by)
660-
}
661-
}};
662-
}
663-
664646
macro_rules! slice_ref {
665647
($ptr:expr) => {{
666648
let ptr = $ptr;
@@ -683,22 +665,24 @@ macro_rules! iterator {
683665
#[inline]
684666
fn next(&mut self) -> Option<$elem> {
685667
// could be implemented with slices, but this avoids bounds checks
686-
unsafe {
687-
::intrinsics::assume(!self.ptr.is_null());
688-
::intrinsics::assume(!self.end.is_null());
689-
if self.ptr == self.end {
690-
None
691-
} else {
668+
if self.ptr == self.end {
669+
None
670+
} else {
671+
unsafe {
672+
if mem::size_of::<T>() != 0 {
673+
::intrinsics::assume(!self.ptr.is_null());
674+
::intrinsics::assume(!self.end.is_null());
675+
}
692676
let old = self.ptr;
693-
self.ptr = slice_add_offset!(self.ptr, 1);
677+
self.ptr = slice_offset!(self.ptr, 1);
694678
Some(slice_ref!(old))
695679
}
696680
}
697681
}
698682

699683
#[inline]
700684
fn size_hint(&self) -> (usize, Option<usize>) {
701-
let diff = (self.end as usize) - (self.ptr as usize);
685+
let diff = (self.end as usize).wrapping_sub(self.ptr as usize);
702686
let size = mem::size_of::<T>();
703687
let exact = diff / (if size == 0 {1} else {size});
704688
(exact, Some(exact))
@@ -726,13 +710,15 @@ macro_rules! iterator {
726710
#[inline]
727711
fn next_back(&mut self) -> Option<$elem> {
728712
// could be implemented with slices, but this avoids bounds checks
729-
unsafe {
730-
::intrinsics::assume(!self.ptr.is_null());
731-
::intrinsics::assume(!self.end.is_null());
732-
if self.end == self.ptr {
733-
None
734-
} else {
735-
self.end = slice_sub_offset!(self.end, 1);
713+
if self.end == self.ptr {
714+
None
715+
} else {
716+
unsafe {
717+
self.end = slice_offset!(self.end, -1);
718+
if mem::size_of::<T>() != 0 {
719+
::intrinsics::assume(!self.ptr.is_null());
720+
::intrinsics::assume(!self.end.is_null());
721+
}
736722
Some(slice_ref!(self.end))
737723
}
738724
}
@@ -742,29 +728,29 @@ macro_rules! iterator {
742728
}
743729

744730
macro_rules! make_slice {
745-
($t: ty => $result: ty: $start: expr, $end: expr) => {{
746-
let diff = $end as usize - $start as usize;
747-
let len = if mem::size_of::<T>() == 0 {
748-
diff
731+
($start: expr, $end: expr) => {{
732+
let start = $start;
733+
let diff = ($end as usize).wrapping_sub(start as usize);
734+
if size_from_ptr(start) == 0 {
735+
// use a non-null pointer value
736+
unsafe { from_raw_parts(1 as *const _, diff) }
749737
} else {
750-
diff / mem::size_of::<$t>()
751-
};
752-
unsafe {
753-
from_raw_parts($start, len)
738+
let len = diff / size_from_ptr(start);
739+
unsafe { from_raw_parts(start, len) }
754740
}
755741
}}
756742
}
757743

758744
macro_rules! make_mut_slice {
759-
($t: ty => $result: ty: $start: expr, $end: expr) => {{
760-
let diff = $end as usize - $start as usize;
761-
let len = if mem::size_of::<T>() == 0 {
762-
diff
745+
($start: expr, $end: expr) => {{
746+
let start = $start;
747+
let diff = ($end as usize).wrapping_sub(start as usize);
748+
if size_from_ptr(start) == 0 {
749+
// use a non-null pointer value
750+
unsafe { from_raw_parts_mut(1 as *mut _, diff) }
763751
} else {
764-
diff / mem::size_of::<$t>()
765-
};
766-
unsafe {
767-
from_raw_parts_mut($start, len)
752+
let len = diff / size_from_ptr(start);
753+
unsafe { from_raw_parts_mut(start, len) }
768754
}
769755
}}
770756
}
@@ -787,14 +773,14 @@ impl<'a, T> Iter<'a, T> {
787773
/// iterator can continue to be used while this exists.
788774
#[unstable(feature = "core")]
789775
pub fn as_slice(&self) -> &'a [T] {
790-
make_slice!(T => &'a [T]: self.ptr, self.end)
776+
make_slice!(self.ptr, self.end)
791777
}
792778

793779
// Helper function for Iter::nth
794780
fn iter_nth(&mut self, n: usize) -> Option<&'a T> {
795781
match self.as_slice().get(n) {
796782
Some(elem_ref) => unsafe {
797-
self.ptr = slice_add_offset!(elem_ref as *const _, 1);
783+
self.ptr = slice_offset!(self.ptr, (n as isize).wrapping_add(1));
798784
Some(slice_ref!(elem_ref))
799785
},
800786
None => {
@@ -827,12 +813,7 @@ impl<'a, T> RandomAccessIterator for Iter<'a, T> {
827813
fn idx(&mut self, index: usize) -> Option<&'a T> {
828814
unsafe {
829815
if index < self.indexable() {
830-
if mem::size_of::<T>() == 0 {
831-
// Use a non-null pointer value
832-
Some(&mut *(1 as *mut _))
833-
} else {
834-
Some(transmute(self.ptr.offset(index as isize)))
835-
}
816+
Some(slice_ref!(self.ptr.offset(index as isize)))
836817
} else {
837818
None
838819
}
@@ -860,14 +841,14 @@ impl<'a, T> IterMut<'a, T> {
860841
/// restricted lifetimes that do not consume the iterator.
861842
#[unstable(feature = "core")]
862843
pub fn into_slice(self) -> &'a mut [T] {
863-
make_mut_slice!(T => &'a mut [T]: self.ptr, self.end)
844+
make_mut_slice!(self.ptr, self.end)
864845
}
865846

866847
// Helper function for IterMut::nth
867848
fn iter_nth(&mut self, n: usize) -> Option<&'a mut T> {
868-
match make_mut_slice!(T => &'a mut [T]: self.ptr, self.end).get_mut(n) {
849+
match make_mut_slice!(self.ptr, self.end).get_mut(n) {
869850
Some(elem_ref) => unsafe {
870-
self.ptr = slice_add_offset!(elem_ref as *mut _, 1);
851+
self.ptr = slice_offset!(self.ptr, (n as isize).wrapping_add(1));
871852
Some(slice_ref!(elem_ref))
872853
},
873854
None => {

‎src/test/run-pass/slice-of-zero-size-elements.rs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// compile-flags: -C debug-assertions
12+
13+
#![feature(core)]
14+
15+
use std::slice;
16+
17+
fn foo<T>(v: &[T]) -> Option<&[T]> {
18+
let mut it = v.iter();
19+
for _ in 0..5 {
20+
let _ = it.next();
21+
}
22+
Some(it.as_slice())
23+
}
24+
25+
fn foo_mut<T>(v: &mut [T]) -> Option<&mut [T]> {
26+
let mut it = v.iter_mut();
27+
for _ in 0..5 {
28+
let _ = it.next();
29+
}
30+
Some(it.into_slice())
31+
}
32+
33+
pub fn main() {
34+
// In a slice of zero-size elements the pointer is meaningless.
35+
// Ensure iteration still works even if the pointer is at the end of the address space.
36+
let slice: &[()] = unsafe { slice::from_raw_parts(-5isize as *const (), 10) };
37+
assert_eq!(slice.len(), 10);
38+
assert_eq!(slice.iter().count(), 10);
39+
40+
// .nth() on the iterator should also behave correctly
41+
let mut it = slice.iter();
42+
assert!(it.nth(5).is_some());
43+
assert_eq!(it.count(), 4);
44+
45+
// Converting Iter to a slice should never have a null pointer
46+
assert!(foo(slice).is_some());
47+
48+
// Test mutable iterators as well
49+
let slice: &mut [()] = unsafe { slice::from_raw_parts_mut(-5isize as *mut (), 10) };
50+
assert_eq!(slice.len(), 10);
51+
assert_eq!(slice.iter_mut().count(), 10);
52+
53+
{
54+
let mut it = slice.iter_mut();
55+
assert!(it.nth(5).is_some());
56+
assert_eq!(it.count(), 4);
57+
}
58+
59+
assert!(foo_mut(slice).is_some())
60+
}

0 commit comments

Comments
 (0)