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.

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

core: use compare_bytes for more slice element types #128495

Merged
merged 3 commits into from
Sep 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
25 changes: 19 additions & 6 deletions library/core/src/slice/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
use super::{from_raw_parts, memchr};
use crate::cmp::{self, BytewiseEq, Ordering};
use crate::intrinsics::compare_bytes;
use crate::mem;
use crate::num::NonZero;
use crate::{ascii, mem};

#[stable(feature = "rust1", since = "1.0.0")]
impl<T, U> PartialEq<[U]> for [T]
Expand Down Expand Up @@ -182,19 +183,31 @@ impl<A: Ord> SliceOrd for A {
}
}

// The type should be treated as an unsigned byte for comparisons.
#[rustc_specialization_trait]
unsafe trait UnsignedByte {}
Copy link
Member

Choose a reason for hiding this comment

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

I know it's a module-private thing, but can you elaborate on the name and add # Safety for the trait? Just "unsigned byte" could accidentally be interpreted as being more a layout thing, where say Rev<u8> is arguably an "unsigned byte" but definitely shouldn't be implementing this.

I see above it's BytewiseEq, so maybe UnsignedBytewiseOrd here or something?


unsafe impl UnsignedByte for bool {}
unsafe impl UnsignedByte for u8 {}
unsafe impl UnsignedByte for NonZero<u8> {}
unsafe impl UnsignedByte for Option<NonZero<u8>> {}
unsafe impl UnsignedByte for ascii::Char {}

// `compare_bytes` compares a sequence of unsigned bytes lexicographically.
// this matches the order we want for [u8], but no others (not even [i8]).
impl SliceOrd for u8 {
impl<A: Ord + UnsignedByte> SliceOrd for A {
#[inline]
fn compare(left: &[Self], right: &[Self]) -> Ordering {
// Since the length of a slice is always less than or equal to isize::MAX, this never underflows.
let diff = left.len() as isize - right.len() as isize;
// This comparison gets optimized away (on x86_64 and ARM) because the subtraction updates flags.
let len = if left.len() < right.len() { left.len() } else { right.len() };
let left = left.as_ptr().cast();
let right = right.as_ptr().cast();
// SAFETY: `left` and `right` are references and are thus guaranteed to be valid.
// We use the minimum of both lengths which guarantees that both regions are
// valid for reads in that interval.
let mut order = unsafe { compare_bytes(left.as_ptr(), right.as_ptr(), len) as isize };
// `UnsignedByte` is only implemented for types that are valid u8s. We use the
// minimum of both lengths which guarantees that both regions are valid for reads
// in that interval.
let mut order = unsafe { compare_bytes(left, right, len) as isize };
if order == 0 {
order = diff;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: reading memory at ALLOC[0x0..0x10], but memory is uninitialized at [0x4..0x10], and this operation requires initialized memory
--> RUSTLIB/core/src/slice/cmp.rs:LL:CC
|
LL | let mut order = unsafe { compare_bytes(left.as_ptr(), right.as_ptr(), len) as isize };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading memory at ALLOC[0x0..0x10], but memory is uninitialized at [0x4..0x10], and this operation requires initialized memory
LL | let mut order = unsafe { compare_bytes(left, right, len) as isize };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading memory at ALLOC[0x0..0x10], but memory is uninitialized at [0x4..0x10], and this operation requires initialized memory
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: reading memory at ALLOC[0x0..0x8], but memory is uninitialized at [0x4..0x8], and this operation requires initialized memory
--> RUSTLIB/core/src/slice/cmp.rs:LL:CC
|
LL | let mut order = unsafe { compare_bytes(left.as_ptr(), right.as_ptr(), len) as isize };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading memory at ALLOC[0x0..0x8], but memory is uninitialized at [0x4..0x8], and this operation requires initialized memory
LL | let mut order = unsafe { compare_bytes(left, right, len) as isize };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reading memory at ALLOC[0x0..0x8], but memory is uninitialized at [0x4..0x8], and this operation requires initialized memory
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Expand Down
Loading