-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Make copy[_nonoverlapping] const #79684
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.
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
Changes from all commits
1b77f8e
7594d2a
d4fd798
1975a6e
5e27765
0cea1c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -1846,20 +1846,22 @@ pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) - | |||
/// [`Vec::append`]: ../../std/vec/struct.Vec.html#method.append | ||||
#[doc(alias = "memcpy")] | ||||
#[stable(feature = "rust1", since = "1.0.0")] | ||||
#[rustc_const_unstable(feature = "const_intrinsic_copy", issue = "none")] | ||||
#[inline] | ||||
pub unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize) { | ||||
pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize) { | ||||
extern "rust-intrinsic" { | ||||
fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize); | ||||
} | ||||
|
||||
if cfg!(debug_assertions) | ||||
// FIXME: Perform these checks only at run time | ||||
/*if cfg!(debug_assertions) | ||||
&& !(is_aligned_and_not_null(src) | ||||
&& is_aligned_and_not_null(dst) | ||||
&& is_nonoverlapping(src, dst, count)) | ||||
|
pub(crate) fn is_aligned_and_not_null<T>(ptr: *const T) -> bool { |
const fn
capable one. This should be doable by using guaranteed_ne
for the null pointer check and align_offset
for the alignment check. (you should be able to experiment with this on the playground, so you don't have to keep recompiling libcore).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be useful to continue working on this PR draft, or open a more restricted one, as an example use case/background for the problem and one potential solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR, I see two options:
- Leave it as "something we can do once we have a story for const-dependent dispatch".
- Comment out the debug assertions for now. Their usefulness is anyway limited since the libstd everyone uses is compiled without debug assertions.
I guess the question is one of evaluating the relative usefulness of these new const operations vs the assertions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this PR, I see two options:
- Leave it as "something we can do once we have a story for const-dependent dispatch".
- Comment out the debug assertions for now. Their usefulness is anyway limited since the libstd everyone uses is compiled without debug assertions.
I guess the question is one of evaluating the relative usefulness of these new const operations vs the assertions.
I have constified ptr::read[_unaligned]
and ptr::write
locally by commenting out the call to is_nonoverlapping
and align_offset
. I assume we can always remove the comments to bring back the checks and wait for const-dependent dispatch should we go for the latter option, right? ptr::read[_unaligned]
requires #79621, should I rebase and push my changes (does not include the unleash_the_miri_inside_of_you_here
attribute)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ptr::read[_unaligned] requires #79621, should I rebase and push my changes (does not include the unleash_the_miri_inside_of_you_here attribute)?
yes, that sounds right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by commenting out the call to is_nonoverlapping and align_offset
Do you mean is_aligned_and_not_null
? Which align_offset
call?
usbalbin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// Aligned to two bytes | ||
const DATA: [u16; 2] = [u16::from_ne_bytes([0x01, 0x23]), u16::from_ne_bytes([0x45, 0x67])]; | ||
|
||
const fn unaligned_ptr() -> *const u16 { | ||
// Since DATA.as_ptr() is aligned to two bytes, adding 1 byte to that produces an unaligned *const u16 | ||
unsafe { (DATA.as_ptr() as *const u8).add(1) as *const u16 } | ||
} | ||
|
||
#[test] | ||
fn read() { | ||
use core::ptr; | ||
|
||
const FOO: i32 = unsafe { ptr::read(&42 as *const i32) }; | ||
assert_eq!(FOO, 42); | ||
|
||
const ALIGNED: i32 = unsafe { ptr::read_unaligned(&42 as *const i32) }; | ||
assert_eq!(ALIGNED, 42); | ||
|
||
const UNALIGNED_PTR: *const u16 = unaligned_ptr(); | ||
|
||
const UNALIGNED: u16 = unsafe { ptr::read_unaligned(UNALIGNED_PTR) }; | ||
assert_eq!(UNALIGNED, u16::from_ne_bytes([0x23, 0x45])); | ||
oli-obk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
} | ||
|
||
#[test] | ||
fn const_ptr_read() { | ||
const FOO: i32 = unsafe { (&42 as *const i32).read() }; | ||
assert_eq!(FOO, 42); | ||
|
||
const ALIGNED: i32 = unsafe { (&42 as *const i32).read_unaligned() }; | ||
assert_eq!(ALIGNED, 42); | ||
|
||
const UNALIGNED_PTR: *const u16 = unaligned_ptr(); | ||
|
||
const UNALIGNED: u16 = unsafe { UNALIGNED_PTR.read_unaligned() }; | ||
assert_eq!(UNALIGNED, u16::from_ne_bytes([0x23, 0x45])); | ||
} | ||
|
||
#[test] | ||
fn mut_ptr_read() { | ||
const FOO: i32 = unsafe { (&42 as *const i32 as *mut i32).read() }; | ||
assert_eq!(FOO, 42); | ||
|
||
const ALIGNED: i32 = unsafe { (&42 as *const i32 as *mut i32).read_unaligned() }; | ||
assert_eq!(ALIGNED, 42); | ||
|
||
const UNALIGNED_PTR: *mut u16 = unaligned_ptr() as *mut u16; | ||
|
||
const UNALIGNED: u16 = unsafe { UNALIGNED_PTR.read_unaligned() }; | ||
assert_eq!(UNALIGNED, u16::from_ne_bytes([0x23, 0x45])); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// error-pattern: any use of this value will cause an error | ||
|
||
#![feature(const_ptr_read)] | ||
#![feature(const_ptr_offset)] | ||
|
||
fn main() { | ||
use std::ptr; | ||
|
||
const DATA: [u32; 1] = [42]; | ||
|
||
const PAST_END_PTR: *const u32 = unsafe { DATA.as_ptr().add(1) }; | ||
|
||
const _READ: u32 = unsafe { ptr::read(PAST_END_PTR) }; | ||
const _CONST_READ: u32 = unsafe { PAST_END_PTR.read() }; | ||
const _MUT_READ: u32 = unsafe { (PAST_END_PTR as *mut u32).read() }; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
error: any use of this value will cause an error | ||
--> $SRC_DIR/core/src/intrinsics.rs:LL:COL | ||
| | ||
LL | unsafe { copy_nonoverlapping(src, dst, count) } | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| memory access failed: pointer must be in-bounds at offset 8, but is outside bounds of alloc6 which has size 4 | ||
| inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL | ||
| inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL | ||
| inside `_READ` at $DIR/out_of_bounds_read.rs:13:33 | ||
| | ||
::: $DIR/out_of_bounds_read.rs:13:5 | ||
| | ||
LL | const _READ: u32 = unsafe { ptr::read(PAST_END_PTR) }; | ||
| ------------------------------------------------------ | ||
| | ||
= note: `#[deny(const_err)]` on by default | ||
|
||
error: any use of this value will cause an error | ||
--> $SRC_DIR/core/src/intrinsics.rs:LL:COL | ||
| | ||
LL | unsafe { copy_nonoverlapping(src, dst, count) } | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| memory access failed: pointer must be in-bounds at offset 8, but is outside bounds of alloc6 which has size 4 | ||
| inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL | ||
| inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL | ||
| inside `ptr::const_ptr::<impl *const u32>::read` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | ||
| inside `_CONST_READ` at $DIR/out_of_bounds_read.rs:14:39 | ||
| | ||
::: $DIR/out_of_bounds_read.rs:14:5 | ||
| | ||
LL | const _CONST_READ: u32 = unsafe { PAST_END_PTR.read() }; | ||
| -------------------------------------------------------- | ||
|
||
error: any use of this value will cause an error | ||
oli-obk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
--> $SRC_DIR/core/src/intrinsics.rs:LL:COL | ||
| | ||
LL | unsafe { copy_nonoverlapping(src, dst, count) } | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| memory access failed: pointer must be in-bounds at offset 8, but is outside bounds of alloc6 which has size 4 | ||
| inside `copy_nonoverlapping::<u32>` at $SRC_DIR/core/src/intrinsics.rs:LL:COL | ||
| inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL | ||
| inside `ptr::mut_ptr::<impl *mut u32>::read` at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL | ||
| inside `_MUT_READ` at $DIR/out_of_bounds_read.rs:15:37 | ||
| | ||
::: $DIR/out_of_bounds_read.rs:15:5 | ||
| | ||
LL | const _MUT_READ: u32 = unsafe { (PAST_END_PTR as *mut u32).read() }; | ||
| -------------------------------------------------------------------- | ||
|
||
error: aborting due to 3 previous errors | ||
|
Uh oh!
There was an error while loading. Please reload this page.