From 88fda58bf5396667a022461f9d7465f24fd3395d Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 1 Aug 2024 14:38:18 +0200 Subject: [PATCH 1/3] core: use `compare_bytes` for more slice element types --- library/core/src/slice/cmp.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/library/core/src/slice/cmp.rs b/library/core/src/slice/cmp.rs index d19d0eae16671..dd43b58ec0f58 100644 --- a/library/core/src/slice/cmp.rs +++ b/library/core/src/slice/cmp.rs @@ -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 PartialEq<[U]> for [T] @@ -182,19 +183,31 @@ impl SliceOrd for A { } } +// The type should be treated as an unsigned byte for comparisons. +#[rustc_specialization_trait] +unsafe trait UnsignedByte {} + +unsafe impl UnsignedByte for bool {} +unsafe impl UnsignedByte for u8 {} +unsafe impl UnsignedByte for NonZero {} +unsafe impl UnsignedByte for Option> {} +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 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; } From 32d6d0e187c84ec55b8a4aeb4a1bcd267aa5096f Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 1 Aug 2024 15:44:49 +0200 Subject: [PATCH 2/3] bless miri tests --- .../miri/tests/fail/uninit/uninit_alloc_diagnostic.stderr | 4 ++-- .../uninit/uninit_alloc_diagnostic_with_provenance.stderr | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic.stderr b/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic.stderr index 960cae9012484..dfe3ed4b522ab 100644 --- a/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic.stderr +++ b/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic.stderr @@ -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 diff --git a/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic_with_provenance.stderr b/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic_with_provenance.stderr index 5439418f26771..48ce9ae76568a 100644 --- a/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic_with_provenance.stderr +++ b/src/tools/miri/tests/fail/uninit/uninit_alloc_diagnostic_with_provenance.stderr @@ -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 From 8301dd4767ce0d9d6dd1a06ff5f91319a42263e5 Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 12 Aug 2024 10:20:32 +0200 Subject: [PATCH 3/3] core: make documentation clearer, rename slice comparison specialization trait --- library/core/src/slice/cmp.rs | 40 ++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/library/core/src/slice/cmp.rs b/library/core/src/slice/cmp.rs index dd43b58ec0f58..1769612def0a5 100644 --- a/library/core/src/slice/cmp.rs +++ b/library/core/src/slice/cmp.rs @@ -183,30 +183,40 @@ impl SliceOrd for A { } } -// The type should be treated as an unsigned byte for comparisons. +/// Marks that a type should be treated as an unsigned byte for comparisons. +/// +/// # Safety +/// * The type must be readable as an `u8`, meaning it has to have the same +/// layout as `u8` and always be initialized. +/// * For every `x` and `y` of this type, `Ord(x, y)` must return the same +/// value as `Ord::cmp(transmute::<_, u8>(x), transmute::<_, u8>(y))`. #[rustc_specialization_trait] -unsafe trait UnsignedByte {} +unsafe trait UnsignedBytewiseOrd {} -unsafe impl UnsignedByte for bool {} -unsafe impl UnsignedByte for u8 {} -unsafe impl UnsignedByte for NonZero {} -unsafe impl UnsignedByte for Option> {} -unsafe impl UnsignedByte for ascii::Char {} +unsafe impl UnsignedBytewiseOrd for bool {} +unsafe impl UnsignedBytewiseOrd for u8 {} +unsafe impl UnsignedBytewiseOrd for NonZero {} +unsafe impl UnsignedBytewiseOrd for Option> {} +unsafe impl UnsignedBytewiseOrd for ascii::Char {} -// `compare_bytes` compares a sequence of unsigned bytes lexicographically. -impl SliceOrd for A { +// `compare_bytes` compares a sequence of unsigned bytes lexicographically, so +// use it if the requirements for `UnsignedBytewiseOrd` are fulfilled. +impl 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. + // 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. + // 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. - // `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. + // SAFETY: `left` and `right` are references and are thus guaranteed to + // be valid. `UnsignedBytewiseOrd` is only implemented for types that + // are valid u8s and can be compared the same way. 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;