libs: Avoid raw pointer comparisons in std::thread

Fixes #225.

Author: RyanGlScott

libs/Patches.md

@@ -238,6 +238,14 @@ into the main commit for that patch, and then the *Update* line can be removed.
   This feature was using an `AtomicPtr<()>` with a cast from function
   pointer to `*mut ()` that we don't support in its initializer.
 
+* Avoid raw pointer comparisons in `std::thread` (last applied: January 22, 2026)
+
+  `std::thread` reserves raw pointers with the addresses 0-3 as sentinel
+  values. Instead of checking if a thread's pointer is not a sentinel value by
+  seeing if it is larger than the sentinel with the largest address, we instead
+  check if the pointer is not equal to each of the individual sentinel values.
+  See also the "Avoid raw pointer comparisons" note below.
+
 # Notes
 
 This section contains more detailed notes about why certain patches are written
@@ -260,3 +268,48 @@ this actually happened.)
 
 As a safeguard, we mark all custom hook functions as `#[inline(never)]` to
 ensure that they persist when optimizations are applied.
+
+## Avoid raw pointer comparisons
+
+We avoid using `PartialOrd`-based comparisons with raw pointers values, e.g.,
+
+```rs
+fn f(p: *const ()) {
+    if (p > ptr::null()) {
+        ...
+    }
+}
+```
+
+Instead, we prefer using `PartialEq`-based equality checks, e.g.,
+
+```rs
+    if (p != ptr::null()) {
+        ...
+    }
+
+```
+
+The reason for this is because the `PartialOrd` impl for raw pointers compares
+their underlying addresses, but `crucible-mir` pointers do not always have
+addresses. `MirReference_Integer` pointers can treat their underlying integer
+as an address (e.g., `ptr::null()` has the address 0), but other forms of
+`MirReference`s (i.e., _valid_ `MirReference`s) do not have a tangible address,
+so it is not currently possible to compare `MirReference_Integer`s against
+valid `MirReference`s. Attempting to do so will raise a simulation error.
+
+Using the `PartialEq` impl for raw pointers is safer. Instead of raising a
+simulation error, `crucible-mir` will simply return `False` when checking if a
+`MirReference_Integer` is equal to a valid `MirReference`. This is slightly
+unsound, as it is theoretically possible (albeit very unlikely) for valid
+`MirReference`s to have very small addresses. For instance, the expression
+`(0 .. usize::MAX).any(|addr| addr as *const T == ptr)` will return `true`
+(where `ptr` is a valid pointer) in normal execution of Rust code, as all
+pointers have an address that lies somewhere within the range of a `usize`
+value. In `crucible-mir`, however, this will return `false`, since a valid
+pointer will never be equal to a pointer cast from an integer.
+
+Again, it is worth emphasizing that this unsoundness poses a very small risk in
+practice, as the vast majority of pointers are valid ones. For instance, we use
+the `PartialEq` impl for raw pointers to great effect in the internals of
+`std::thread`, where most threads are backed by valid raw pointers.

libs/std/src/thread/current.rs

@@ -169,7 +169,7 @@ where
     F: FnOnce(Option<&Thread>) -> R,
 {
     let current = CURRENT.get();
-    if current > DESTROYED {
+    if current != NONE && current != BUSY && current != DESTROYED {
         // SAFETY: `Arc` does not contain interior mutability, so it does not
         // matter that the address of the handle might be different depending
         // on where this is called.
@@ -187,7 +187,7 @@ where
 /// handle to allow thread parking in nearly all situations.
 pub(crate) fn current_or_unnamed() -> Thread {
     let current = CURRENT.get();
-    if current > DESTROYED {
+    if current != NONE && current != BUSY && current != DESTROYED {
         unsafe {
             let current = ManuallyDrop::new(Thread::from_raw(current));
             (*current).clone()
@@ -222,7 +222,7 @@ pub(crate) fn current_or_unnamed() -> Thread {
 #[stable(feature = "rust1", since = "1.0.0")]
 pub fn current() -> Thread {
     let current = CURRENT.get();
-    if current > DESTROYED {
+    if current != NONE && current != BUSY && current != DESTROYED {
         unsafe {
             let current = ManuallyDrop::new(Thread::from_raw(current));
             (*current).clone()
@@ -279,7 +279,7 @@ fn init_current(current: *mut ()) -> Thread {
 /// handle.
 pub(crate) fn drop_current() {
     let current = CURRENT.get();
-    if current > DESTROYED {
+    if current != NONE && current != BUSY && current != DESTROYED {
         unsafe {
             CURRENT.set(DESTROYED);
             drop(Thread::from_raw(current));