Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Document that slices cannot be larger than isize::MAX bytes #53784

Merged
merged 7 commits into from
Oct 1, 2018
Merged

Document that slices cannot be larger than isize::MAX bytes #53784

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1975b8d
Document that slices cannot be larger than `isize::MAX` bytes
tbu- Aug 29, 2018
7b77508
Add `debug_assert!`s to `slice::from_raw_parts`
tbu- Sep 4, 2018
1aac00f
A slice covering exactly half the address space is not OK
tbu- Sep 10, 2018
5d9086f
Fix overflow in `from_raw_parts` size check
tbu- Sep 11, 2018
11c6318
Use more obvious calculation for slice size check
tbu- Sep 17, 2018
ebffb67
Adjust the docs of `from_raw_parts` to match the implementation
tbu- Sep 17, 2018
e370b1c
Don't have two adjacent "see also" sentences
tbu- Sep 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/libcore/slice/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use cmp::Ordering::{self, Less, Equal, Greater};
use cmp;
use fmt;
use intrinsics::assume;
use isize;
use iter::*;
use ops::{FnMut, Try, self};
use option::Option;
Expand Down Expand Up @@ -3851,6 +3852,9 @@ unsafe impl<'a, T> TrustedRandomAccess for ExactChunksMut<'a, T> {
/// them from other data. You can obtain a pointer that is usable as `data`
/// for zero-length slices using [`NonNull::dangling()`].
///
/// The total size of the slice must lower than `isize::MAX` **bytes** in
/// memory. See the safety documentation of [`pointer::offset`].
///
/// # Caveat
///
/// The lifetime for the returned slice is inferred from its usage. To
Expand All @@ -3872,10 +3876,13 @@ unsafe impl<'a, T> TrustedRandomAccess for ExactChunksMut<'a, T> {
/// ```
///
/// [`NonNull::dangling()`]: ../../std/ptr/struct.NonNull.html#method.dangling
/// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
debug_assert!(data as usize % mem::align_of::<T>() == 0, "attempt to create unaligned slice");
debug_assert!(len * mem::size_of::<T>() < isize::MAX as usize,
"attempt to create slice covering half the address space");
Copy link
Member

@RalfJung RalfJung Sep 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that "exactly as large as half the address space" works correctly? That length (in bytes) does already not fit into a signed integer, so computing the address one-past-the-end (e.g. for iteration) would overflow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it'd be best written as len < isize::MAX as usize / mem::size_of::<T>()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RalfJung True.

@ubsan I don't think this is clearer.

Copy link
Contributor

@strega-nil strega-nil Sep 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tbu- not clearer, but it fixes overflow (and also happens at compile-time, as opposed to run-time)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, since the docs phrase the restriction in bytes, maybe write the assertion that way? Perhaps

debug_assert!(mem::size_of::<T>().saturating_mul(len) <= isize::MAX as usize);

That handles ZSTs automatically and avoids non-obvious rounding code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this!

Repr { raw: FatPtr { data, len } }.rust
}

Expand All @@ -3885,14 +3892,20 @@ pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
/// This function is unsafe for the same reasons as [`from_raw_parts`], as well
/// as not being able to provide a non-aliasing guarantee of the returned
/// mutable slice. `data` must be non-null and aligned even for zero-length
/// slices as with [`from_raw_parts`]. See the documentation of
/// [`from_raw_parts`] for more details.
/// slices as with [`from_raw_parts`]. The total size of the slice must be
/// lower than `isize::MAX` **bytes** in memory. See the safety documentation
/// of [`pointer::offset`].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit strange, with two "See ..." sentences right after one another. Also you made this new remark a new paragraph in the other method. Maybe remove the last sentence of this paragraph? Or else restore this paragraph to its original form, and add a new paragraph for the size limit.

///
/// See the documentation of [`from_raw_parts`] for more details.
///
/// [`from_raw_parts`]: ../../std/slice/fn.from_raw_parts.html
/// [`pointer::offset`]: ../../std/primitive.pointer.html#method.offset
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
pub unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] {
debug_assert!(data as usize % mem::align_of::<T>() == 0, "attempt to create unaligned slice");
debug_assert!(len * mem::size_of::<T>() < isize::MAX as usize,
"attempt to create slice covering half the address space");
Repr { raw: FatPtr { data, len} }.rust_mut
}

Expand Down