Skip to content

Commit 899d960

Browse files
the8472the8472
committed
mark vec::IntoIter pointers as !nonnull 
1 parent f7fe213 commit 899d960

File tree

6 files changed

+120
-49
lines changed

6 files changed

+120
-49
lines changed

‎library/alloc/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@
178178
#![feature(const_ptr_write)]
179179
#![feature(const_trait_impl)]
180180
#![feature(const_try)]
181+
#![feature(decl_macro)]
181182
#![feature(dropck_eyepatch)]
182183
#![feature(exclusive_range_pattern)]
183184
#![feature(fundamental)]

‎library/alloc/src/vec/in_place_collect.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ where
190190
// then the source pointer will stay in its initial position and we can't use it as reference
191191
if src.ptr != src_ptr {
192192
debug_assert!(
193-
unsafe { dst_buf.add(len) as *const _ } <= src.ptr,
193+
unsafe { dst_buf.add(len) as *const _ } <= src.ptr.as_ptr(),
194194
"InPlaceIterable contract violation, write pointer advanced beyond read pointer"
195195
);
196196
}

‎library/alloc/src/vec/into_iter.rs

+68-38
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,17 @@ use core::ops::Deref;
1717
use core::ptr::{self, NonNull};
1818
use core::slice::{self};
1919

20+
macro non_null {
21+
(mut $place:expr, $t:ident) => {{
22+
#![allow(unused_unsafe)] // we're sometimes used within an unsafe block
23+
unsafe { &mut *ptr::addr_of_mut!($place).cast::<NonNull<$t>>() }
24+
}},
25+
($place:expr, $t:ident) => {{
26+
#![allow(unused_unsafe)] // we're sometimes used within an unsafe block
27+
unsafe { *ptr::addr_of!($place).cast::<NonNull<$t>>() }
28+
}},
29+
}
30+
2031
/// An iterator that moves out of a vector.
2132
///
2233
/// This `struct` is created by the `into_iter` method on [`Vec`](super::Vec)
@@ -40,10 +51,12 @@ pub struct IntoIter<
4051
// the drop impl reconstructs a RawVec from buf, cap and alloc
4152
// to avoid dropping the allocator twice we need to wrap it into ManuallyDrop
4253
pub(super) alloc: ManuallyDrop<A>,
43-
pub(super) ptr: *const T,
44-
pub(super) end: *const T, // If T is a ZST, this is actually ptr+len. This encoding is picked so that
45-
// ptr == end is a quick test for the Iterator being empty, that works
46-
// for both ZST and non-ZST.
54+
pub(super) ptr: NonNull<T>,
55+
/// If T is a ZST, this is actually ptr+len. This encoding is picked so that
56+
/// ptr == end is a quick test for the Iterator being empty, that works
57+
/// for both ZST and non-ZST.
58+
/// For non-ZSTs the pointer is treated as `NonNull<T>`
59+
pub(super) end: *const T,
4760
}
4861

4962
#[stable(feature = "vec_intoiter_debug", since = "1.13.0")]
@@ -67,7 +80,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
6780
/// ```
6881
#[stable(feature = "vec_into_iter_as_slice", since = "1.15.0")]
6982
pub fn as_slice(&self) -> &[T] {
70-
unsafe { slice::from_raw_parts(self.ptr, self.len()) }
83+
unsafe { slice::from_raw_parts(self.ptr.as_ptr(), self.len()) }
7184
}
7285

7386
/// Returns the remaining items of this iterator as a mutable slice.
@@ -96,7 +109,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
96109
}
97110

98111
fn as_raw_mut_slice(&mut self) -> *mut [T] {
99-
ptr::slice_from_raw_parts_mut(self.ptr as *mut T, self.len())
112+
ptr::slice_from_raw_parts_mut(self.ptr.as_ptr(), self.len())
100113
}
101114

102115
/// Drops remaining elements and relinquishes the backing allocation.
@@ -123,7 +136,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
123136
// this creates less assembly
124137
self.cap = 0;
125138
self.buf = unsafe { NonNull::new_unchecked(RawVec::NEW.ptr()) };
126-
self.ptr = self.buf.as_ptr();
139+
self.ptr = self.buf;
127140
self.end = self.buf.as_ptr();
128141

129142
// Dropping the remaining elements can panic, so this needs to be
@@ -137,7 +150,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
137150
pub(crate) fn forget_remaining_elements(&mut self) {
138151
// For the ZST case, it is crucial that we mutate `end` here, not `ptr`.
139152
// `ptr` must stay aligned, while `end` may be unaligned.
140-
self.end = self.ptr;
153+
self.end = self.ptr.as_ptr();
141154
}
142155

