From 114dd2061e0bfa6bc1353d0265389cfaa04d8858 Mon Sep 17 00:00:00 2001 From: Alan Egerton Date: Wed, 12 Jun 2024 13:01:22 +0100 Subject: [PATCH] Un-unsafe the `StableOrd` trait Whilst incorrect implementations of this trait can cause miscompilation, they cannot cause memory unsafety in rustc. --- compiler/rustc_abi/src/lib.rs | 4 +-- .../src/stable_hasher.rs | 33 ++++++++++--------- compiler/rustc_hir/src/hir_id.rs | 4 +-- .../src/dep_graph/dep_node.rs | 2 +- compiler/rustc_session/src/config.rs | 4 +-- compiler/rustc_span/src/def_id.rs | 4 +-- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index b1a17d5a24b..5d8c882803d 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -425,10 +425,10 @@ pub struct Size { raw: u64, } -// Safety: Ord is implement as just comparing numerical values and numerical values +// Ord is implement as just comparing numerical values and numerical values // are not changed by (de-)serialization. #[cfg(feature = "nightly")] -unsafe impl StableOrd for Size { +impl StableOrd for Size { const CAN_USE_UNSTABLE_SORT: bool = true; } diff --git a/compiler/rustc_data_structures/src/stable_hasher.rs b/compiler/rustc_data_structures/src/stable_hasher.rs index b5bdf2e1790..206146cc60d 100644 --- a/compiler/rustc_data_structures/src/stable_hasher.rs +++ b/compiler/rustc_data_structures/src/stable_hasher.rs @@ -238,11 +238,14 @@ pub trait ToStableHashKey { /// The associated constant `CAN_USE_UNSTABLE_SORT` denotes whether /// unstable sorting can be used for this type. Set to true if and /// only if `a == b` implies `a` and `b` are fully indistinguishable. -pub unsafe trait StableOrd: Ord { +/// +/// **Be careful when implementing this trait, as an incorrect +/// implementation can cause miscompilation!** +pub trait StableOrd: Ord { const CAN_USE_UNSTABLE_SORT: bool; } -unsafe impl StableOrd for &T { +impl StableOrd for &T { const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT; } @@ -290,7 +293,7 @@ fn hash_stable(&self, _: &mut CTX, hasher: &mut $crate::stable_hasher::StableHas } } - unsafe impl $crate::stable_hasher::StableOrd for $t { + impl $crate::stable_hasher::StableOrd for $t { const CAN_USE_UNSTABLE_SORT: bool = true; } }; @@ -327,7 +330,7 @@ fn hash_stable(&self, _: &mut CTX, hasher: &mut StableHasher) { } } -unsafe impl StableOrd for Hash128 { +impl StableOrd for Hash128 { const CAN_USE_UNSTABLE_SORT: bool = true; } @@ -392,7 +395,7 @@ fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { } } -unsafe impl StableOrd for (T1, T2) { +impl StableOrd for (T1, T2) { const CAN_USE_UNSTABLE_SORT: bool = T1::CAN_USE_UNSTABLE_SORT && T2::CAN_USE_UNSTABLE_SORT; } @@ -410,7 +413,7 @@ fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { } } -unsafe impl StableOrd for (T1, T2, T3) { +impl StableOrd for (T1, T2, T3) { const CAN_USE_UNSTABLE_SORT: bool = T1::CAN_USE_UNSTABLE_SORT && T2::CAN_USE_UNSTABLE_SORT && T3::CAN_USE_UNSTABLE_SORT; } @@ -431,9 +434,7 @@ fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { } } -unsafe impl StableOrd - for (T1, T2, T3, T4) -{ +impl StableOrd for (T1, T2, T3, T4) { const CAN_USE_UNSTABLE_SORT: bool = T1::CAN_USE_UNSTABLE_SORT && T2::CAN_USE_UNSTABLE_SORT && T3::CAN_USE_UNSTABLE_SORT @@ -530,7 +531,7 @@ fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { } } -unsafe impl StableOrd for &str { +impl StableOrd for &str { const CAN_USE_UNSTABLE_SORT: bool = true; } @@ -541,9 +542,9 @@ fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { } } -// Safety: String comparison only depends on their contents and the +// String comparison only depends on their contents and the // contents are not changed by (de-)serialization. -unsafe impl StableOrd for String { +impl StableOrd for String { const CAN_USE_UNSTABLE_SORT: bool = true; } @@ -570,8 +571,8 @@ fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { } } -// Safety: sort order of bools is not changed by (de-)serialization. -unsafe impl StableOrd for bool { +// sort order of bools is not changed by (de-)serialization. +impl StableOrd for bool { const CAN_USE_UNSTABLE_SORT: bool = true; } @@ -590,8 +591,8 @@ fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { } } -// Safety: the Option wrapper does not add instability to comparison. -unsafe impl StableOrd for Option { +// the Option wrapper does not add instability to comparison. +impl StableOrd for Option { const CAN_USE_UNSTABLE_SORT: bool = T::CAN_USE_UNSTABLE_SORT; } diff --git a/compiler/rustc_hir/src/hir_id.rs b/compiler/rustc_hir/src/hir_id.rs index ac487469507..1ed84fb0968 100644 --- a/compiler/rustc_hir/src/hir_id.rs +++ b/compiler/rustc_hir/src/hir_id.rs @@ -165,9 +165,9 @@ impl ItemLocalId { pub const INVALID: ItemLocalId = ItemLocalId::MAX; } -// Safety: Ord is implement as just comparing the ItemLocalId's numerical +// Ord is implement as just comparing the ItemLocalId's numerical // values and these are not changed by (de-)serialization. -unsafe impl StableOrd for ItemLocalId { +impl StableOrd for ItemLocalId { const CAN_USE_UNSTABLE_SORT: bool = true; } diff --git a/compiler/rustc_query_system/src/dep_graph/dep_node.rs b/compiler/rustc_query_system/src/dep_graph/dep_node.rs index 5f1a03502a7..3fb59ad26f6 100644 --- a/compiler/rustc_query_system/src/dep_graph/dep_node.rs +++ b/compiler/rustc_query_system/src/dep_graph/dep_node.rs @@ -301,7 +301,7 @@ fn to_stable_hash_key(&self, _: &HCX) -> Self::KeyType { self.hash } } -unsafe impl StableOrd for WorkProductId { +impl StableOrd for WorkProductId { // Fingerprint can use unstable (just a tuple of `u64`s), so WorkProductId can as well const CAN_USE_UNSTABLE_SORT: bool = true; } diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index a622f1b577d..d5428df0329 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -491,8 +491,8 @@ pub enum OutputType { DepInfo, } -// Safety: Trivial C-Style enums have a stable sort order across compilation sessions. -unsafe impl StableOrd for OutputType { +// Trivial C-Style enums have a stable sort order across compilation sessions. +impl StableOrd for OutputType { const CAN_USE_UNSTABLE_SORT: bool = true; } diff --git a/compiler/rustc_span/src/def_id.rs b/compiler/rustc_span/src/def_id.rs index 1ac3a817bba..4d534ad8007 100644 --- a/compiler/rustc_span/src/def_id.rs +++ b/compiler/rustc_span/src/def_id.rs @@ -120,8 +120,8 @@ fn default() -> Self { } } -// Safety: `DefPathHash` sort order is not affected (de)serialization. -unsafe impl StableOrd for DefPathHash { +// `DefPathHash` sort order is not affected (de)serialization. +impl StableOrd for DefPathHash { const CAN_USE_UNSTABLE_SORT: bool = true; }