From 23dad2e1f8591f734aa1bad1dc5dd8c13c9c5bab Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 26 Jun 2017 14:22:14 +0200 Subject: [PATCH 1/8] Start `is_aligned` intrinsic rfc --- text/0000-is-aligned-intrinsic.md | 99 +++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 text/0000-is-aligned-intrinsic.md diff --git a/text/0000-is-aligned-intrinsic.md b/text/0000-is-aligned-intrinsic.md new file mode 100644 index 00000000000..e93dd713138 --- /dev/null +++ b/text/0000-is-aligned-intrinsic.md @@ -0,0 +1,99 @@ +- Feature Name: is_aligned_intrinsic +- Start Date: 2017-06-20 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Add an intrinsic (`fn is_aligned(*const T) -> bool`) which returns `true` if +a read through the pointer would not fail or suffer from slowdown due to the +alignment of the pointer. There are no further guarantees given about the pointer +it is perfectly valid for `is_aligned(0 as *const T)` to return `true`. + +# Motivation +[motivation]: #motivation + +The standard library (and most likely many crates) use code like + +```rust +let is_aligned = (ptr as usize) & ((1 << (align - 1)) - 1) == 0; +let is_2_byte_aligned = ((ptr as usize + index) & (usize_bytes - 1)) == 0; +let is_t_aligned = ((ptr as usize) % std::mem::align_of::()) == 0; +``` + +to check whether a pointer is aligned in order to perform optimizations like +reading multiple bytes at once. Not only is this code which is easy to get +wrong, and which is hard to read (and thus increasing the chance of future breakage) +but it also makes it impossible for `miri` to evaluate such statements. This +means that `miri` cannot do utf8-checking, since that code contains such +optimizations. Without utf8-checking, Rustc's future const evaluation would not +be able to convert a `[u8]` into a `str`. + +# Detailed design +[design]: #detailed-design + +Add a new intrinsic `is_aligned` with the following signature: + +```rust +fn is_aligned(*const T) -> bool +``` + +Calls to `is_aligned` are expanded to + +```rust +(ptr as usize) % core::mem::align_of::() == 0 +``` + +on all current platforms. + +An Rust backend may decide to expand it differently, but it needs to hold up the +guarantee from `core::mem::align_of`: + +> Every valid address of a value of the type `T` must be a multiple of this number. + +The intrinsic may be stricter than this and conservatively return `false` for +arbitrary platform specific reasons. + +The intrinsic *must* return `true` for pointers to stack or heap allocations +of the type. This means `is_aligned::(Box::::new(t).to_ptr())` is always +true. The same goes for `is_aligned::(&t)`. + +The intrinsic is exported in `core::ptr` and `std::ptr` as a safe `is_aligned` +function with the same signature. It is safe to call, since the pointer is +not dereferenced and no operation is applied to it which could overflow. + +# How We Teach This +[how-we-teach-this]: #how-we-teach-this + +On most platforms alignment is a well known concept independent of Rust. The +important thing that has to be taught about this intrinsic is that it may not +ever return "true" no matter which operations are applied to the pointer. The +only guaranteed way to make the intrinsic return to is to give it a non dangling +pointer to an object of the type given to the intrinsic. + +This means that `while !is_aligned(ptr) { ... }` should be considered a +potential infinite loop. + +A lint could be implemented which detects hand-written alignment checks and +suggests to use the `is_aligned` function instead. + +# Drawbacks +[drawbacks]: #drawbacks + +None known to the author. + +# Alternatives +[alternatives]: #alternatives + +Miri could intercept calls to functions known to do alignment checks on pointers +and roll its own implementation for them. This doesn't scale well and is prone +to errors due to code duplication. + +# Unresolved questions +[unresolved]: #unresolved-questions + +Should the guarantees be even lower? Calling the intrinsic on the same pointer +twice could suddenly yield `false` even if it yielded `true` before. Since it +should only be used for optimizations this would not be an issue, but be even +more suprising behaviour for no benefit known to the auther. From 9c8f0237572804d0b058edc1659805f4b75b7125 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 4 Jul 2017 13:01:24 +0200 Subject: [PATCH 2/8] Address RFC comments --- text/0000-is-aligned-intrinsic.md | 76 +++++++++++++++++-------------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/text/0000-is-aligned-intrinsic.md b/text/0000-is-aligned-intrinsic.md index e93dd713138..3be52a8f0c9 100644 --- a/text/0000-is-aligned-intrinsic.md +++ b/text/0000-is-aligned-intrinsic.md @@ -1,4 +1,4 @@ -- Feature Name: is_aligned_intrinsic +- Feature Name: alignto_intrinsic - Start Date: 2017-06-20 - RFC PR: (leave this empty) - Rust Issue: (leave this empty) @@ -6,10 +6,12 @@ # Summary [summary]: #summary -Add an intrinsic (`fn is_aligned(*const T) -> bool`) which returns `true` if -a read through the pointer would not fail or suffer from slowdown due to the -alignment of the pointer. There are no further guarantees given about the pointer -it is perfectly valid for `is_aligned(0 as *const T)` to return `true`. +Add an intrinsic (`fn alignto(&[u8]) -> (&[u8], &[T], &[u8])`) which returns +a slice to correctly aligned objects of type `T` and the unalignable parts +before and after as `[u8]`. Also add an `unsafe` library function of the same +name and signature under `core::mem` and `std::mem`. The function is unsafe, +because it essentially contains a `transmute<[u8; N], T>` if one reads from +the aligned slice. # Motivation [motivation]: #motivation @@ -33,50 +35,59 @@ be able to convert a `[u8]` into a `str`. # Detailed design [design]: #detailed-design -Add a new intrinsic `is_aligned` with the following signature: +Add two new intrinsics `alignto` and `alignto_mut` with the following signature: ```rust -fn is_aligned(*const T) -> bool +fn alignto(&[u8]) -> (&[u8], &[T], &[u8]) { /**/ } +fn alignto_mut(&mut [u8]) -> (&mut [u8], &mut [T], &mut [u8]) { /**/ } ``` -Calls to `is_aligned` are expanded to +Calls to `alignto` are expanded to ```rust -(ptr as usize) % core::mem::align_of::() == 0 +fn alignto(slice: &[u8]) -> (&[u8], &[T], &[u8]) { + let align = core::mem::align_of::(); + let start = slice.as_ptr() as usize; + let offset = start % align; + let (head, tail) = if offset == 0 { + (&[], slice) + } else { + slice.split_at(core::cmp::max(slice.len(), align - offset)) + }; + let count = tail.len() / core::mem::size_of::(); + let mid = core::slice::from_raw_parts::(tail.as_ptr() as *const _, count); + let tail = &tail[count * core::mem::size_of::()..]; + (head, mid, tail) +} ``` -on all current platforms. +on all current platforms. `alignto_mut` is expanded accordingly. -An Rust backend may decide to expand it differently, but it needs to hold up the -guarantee from `core::mem::align_of`: +A Rust backend may decide to expand it as -> Every valid address of a value of the type `T` must be a multiple of this number. - -The intrinsic may be stricter than this and conservatively return `false` for -arbitrary platform specific reasons. +```rust +fn alignto(slice: &[u8]) -> (&[u8], &[T], &[u8]) { + (slice, &[], &[]) +} +``` -The intrinsic *must* return `true` for pointers to stack or heap allocations -of the type. This means `is_aligned::(Box::::new(t).to_ptr())` is always -true. The same goes for `is_aligned::(&t)`. +Users of the intrinsics and functions must process all the returned slices and +cannot rely on any behaviour except that the `&[T]`'s elements are correctly +aligned. -The intrinsic is exported in `core::ptr` and `std::ptr` as a safe `is_aligned` -function with the same signature. It is safe to call, since the pointer is -not dereferenced and no operation is applied to it which could overflow. +The intrinsics have `unsafe` functions wrappers in `core::mem` and `std::mem` +which simply forward to the intrinsics and which can be stabilized in the future. # How We Teach This [how-we-teach-this]: #how-we-teach-this On most platforms alignment is a well known concept independent of Rust. The -important thing that has to be taught about this intrinsic is that it may not -ever return "true" no matter which operations are applied to the pointer. The -only guaranteed way to make the intrinsic return to is to give it a non dangling -pointer to an object of the type given to the intrinsic. - -This means that `while !is_aligned(ptr) { ... }` should be considered a -potential infinite loop. +important thing that has to be taught about these intrinsics is that they may not +ever actually yield an aligned slice. The functions and intrinsics may only be +used for optimizations. A lint could be implemented which detects hand-written alignment checks and -suggests to use the `is_aligned` function instead. +suggests to use the `alignto` function instead. # Drawbacks [drawbacks]: #drawbacks @@ -93,7 +104,4 @@ to errors due to code duplication. # Unresolved questions [unresolved]: #unresolved-questions -Should the guarantees be even lower? Calling the intrinsic on the same pointer -twice could suddenly yield `false` even if it yielded `true` before. Since it -should only be used for optimizations this would not be an issue, but be even -more suprising behaviour for no benefit known to the auther. +None From 6fccc7b227a0ec9385ed1eaec163b63ea2c031ae Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Sun, 16 Jul 2017 14:53:07 +0200 Subject: [PATCH 3/8] Address comments --- text/0000-is-aligned-intrinsic.md | 250 +++++++++++++++++++++++++----- 1 file changed, 215 insertions(+), 35 deletions(-) diff --git a/text/0000-is-aligned-intrinsic.md b/text/0000-is-aligned-intrinsic.md index 3be52a8f0c9..0bb0f74ad7f 100644 --- a/text/0000-is-aligned-intrinsic.md +++ b/text/0000-is-aligned-intrinsic.md @@ -6,12 +6,15 @@ # Summary [summary]: #summary -Add an intrinsic (`fn alignto(&[u8]) -> (&[u8], &[T], &[u8])`) which returns -a slice to correctly aligned objects of type `T` and the unalignable parts -before and after as `[u8]`. Also add an `unsafe` library function of the same -name and signature under `core::mem` and `std::mem`. The function is unsafe, -because it essentially contains a `transmute<[u8; N], T>` if one reads from -the aligned slice. +Add an intrinsic (`fn alignto(ptr: *const (), align: usize) -> usize`) +which returns the number of bytes that need to be skipped in order to correctly align the +pointer `ptr` to `align`. + +Also add an `unsafe fn alignto(&[U]) -> (&[U], &[T], &[U])` library function +under `core::mem` and `std::mem` that simplifies the common use case, returning +the unaligned prefix, the aligned center part and the unaligned trailing elements. +The function is unsafe because it essentially contains a `transmute<[U; N], T>` +if one reads from the aligned slice. # Motivation [motivation]: #motivation @@ -20,7 +23,7 @@ The standard library (and most likely many crates) use code like ```rust let is_aligned = (ptr as usize) & ((1 << (align - 1)) - 1) == 0; -let is_2_byte_aligned = ((ptr as usize + index) & (usize_bytes - 1)) == 0; +let is_2_word_aligned = ((ptr as usize + index) & (usize_bytes - 1)) == 0; let is_t_aligned = ((ptr as usize) % std::mem::align_of::()) == 0; ``` @@ -35,59 +38,232 @@ be able to convert a `[u8]` into a `str`. # Detailed design [design]: #detailed-design -Add two new intrinsics `alignto` and `alignto_mut` with the following signature: +## supporting intrinsic + +Add a new intrinsic + +```rust +fn alignto(ptr: *const (), align: usize) -> usize; +``` + +which takes an arbitrary pointer it never reads from and a desired alignment +and returns the number of bytes that the pointer needs to be offset in order +to make it aligned to the desired alignment. It is perfectly valid for an +implementation to always yield `usize::max_value()` to signal that the pointer +cannot be aligned. Since the caller needs to check whether the returned offset +would be in-bounds of the allocation that the pointer points into, returning +`usize::max_value()` will never be in-bounds of the allocation and therefor +the caller cannot act upon the returned offset. + +Most implementations will expand this intrinsic to + +```rust +fn alignto(ptr: *const (), align: usize) -> usize { + align - (ptr as usize % align) +} +``` + +## standard library functions + +Add a new method `alignto` to `*const T` and `*mut T`, which forwards to the +`alignto` + +Add two new functions `alignto` and `alignto_mut` to `core::mem` and `std::mem` +with the following signature: ```rust -fn alignto(&[u8]) -> (&[u8], &[T], &[u8]) { /**/ } -fn alignto_mut(&mut [u8]) -> (&mut [u8], &mut [T], &mut [u8]) { /**/ } +unsafe fn alignto(&[U]) -> (&[U], &[T], &[U]) { /**/ } +unsafe fn alignto_mut(&mut [U]) -> (&mut [U], &mut [T], &mut [U]) { /**/ } ``` Calls to `alignto` are expanded to ```rust -fn alignto(slice: &[u8]) -> (&[u8], &[T], &[u8]) { - let align = core::mem::align_of::(); - let start = slice.as_ptr() as usize; - let offset = start % align; - let (head, tail) = if offset == 0 { - (&[], slice) +unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { + if size_of::() % size_of::() == 0 { + let align = core::mem::align_of::(); + let size = core::mem::size_of::(); + let source_size = core::mem::size_of::(); + // number of bytes that need to be skipped until the pointer is aligned + let offset = core::intrinsics::alignto(slice.as_ptr(), align); + // if `align_of::() <= align_of::()`, or if pointer is accidentally aligned, then `offset == 0` + // + // due to `size_of::() % size_of::() == 0`, + // the fact that `size_of::() > align_of::()`, + // and the fact that `align_of::() > align_of::()` if `offset != 0` we know + // that `offset % source_size == 0` + let head_count = offset / source_size; + let split_position = core::cmp::max(slice.len(), head_count); + let (head, tail) = slice.split_at(split_position); + // might be zero if not enough elements + let mid_count = tail.len() * source_size / size; + let mid = core::slice::from_raw_parts::(tail.as_ptr() as *const _, mid_count); + let tail = &tail[mid_count * core::mem::size_of::()..]; + (head, mid, tail) } else { - slice.split_at(core::cmp::max(slice.len(), align - offset)) - }; - let count = tail.len() / core::mem::size_of::(); - let mid = core::slice::from_raw_parts::(tail.as_ptr() as *const _, count); - let tail = &tail[count * core::mem::size_of::()..]; - (head, mid, tail) + // can't properly fit a T into a sequence of `U` + // FIXME: use GCD(size_of::(), size_of::()) as minimum `mid` size + (slice, &[], &[]) + } } ``` on all current platforms. `alignto_mut` is expanded accordingly. -A Rust backend may decide to expand it as +Any backend may choose to always produce the following implementation, no matter +the given arguments, since the intrinsic is purely an optimization aid. ```rust -fn alignto(slice: &[u8]) -> (&[u8], &[T], &[u8]) { +unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { (slice, &[], &[]) } ``` -Users of the intrinsics and functions must process all the returned slices and +Users of the functions must process all the returned slices and cannot rely on any behaviour except that the `&[T]`'s elements are correctly -aligned. - -The intrinsics have `unsafe` functions wrappers in `core::mem` and `std::mem` -which simply forward to the intrinsics and which can be stabilized in the future. +aligned and that all bytes of the original slice are present in the resulting +three slices. +The memory behind the reference must be treated as if created by +`transmute<[U; N], T>` with all the dangers that brings. # How We Teach This [how-we-teach-this]: #how-we-teach-this -On most platforms alignment is a well known concept independent of Rust. The -important thing that has to be taught about these intrinsics is that they may not -ever actually yield an aligned slice. The functions and intrinsics may only be -used for optimizations. +## By example + +On most platforms alignment is a well known concept independent of Rust. +Currently unsafe Rust code doing alignment checks needs to reproduce the known +patterns from C, which are hard to read and prone to errors when modified later. + +### Example 1 (pointers) + +The standard library uses an alignment optimization for quickly +skipping over ascii code during utf8 checking a byte slice. The current code +looks as follows: + +```rust +// Ascii case, try to skip forward quickly. +// When the pointer is aligned, read 2 words of data per iteration +// until we find a word containing a non-ascii byte. +let ptr = v.as_ptr(); +let align = (ptr as usize + index) & (usize_bytes - 1); + +``` + +One can clearly see the alignment check and the three slice parts. +The first part is the `else` branch of the outer `if align == 0`. +The (aligned) second part is the large `while` loop, and the third +art is the small `while` loop. + +With the `alignto` method the code can be changed to + +```rust +let ptr = v.as_ptr(); +let align = ptr.offset(index).alignto(usize_bytes); +``` + +## Example 2 (slices) + +The `memchr` impl in the standard library explicitly uses the three phases of +the `align_to` functions: + +```rust +// Split `text` in three parts +// - unaligned initial part, before the first word aligned address in text +// - body, scan by 2 words at a time +// - the last remaining part, < 2 word size +let len = text.len(); +let ptr = text.as_ptr(); +let usize_bytes = mem::size_of::(); + +// search up to an aligned boundary +let align = (ptr as usize) & (usize_bytes- 1); +let mut offset; +if align > 0 { + offset = cmp::min(usize_bytes - align, len); + if let Some(index) = text[..offset].iter().position(|elt| *elt == x) { + return Some(index); + } +} else { + offset = 0; +} + +// search the body of the text +let repeated_x = repeat_byte(x); + +if len >= 2 * usize_bytes { + while offset <= len - 2 * usize_bytes { + unsafe { + let u = *(ptr.offset(offset as isize) as *const usize); + let v = *(ptr.offset((offset + usize_bytes) as isize) as *const usize); + + // break if there is a matching byte + let zu = contains_zero_byte(u ^ repeated_x); + let zv = contains_zero_byte(v ^ repeated_x); + if zu || zv { + break; + } + } + offset += usize_bytes * 2; + } +} + +// find the byte after the point the body loop stopped +text[offset..].iter().position(|elt| *elt == x).map(|i| offset + i) +``` + +With the `alignto` function this could be written as + + +```rust +// Split `text` in three parts +// - unaligned initial part, before the first word aligned address in text +// - body, scan by 2 words at a time +// - the last remaining part, < 2 word size +let len = text.len(); +let ptr = text.as_ptr(); +let two_word_bytes = mem::size_of::(); + +// choose alignment depending on platform +#[repr(align = 64)] +struct TwoWord([usize; 2]); + +let (head, mid, tail) = std::mem::align_to(text); + +// search up to an aligned boundary +if let Some(index) = head.iter().position(|elt| *elt == x) { + return Some(index); +} + +// search the body of the text +let repeated_x = repeat_byte(x); + +let position = mid.iter().position(|two| { + // break if there is a matching byte + let zu = contains_zero_byte(two.0[0] ^ repeated_x); + let zv = contains_zero_byte(two.0[1] ^ repeated_x); + zu || zv +}); + +if let Some(index) = position { + let offset = index * two_word_bytes + head.len(); + return text[offset..].iter().position(|elt| *elt == x).map(|i| offset + i) +} + +// find the byte in the trailing unaligned part +tail.iter().position(|elt| *elt == x).map(|i| head.len() + mid.len() + i) +``` + +## Documentation A lint could be implemented which detects hand-written alignment checks and -suggests to use the `alignto` function instead. +suggests to use the `alignto` function instead. + +The `std::mem::align` function's documentation should point to `std::mem::alignto` +in order to increase the visibility of the function. The documentation of +`std::mem::align` should note that it is unidiomatic to manually align pointers, +since that might not be supported on all platforms and is prone to implementation +errors. # Drawbacks [drawbacks]: #drawbacks @@ -97,6 +273,8 @@ None known to the author. # Alternatives [alternatives]: #alternatives +## Duplicate functions without optimizations for miri + Miri could intercept calls to functions known to do alignment checks on pointers and roll its own implementation for them. This doesn't scale well and is prone to errors due to code duplication. @@ -104,4 +282,6 @@ to errors due to code duplication. # Unresolved questions [unresolved]: #unresolved-questions -None +* produce a lint in case `sizeof() % sizeof() != 0` and in case the expansion + is not part of a monomorphisation, since in that case `alignto` is statically + known to never be effective From 820df39b5e6b3719ff9779115ed23121fa1dfda5 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 17 Jul 2017 14:28:04 +0200 Subject: [PATCH 4/8] Address next round of comments --- text/0000-is-aligned-intrinsic.md | 89 +++++++++++++++++-------------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/text/0000-is-aligned-intrinsic.md b/text/0000-is-aligned-intrinsic.md index 0bb0f74ad7f..d63a10cb3fb 100644 --- a/text/0000-is-aligned-intrinsic.md +++ b/text/0000-is-aligned-intrinsic.md @@ -6,15 +6,17 @@ # Summary [summary]: #summary -Add an intrinsic (`fn alignto(ptr: *const (), align: usize) -> usize`) +Add an intrinsic (`fn align_offset(ptr: *const (), align: usize) -> usize`) which returns the number of bytes that need to be skipped in order to correctly align the pointer `ptr` to `align`. +The intrinsic is reexported as a method on `*const T` and `*mut T`. + Also add an `unsafe fn alignto(&[U]) -> (&[U], &[T], &[U])` library function under `core::mem` and `std::mem` that simplifies the common use case, returning the unaligned prefix, the aligned center part and the unaligned trailing elements. -The function is unsafe because it essentially contains a `transmute<[U; N], T>` -if one reads from the aligned slice. +The function is unsafe because it produces a `&T` to the memory location of a `U`, +which might expose padding bytes or violate invariants of `T` or `U`. # Motivation [motivation]: #motivation @@ -43,7 +45,7 @@ be able to convert a `[u8]` into a `str`. Add a new intrinsic ```rust -fn alignto(ptr: *const (), align: usize) -> usize; +fn align_offset(ptr: *const (), align: usize) -> usize; ``` which takes an arbitrary pointer it never reads from and a desired alignment @@ -55,18 +57,30 @@ would be in-bounds of the allocation that the pointer points into, returning `usize::max_value()` will never be in-bounds of the allocation and therefor the caller cannot act upon the returned offset. +It might be expected that the maximum offset returned is `align - 1`, but as +the motivation of the rfc states, `miri` cannot guarantee that a pointer can +be aligned irrelevant of the operations done on it. + Most implementations will expand this intrinsic to ```rust -fn alignto(ptr: *const (), align: usize) -> usize { - align - (ptr as usize % align) +fn align_offset(ptr: *const (), align: usize) -> usize { + let offset = ptr as usize % align; + if offset == 0 { + 0 + } else { + align - offset + } } ``` +The `align` parameter must be a power of two and smaller than `2^32`. +Usually one should pass in the result of an `align_of` call. + ## standard library functions -Add a new method `alignto` to `*const T` and `*mut T`, which forwards to the -`alignto` +Add a new method `align_offset` to `*const T` and `*mut T`, which forwards to the +`align_offset` intrinsic. Add two new functions `alignto` and `alignto_mut` to `core::mem` and `std::mem` with the following signature: @@ -76,16 +90,18 @@ unsafe fn alignto(&[U]) -> (&[U], &[T], &[U]) { /**/ } unsafe fn alignto_mut(&mut [U]) -> (&mut [U], &mut [T], &mut [U]) { /**/ } ``` -Calls to `alignto` are expanded to +`alignto` can be implemented as ```rust unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { + use core::mem::{size_of, align_of}; + assert!(size_of::() != 0 && size_of::() != 0, "don't use `alignto` with zsts"); if size_of::() % size_of::() == 0 { - let align = core::mem::align_of::(); - let size = core::mem::size_of::(); - let source_size = core::mem::size_of::(); + let align = align_of::(); + let size = size_of::(); + let source_size = size_of::(); // number of bytes that need to be skipped until the pointer is aligned - let offset = core::intrinsics::alignto(slice.as_ptr(), align); + let offset = slice.as_ptr().align_offset(align); // if `align_of::() <= align_of::()`, or if pointer is accidentally aligned, then `offset == 0` // // due to `size_of::() % size_of::() == 0`, @@ -98,7 +114,7 @@ unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { // might be zero if not enough elements let mid_count = tail.len() * source_size / size; let mid = core::slice::from_raw_parts::(tail.as_ptr() as *const _, mid_count); - let tail = &tail[mid_count * core::mem::size_of::()..]; + let tail = &tail[mid_count * size_of::()..]; (head, mid, tail) } else { // can't properly fit a T into a sequence of `U` @@ -110,21 +126,10 @@ unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { on all current platforms. `alignto_mut` is expanded accordingly. -Any backend may choose to always produce the following implementation, no matter -the given arguments, since the intrinsic is purely an optimization aid. - -```rust -unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { - (slice, &[], &[]) -} -``` - Users of the functions must process all the returned slices and cannot rely on any behaviour except that the `&[T]`'s elements are correctly aligned and that all bytes of the original slice are present in the resulting three slices. -The memory behind the reference must be treated as if created by -`transmute<[U; N], T>` with all the dangers that brings. # How We Teach This [how-we-teach-this]: #how-we-teach-this @@ -135,6 +140,15 @@ On most platforms alignment is a well known concept independent of Rust. Currently unsafe Rust code doing alignment checks needs to reproduce the known patterns from C, which are hard to read and prone to errors when modified later. +Thus, whenever pointers need to be manually aligned, the developer is given a +choice: + +1. In the case where processing the initial unaligned bits might abort the entire + process, use `align_offset` +2. If it is likely that all bytes are going to get processed, use `alignto` + * `alignto` has a slight overhead for creating the slices in case not all + slices are used + ### Example 1 (pointers) The standard library uses an alignment optimization for quickly @@ -150,22 +164,20 @@ let align = (ptr as usize + index) & (usize_bytes - 1); ``` -One can clearly see the alignment check and the three slice parts. -The first part is the `else` branch of the outer `if align == 0`. -The (aligned) second part is the large `while` loop, and the third -art is the small `while` loop. - -With the `alignto` method the code can be changed to +With the `align_offset` method the code can be changed to ```rust let ptr = v.as_ptr(); -let align = ptr.offset(index).alignto(usize_bytes); +let align = unsafe { + // the offset is safe, because `index` is guaranteed inbounds + ptr.offset(index).align_offset(usize_bytes); +}; ``` ## Example 2 (slices) The `memchr` impl in the standard library explicitly uses the three phases of -the `align_to` functions: +the `alignto` functions: ```rust // Split `text` in three parts @@ -222,13 +234,8 @@ With the `alignto` function this could be written as // - the last remaining part, < 2 word size let len = text.len(); let ptr = text.as_ptr(); -let two_word_bytes = mem::size_of::(); - -// choose alignment depending on platform -#[repr(align = 64)] -struct TwoWord([usize; 2]); -let (head, mid, tail) = std::mem::align_to(text); +let (head, mid, tail) = std::mem::alignto::<(usize, usize)>(text); // search up to an aligned boundary if let Some(index) = head.iter().position(|elt| *elt == x) { @@ -240,8 +247,8 @@ let repeated_x = repeat_byte(x); let position = mid.iter().position(|two| { // break if there is a matching byte - let zu = contains_zero_byte(two.0[0] ^ repeated_x); - let zv = contains_zero_byte(two.0[1] ^ repeated_x); + let zu = contains_zero_byte(two.0 ^ repeated_x); + let zv = contains_zero_byte(two.1 ^ repeated_x); zu || zv }); From 909ad392e529133ffbd71e9b93c3d704310cc166 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 16 Aug 2017 15:47:21 +0200 Subject: [PATCH 5/8] Rename `alignto` to `align_to` --- text/0000-is-aligned-intrinsic.md | 34 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/text/0000-is-aligned-intrinsic.md b/text/0000-is-aligned-intrinsic.md index d63a10cb3fb..b75e6fe1923 100644 --- a/text/0000-is-aligned-intrinsic.md +++ b/text/0000-is-aligned-intrinsic.md @@ -1,4 +1,4 @@ -- Feature Name: alignto_intrinsic +- Feature Name: align_to_intrinsic - Start Date: 2017-06-20 - RFC PR: (leave this empty) - Rust Issue: (leave this empty) @@ -12,7 +12,7 @@ pointer `ptr` to `align`. The intrinsic is reexported as a method on `*const T` and `*mut T`. -Also add an `unsafe fn alignto(&[U]) -> (&[U], &[T], &[U])` library function +Also add an `unsafe fn align_to(&[U]) -> (&[U], &[T], &[U])` library function under `core::mem` and `std::mem` that simplifies the common use case, returning the unaligned prefix, the aligned center part and the unaligned trailing elements. The function is unsafe because it produces a `&T` to the memory location of a `U`, @@ -82,20 +82,20 @@ Usually one should pass in the result of an `align_of` call. Add a new method `align_offset` to `*const T` and `*mut T`, which forwards to the `align_offset` intrinsic. -Add two new functions `alignto` and `alignto_mut` to `core::mem` and `std::mem` +Add two new functions `align_to` and `align_to_mut` to `core::mem` and `std::mem` with the following signature: ```rust -unsafe fn alignto(&[U]) -> (&[U], &[T], &[U]) { /**/ } -unsafe fn alignto_mut(&mut [U]) -> (&mut [U], &mut [T], &mut [U]) { /**/ } +unsafe fn align_to(&[U]) -> (&[U], &[T], &[U]) { /**/ } +unsafe fn align_to_mut(&mut [U]) -> (&mut [U], &mut [T], &mut [U]) { /**/ } ``` -`alignto` can be implemented as +`align_to` can be implemented as ```rust -unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { +unsafe fn align_to(slice: &[U]) -> (&[U], &[T], &[U]) { use core::mem::{size_of, align_of}; - assert!(size_of::() != 0 && size_of::() != 0, "don't use `alignto` with zsts"); + assert!(size_of::() != 0 && size_of::() != 0, "don't use `align_to` with zsts"); if size_of::() % size_of::() == 0 { let align = align_of::(); let size = size_of::(); @@ -124,7 +124,7 @@ unsafe fn alignto(slice: &[U]) -> (&[U], &[T], &[U]) { } ``` -on all current platforms. `alignto_mut` is expanded accordingly. +on all current platforms. `align_to_mut` is expanded accordingly. Users of the functions must process all the returned slices and cannot rely on any behaviour except that the `&[T]`'s elements are correctly @@ -145,8 +145,8 @@ choice: 1. In the case where processing the initial unaligned bits might abort the entire process, use `align_offset` -2. If it is likely that all bytes are going to get processed, use `alignto` - * `alignto` has a slight overhead for creating the slices in case not all +2. If it is likely that all bytes are going to get processed, use `align_to` + * `align_to` has a slight overhead for creating the slices in case not all slices are used ### Example 1 (pointers) @@ -177,7 +177,7 @@ let align = unsafe { ## Example 2 (slices) The `memchr` impl in the standard library explicitly uses the three phases of -the `alignto` functions: +the `align_to` functions: ```rust // Split `text` in three parts @@ -224,7 +224,7 @@ if len >= 2 * usize_bytes { text[offset..].iter().position(|elt| *elt == x).map(|i| offset + i) ``` -With the `alignto` function this could be written as +With the `align_to` function this could be written as ```rust @@ -235,7 +235,7 @@ With the `alignto` function this could be written as let len = text.len(); let ptr = text.as_ptr(); -let (head, mid, tail) = std::mem::alignto::<(usize, usize)>(text); +let (head, mid, tail) = std::mem::align_to::<(usize, usize)>(text); // search up to an aligned boundary if let Some(index) = head.iter().position(|elt| *elt == x) { @@ -264,9 +264,9 @@ tail.iter().position(|elt| *elt == x).map(|i| head.len() + mid.len() + i) ## Documentation A lint could be implemented which detects hand-written alignment checks and -suggests to use the `alignto` function instead. +suggests to use the `align_to` function instead. -The `std::mem::align` function's documentation should point to `std::mem::alignto` +The `std::mem::align` function's documentation should point to `std::mem::align_to` in order to increase the visibility of the function. The documentation of `std::mem::align` should note that it is unidiomatic to manually align pointers, since that might not be supported on all platforms and is prone to implementation @@ -290,5 +290,5 @@ to errors due to code duplication. [unresolved]: #unresolved-questions * produce a lint in case `sizeof() % sizeof() != 0` and in case the expansion - is not part of a monomorphisation, since in that case `alignto` is statically + is not part of a monomorphisation, since in that case `align_to` is statically known to never be effective From e49877c23a012ee3845885d4457e3b5a74e0af53 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 21 Aug 2017 11:19:01 +0200 Subject: [PATCH 6/8] Address comments and move `align_to` to `slice` module --- text/0000-is-aligned-intrinsic.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/text/0000-is-aligned-intrinsic.md b/text/0000-is-aligned-intrinsic.md index b75e6fe1923..13256bab400 100644 --- a/text/0000-is-aligned-intrinsic.md +++ b/text/0000-is-aligned-intrinsic.md @@ -13,7 +13,7 @@ pointer `ptr` to `align`. The intrinsic is reexported as a method on `*const T` and `*mut T`. Also add an `unsafe fn align_to(&[U]) -> (&[U], &[T], &[U])` library function -under `core::mem` and `std::mem` that simplifies the common use case, returning +under `core::slice` and `std::slice` that simplifies the common use case, returning the unaligned prefix, the aligned center part and the unaligned trailing elements. The function is unsafe because it produces a `&T` to the memory location of a `U`, which might expose padding bytes or violate invariants of `T` or `U`. @@ -82,7 +82,7 @@ Usually one should pass in the result of an `align_of` call. Add a new method `align_offset` to `*const T` and `*mut T`, which forwards to the `align_offset` intrinsic. -Add two new functions `align_to` and `align_to_mut` to `core::mem` and `std::mem` +Add two new functions `align_to` and `align_to_mut` to `core::slice` and `std::slice` with the following signature: ```rust @@ -170,7 +170,7 @@ With the `align_offset` method the code can be changed to let ptr = v.as_ptr(); let align = unsafe { // the offset is safe, because `index` is guaranteed inbounds - ptr.offset(index).align_offset(usize_bytes); + ptr.offset(index).align_offset(usize_bytes) }; ``` @@ -235,7 +235,7 @@ With the `align_to` function this could be written as let len = text.len(); let ptr = text.as_ptr(); -let (head, mid, tail) = std::mem::align_to::<(usize, usize)>(text); +let (head, mid, tail) = std::slice::align_to::<(usize, usize), _>(text); // search up to an aligned boundary if let Some(index) = head.iter().position(|elt| *elt == x) { @@ -263,10 +263,10 @@ tail.iter().position(|elt| *elt == x).map(|i| head.len() + mid.len() + i) ## Documentation -A lint could be implemented which detects hand-written alignment checks and +A lint could be added to `clippy` which detects hand-written alignment checks and suggests to use the `align_to` function instead. -The `std::mem::align` function's documentation should point to `std::mem::align_to` +The `std::mem::align` function's documentation should point to `std::slice::align_to` in order to increase the visibility of the function. The documentation of `std::mem::align` should note that it is unidiomatic to manually align pointers, since that might not be supported on all platforms and is prone to implementation From f5293d6c71aa602243cd8c00843a0533ef703a7e Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 23 Aug 2017 10:07:40 +0200 Subject: [PATCH 7/8] Make `align_to` an inherent method --- text/0000-is-aligned-intrinsic.md | 58 ++++++++++++++++--------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/text/0000-is-aligned-intrinsic.md b/text/0000-is-aligned-intrinsic.md index 13256bab400..bf853552652 100644 --- a/text/0000-is-aligned-intrinsic.md +++ b/text/0000-is-aligned-intrinsic.md @@ -12,10 +12,10 @@ pointer `ptr` to `align`. The intrinsic is reexported as a method on `*const T` and `*mut T`. -Also add an `unsafe fn align_to(&[U]) -> (&[U], &[T], &[U])` library function -under `core::slice` and `std::slice` that simplifies the common use case, returning +Also add an `unsafe fn align_to(&self) -> (&[T], &[U], &[T])` method to `[T]`. +The method simplifies the common use case, returning the unaligned prefix, the aligned center part and the unaligned trailing elements. -The function is unsafe because it produces a `&T` to the memory location of a `U`, +The function is unsafe because it produces a `&U` to the memory location of a `T`, which might expose padding bytes or violate invariants of `T` or `U`. # Motivation @@ -82,44 +82,46 @@ Usually one should pass in the result of an `align_of` call. Add a new method `align_offset` to `*const T` and `*mut T`, which forwards to the `align_offset` intrinsic. -Add two new functions `align_to` and `align_to_mut` to `core::slice` and `std::slice` -with the following signature: +Add two new methods `align_to` and `align_to_mut` to the slice type. ```rust -unsafe fn align_to(&[U]) -> (&[U], &[T], &[U]) { /**/ } -unsafe fn align_to_mut(&mut [U]) -> (&mut [U], &mut [T], &mut [U]) { /**/ } +impl [T] { + /* ... other methods ... */ + unsafe fn align_to(&self) -> (&[T], &[U], &[T]) { /**/ } + unsafe fn align_to_mut(&mut self) -> (&mut [T], &mut [U], &mut [T]) { /**/ } +} ``` `align_to` can be implemented as ```rust -unsafe fn align_to(slice: &[U]) -> (&[U], &[T], &[U]) { +unsafe fn align_to(&self) -> (&[T], &[U], &[T]) { use core::mem::{size_of, align_of}; - assert!(size_of::() != 0 && size_of::() != 0, "don't use `align_to` with zsts"); - if size_of::() % size_of::() == 0 { - let align = align_of::(); - let size = size_of::(); - let source_size = size_of::(); + assert!(size_of::() != 0 && size_of::() != 0, "don't use `align_to` with zsts"); + if size_of::() % size_of::() == 0 { + let align = align_of::(); + let size = size_of::(); + let source_size = size_of::(); // number of bytes that need to be skipped until the pointer is aligned - let offset = slice.as_ptr().align_offset(align); - // if `align_of::() <= align_of::()`, or if pointer is accidentally aligned, then `offset == 0` + let offset = self.as_ptr().align_offset(align); + // if `align_of::() <= align_of::()`, or if pointer is accidentally aligned, then `offset == 0` // - // due to `size_of::() % size_of::() == 0`, - // the fact that `size_of::() > align_of::()`, - // and the fact that `align_of::() > align_of::()` if `offset != 0` we know + // due to `size_of::() % size_of::() == 0`, + // the fact that `size_of::() > align_of::()`, + // and the fact that `align_of::() > align_of::()` if `offset != 0` we know // that `offset % source_size == 0` let head_count = offset / source_size; - let split_position = core::cmp::max(slice.len(), head_count); - let (head, tail) = slice.split_at(split_position); + let split_position = core::cmp::max(self.len(), head_count); + let (head, tail) = self.split_at(split_position); // might be zero if not enough elements let mid_count = tail.len() * source_size / size; - let mid = core::slice::from_raw_parts::(tail.as_ptr() as *const _, mid_count); - let tail = &tail[mid_count * size_of::()..]; + let mid = core::slice::from_raw_parts::(tail.as_ptr() as *const _, mid_count); + let tail = &tail[mid_count * size_of::()..]; (head, mid, tail) } else { - // can't properly fit a T into a sequence of `U` - // FIXME: use GCD(size_of::(), size_of::()) as minimum `mid` size - (slice, &[], &[]) + // can't properly fit a U into a sequence of `T` + // FIXME: use GCD(size_of::(), size_of::()) as minimum `mid` size + (self, &[], &[]) } } ``` @@ -127,7 +129,7 @@ unsafe fn align_to(slice: &[U]) -> (&[U], &[T], &[U]) { on all current platforms. `align_to_mut` is expanded accordingly. Users of the functions must process all the returned slices and -cannot rely on any behaviour except that the `&[T]`'s elements are correctly +cannot rely on any behaviour except that the `&[U]`'s elements are correctly aligned and that all bytes of the original slice are present in the resulting three slices. @@ -235,7 +237,7 @@ With the `align_to` function this could be written as let len = text.len(); let ptr = text.as_ptr(); -let (head, mid, tail) = std::slice::align_to::<(usize, usize), _>(text); +let (head, mid, tail) = text.align_to::<(usize, usize)>(); // search up to an aligned boundary if let Some(index) = head.iter().position(|elt| *elt == x) { @@ -266,7 +268,7 @@ tail.iter().position(|elt| *elt == x).map(|i| head.len() + mid.len() + i) A lint could be added to `clippy` which detects hand-written alignment checks and suggests to use the `align_to` function instead. -The `std::mem::align` function's documentation should point to `std::slice::align_to` +The `std::mem::align` function's documentation should point to `[T]::align_to` in order to increase the visibility of the function. The documentation of `std::mem::align` should note that it is unidiomatic to manually align pointers, since that might not be supported on all platforms and is prone to implementation From 4253d1325a9f02d03c77c54652cdbbecffed863d Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Mon, 11 Sep 2017 08:43:54 -0700 Subject: [PATCH 8/8] RFC 2043: Add `align_offset` intrinsic and `[T]::align_to` function --- ...0-is-aligned-intrinsic.md => 2043-is-aligned-intrinsic.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename text/{0000-is-aligned-intrinsic.md => 2043-is-aligned-intrinsic.md} (98%) diff --git a/text/0000-is-aligned-intrinsic.md b/text/2043-is-aligned-intrinsic.md similarity index 98% rename from text/0000-is-aligned-intrinsic.md rename to text/2043-is-aligned-intrinsic.md index bf853552652..7e4685d0c48 100644 --- a/text/0000-is-aligned-intrinsic.md +++ b/text/2043-is-aligned-intrinsic.md @@ -1,7 +1,7 @@ - Feature Name: align_to_intrinsic - Start Date: 2017-06-20 -- RFC PR: (leave this empty) -- Rust Issue: (leave this empty) +- RFC PR: https://github.com/rust-lang/rfcs/pull/2043 +- Rust Issue: https://github.com/rust-lang/rust/issues/44488 # Summary [summary]: #summary