143156
#[cfg(not(no_global_oom_handling))]
@@ -159,7 +172,7 @@ impl<T, A: Allocator> IntoIter<T, A> {
159172
// say that they're all at the beginning of the "allocation".
160173
0..this.len()
161174
} else {
162-
this.ptr.sub_ptr(buf)..this.end.sub_ptr(buf)
175+
this.ptr.sub_ptr(this.buf)..this.end.sub_ptr(buf)
163176
};
164177
let cap = this.cap;
165178
let alloc = ManuallyDrop::take(&mut this.alloc);
@@ -186,37 +199,43 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
186199

187200
#[inline]
188201
fn next(&mut self) -> Option<T> {
189-
if self.ptr == self.end {
190-
None
191-
} else if T::IS_ZST {
192-
// `ptr` has to stay where it is to remain aligned, so we reduce the length by 1 by
193-
// reducing the `end`.
194-
self.end = self.end.wrapping_byte_sub(1);
195-
196-
// Make up a value of this ZST.
197-
Some(unsafe { mem::zeroed() })
202+
if T::IS_ZST {
203+
if self.ptr.as_ptr().cast_const() == self.end {
204+
None
205+
} else {
206+
// `ptr` has to stay where it is to remain aligned, so we reduce the length by 1 by
207+
// reducing the `end`.
208+
self.end = self.end.wrapping_byte_sub(1);
209+
210+
// Make up a value of this ZST.
211+
Some(unsafe { mem::zeroed() })
212+
}
198213
} else {
199-
let old = self.ptr;
200-
self.ptr = unsafe { self.ptr.add(1) };
214+
if self.ptr == non_null!(self.end, T) {
215+
None
216+
} else {
217+
let old = self.ptr;
218+
self.ptr = unsafe { old.add(1) };
201219

202-
Some(unsafe { ptr::read(old) })
220+
Some(unsafe { ptr::read(old.as_ptr()) })
221+
}
203222
}
204223
}
205224

206225
#[inline]
207226
fn size_hint(&self) -> (usize, Option<usize>) {
208227
let exact = if T::IS_ZST {
209-
self.end.addr().wrapping_sub(self.ptr.addr())
228+
self.end.addr().wrapping_sub(self.ptr.as_ptr().addr())
210229
} else {
211-
unsafe { self.end.sub_ptr(self.ptr) }
230+
unsafe { non_null!(self.end, T).sub_ptr(self.ptr) }
212231
};
213232
(exact, Some(exact))
214233
}
215234

216235
#[inline]
217236
fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> {
218237
let step_size = self.len().min(n);
219-
let to_drop = ptr::slice_from_raw_parts_mut(self.ptr as *mut T, step_size);
238+
let to_drop = ptr::slice_from_raw_parts_mut(self.ptr.as_ptr(), step_size);
220239
if T::IS_ZST {
221240
// See `next` for why we sub `end` here.
222241
self.end = self.end.wrapping_byte_sub(step_size);
@@ -258,7 +277,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
258277
// Safety: `len` indicates that this many elements are available and we just checked that
259278
// it fits into the array.
260279
unsafe {
261-
ptr::copy_nonoverlapping(self.ptr, raw_ary.as_mut_ptr() as *mut T, len);
280+
ptr::copy_nonoverlapping(self.ptr.as_ptr(), raw_ary.as_mut_ptr() as *mut T, len);
262281
self.forget_remaining_elements();
263282
return Err(array::IntoIter::new_unchecked(raw_ary, 0..len));
264283
}
@@ -267,7 +286,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
267286
// Safety: `len` is larger than the array size. Copy a fixed amount here to fully initialize
268287
// the array.
269288
return unsafe {
270-
ptr::copy_nonoverlapping(self.ptr, raw_ary.as_mut_ptr() as *mut T, N);
289+
ptr::copy_nonoverlapping(self.ptr.as_ptr(), raw_ary.as_mut_ptr() as *mut T, N);
271290
self.ptr = self.ptr.add(N);
272291
Ok(raw_ary.transpose().assume_init())
273292
};
@@ -286,7 +305,7 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
286305
// that `T: Copy` so reading elements from the buffer doesn't invalidate
287306
// them for `Drop`.
288307
unsafe {
289-
if T::IS_ZST { mem::zeroed() } else { ptr::read(self.ptr.add(i)) }
308+
if T::IS_ZST { mem::zeroed() } else { ptr::read(self.ptr.add(i).as_ptr()) }
290309
}
291310
}
292311
}
@@ -295,18 +314,25 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
295314
impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
296315
#[inline]
297316
fn next_back(&mut self) -> Option<T> {
298-
if self.end == self.ptr {
299-
None
300-
} else if T::IS_ZST {
301-
// See above for why 'ptr.offset' isn't used
302-
self.end = self.end.wrapping_byte_sub(1);
303-
304-
// Make up a value of this ZST.
305-
Some(unsafe { mem::zeroed() })
317+
if T::IS_ZST {
318+
if self.end == self.ptr.as_ptr().cast_const() {
319+
None
320+
} else {
321+
// See above for why 'ptr.offset' isn't used
322+
self.end = self.end.wrapping_byte_sub(1);
323+
324+
// Make up a value of this ZST.
325+
Some(unsafe { mem::zeroed() })
326+
}
306327
} else {
307-
self.end = unsafe { self.end.sub(1) };
328+
if non_null!(self.end, T) == self.ptr {
329+
None
330+
} else {
331+
let new_end = unsafe { non_null!(self.end, T).sub(1) };
332+
*non_null!(mut self.end, T) = new_end;
308333

309-
Some(unsafe { ptr::read(self.end) })
334+
Some(unsafe { ptr::read(new_end.as_ptr()) })
335+
}
310336
}
311337
}
312338

@@ -332,7 +358,11 @@ impl<T, A: Allocator> DoubleEndedIterator for IntoIter<T, A> {
332358
#[stable(feature = "rust1", since = "1.0.0")]
333359
impl<T, A: Allocator> ExactSizeIterator for IntoIter<T, A> {
334360
fn is_empty(&self) -> bool {
335-
self.ptr == self.end
361+
if T::IS_ZST {
362+
self.ptr.as_ptr().cast_const() == self.end
363+
} else {
364+
self.ptr == non_null!(self.end, T)
365+
}
336366
}
337367
}
338368

‎library/alloc/src/vec/mod.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -2730,14 +2730,8 @@ impl<T, A: Allocator> IntoIterator for Vec<T, A> {
27302730
begin.add(me.len()) as *const T
27312731
};
27322732
let cap = me.buf.capacity();
2733-
IntoIter {
2734-
buf: NonNull::new_unchecked(begin),
2735-
phantom: PhantomData,
2736-
cap,
2737-
alloc,
2738-
ptr: begin,
2739-
end,
2740-
}
2733+
let buf = NonNull::new_unchecked(begin);
2734+
IntoIter { buf, phantom: PhantomData, cap, alloc, ptr: buf, end }
27412735
}
27422736
}
27432737
}

‎library/alloc/src/vec/spec_from_iter.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,12 @@ impl<T> SpecFromIter<T, IntoIter<T>> for Vec<T> {
4444
// than creating it through the generic FromIterator implementation would. That limitation
4545
// is not strictly necessary as Vec's allocation behavior is intentionally unspecified.
4646
// But it is a conservative choice.
47-
let has_advanced = iterator.buf.as_ptr() as *const _ != iterator.ptr;
47+
let has_advanced = iterator.buf != iterator.ptr;
4848
if !has_advanced || iterator.len() >= iterator.cap / 2 {
4949
unsafe {
5050
let it = ManuallyDrop::new(iterator);
5151
if has_advanced {
52-
ptr::copy(it.ptr, it.buf.as_ptr(), it.len());
52+
ptr::copy(it.ptr.as_ptr(), it.buf.as_ptr(), it.len());
5353
}
5454
return Vec::from_raw_parts(it.buf.as_ptr(), it.len(), it.cap);
5555
}

‎tests/codegen/vec-iter.rs

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// ignore-debug: the debug assertions get in the way
2+
// compile-flags: -O
3+
#![crate_type = "lib"]
4+
#![feature(exact_size_is_empty)]
5+
6+
use std::vec;
7+
8+
// CHECK-LABEL: @vec_iter_len_nonnull
9+
#[no_mangle]
10+
pub fn vec_iter_len_nonnull(it: &vec::IntoIter<u8>) -> usize {
11+
// CHECK: %subtrahend.i.i = load ptr
12+
// CHECK-SAME: !nonnull
13+
// CHECK-SAME: !noundef
14+
// CHECK: %self2.i.i = load ptr
15+
// CHECK-SAME: !nonnull
16+
// CHECK-SAME: !noundef
17+
// CHECK: sub nuw
18+
// CHECK: ret
19+
it.len()
20+
}
21+
22+
// CHECK-LABEL: @vec_iter_is_empty_nonnull
23+
#[no_mangle]
24+
pub fn vec_iter_is_empty_nonnull(it: &vec::IntoIter<u8>) -> bool {
25+
// CHECK: %other.i = load ptr
26+
// CHECK-SAME: !nonnull
27+
// CHECK-SAME: !noundef
28+
// CHECK: %self2.i = load ptr
29+
// CHECK-SAME: !nonnull
30+
// CHECK-SAME: !noundef
31+
// CHECK: ret
32+
it.is_empty()
33+
}
34+
35+
// CHECK-LABEL: @vec_iter_next
36+
#[no_mangle]
37+
pub fn vec_iter_next(it: &mut vec::IntoIter<u8>) -> Option<u8> {
38+
// CHECK: %other.i = load ptr
39+
// CHECK-SAME: !nonnull
40+
// CHECK-SAME: !noundef
41+
// CHECK: %self2.i = load ptr
42+
// CHECK-SAME: !nonnull
43+
// CHECK-SAME: !noundef
44+
// CHECK: ret
45+
it.next()
46+
}

0 commit comments

Comments
 (0)