Safety improvements for repr(stabby) enums

CHANGELOG.md

@@ -1,3 +1,14 @@
+# 5.0.0
+- BREAKING CHANGE:
+	- Due to a soundness hole in the previous implementation of `stabby::result::Result`, its representation was overhauled. While it's still technically compatible binary-wise (meaning it still follows the original spec), the soundness hole could possibly lead to UB, so `stabby` will treat them as incompatible.
+- Add support for Rust 1.78:
+	- With 1.78, [Rust merged a breaking change](https://github.com/rust-lang/rust/issues/123281) which impacts `stabby`. This change prevents consts from refering to generics entirely, which was key to `stabby`'s implementation of vtables.
+	- More accurately, it prevents consts from refering to generics that aren't bound by `core::marker::Freeze`, but that trait hasn't been stabilized at the same time as the new error has.
+	- While the team was aware that this would be a breaking change, `crater` failed to report that `stabby` was impacted by the regression, as it tried compiling an obsolete version of `stabby` that could only build with pre-1.77 versions of Rust due to the `u128` ABI-break on x86. This led them to judge that the breaking change was acceptable.
+	- To compensate for this, `stabby` will (for non-nighly `>=1.78` versions of Rust) draw its vtable references from a heap-allocated, lazily populated, global set of vtables. This is in opposition to the `<1.78` and `nightly` behaviour where it'll keep on drawing these vtable references straight from the binary.
+		- From this release onwards, a new priority for `stabby` will be to improve the performance of this behaviour; or better yet find a new way to obtain the previous behaviour that compiles.
+	- While I can't hide that I am very annoyed at this development, I must also state that I understand the Rust Team's choice to ship this breaking change: they considered this potential window for a soundness hole a bug, and even though `crater` didn't report any use of this bug that was unsound, it also failed to report `stabby` as a legitimate user of it. I do wish they'd have waited for `Freeze`'s stabilization to make the breaking change however, as the sound pattern it would prevent, as well as the fact that it couldn't be replicated without `Freeze`, _was_ known.
+
 # 4.0.5
 - Fix for 1.72: `AllocPtr::prefix` is `const fn` from 1.73 onwards rather than 1.72 onwards (@yellowhatter).
 

README.md

@@ -1,6 +1,19 @@
 [![Crates.io (latest)](https://img.shields.io/crates/v/stabby)](https://lib.rs/crates/stabby)
 [![docs.rs](https://img.shields.io/docsrs/stabby)](https://docs.rs/stabby/latest/stabby/)
 
+> [!WARNING]  
+> Due to a breaking change in Rust 1.78, `stabby`'s implementation of trait objects may raise performance issues:
+> - __Only non-nightly, >= 1.78 versions of Rust are affected__
+> - The v-tables backing trait objects are now inserted in a global lock-free set.
+> - This set is leaked: `valgrind` _will_ be angry at you.
+> - This set grows with the number of distinct `(type, trait-set)` pairs. Its current implementation is a vector:
+>   - Lookup is done through linear search (O(n)), which stays the fastest for <100 number of elements.
+>   - Insertion is done by cloning the vector (O(n)) and replacing it atomically, repeating the operation in case of collision.
+>   - Efforts to replace this implementation with immutable b-tree maps are ongoing (they will be scrapped if found to be much slower than the current implementation).
+>
+> This note will be updated as the situation evolves. In the meantime, if your project uses many `stabby`-defined trait objects,
+> I suggest using either `nightly` or a `< 1.78` version of the compiler.
+
 # A Stable ABI for Rust with compact sum-types
 `stabby` is your one-stop-shop to create stable binary interfaces for your shared libraries easily, without having your sum-types (enums) explode in size.
 

examples/dynlinkage/src/main.rs

@@ -14,7 +14,7 @@
 
 #[stabby::import(name = "library")]
 extern "C" {
-    pub fn stable_fn(v: u8);
+    pub fn stable_fn(v: u8) -> stabby::option::Option<()>;
 }
 
 #[stabby::import(canaries = "", name = "library")]

examples/libloading/src/main.rs

@@ -31,7 +31,9 @@ fn main() {
                     .map(|d| d.map(|f| f.unwrap().file_name()).collect::<Vec<_>>())
             )
         });
-        let stable_fn = lib.get_stabbied::<extern "C" fn(u8)>(b"stable_fn").unwrap();
+        let stable_fn = lib
+            .get_stabbied::<extern "C" fn(u8) -> stabby::option::Option<()>>(b"stable_fn")
+            .unwrap();
         let unstable_fn = lib
             .get_canaried::<extern "C" fn(&[u8])>(b"unstable_fn")
             .unwrap();

examples/library/src/lib.rs

@@ -13,8 +13,9 @@
 //
 
 #[stabby::export]
-pub extern "C" fn stable_fn(v: u8) {
-    println!("{v}")
+pub extern "C" fn stable_fn(v: u8) -> stabby::option::Option<()> {
+    println!("{v}");
+    Default::default()
 }
 
 #[stabby::export(canaries)]

stabby-abi/Cargo.toml

@@ -14,7 +14,7 @@
 
 [package]
 name = "stabby-abi"
-version = "4.0.5"
+version = "5.0.0"
 edition = "2021"
 authors = { workspace = true }
 license = { workspace = true }
@@ -38,7 +38,7 @@ abi_stable-channels = ["abi_stable", "abi_stable/channels"]
 # unsafe_wakers = [] # unsafe_wakers is no longer a feature, but a compile option: you can enable them using `RUST_FLAGS='--cfg unsafe_wakers="true"'`
 
 [dependencies]
-stabby-macros = { path = "../stabby-macros/", version = "4.0.5" }
+stabby-macros = { path = "../stabby-macros/", version = "5.0.0" }
 abi_stable = { workspace = true, optional = true }
 libc = { workspace = true, optional = true }
 rustversion = { workspace = true }

stabby-abi/src/alloc/mod.rs

@@ -264,6 +264,13 @@ impl<T, Alloc> AllocPtr<T, Alloc> {
             marker: PhantomData,
         }
     }
+    /// Casts an allocated pointer.
+    pub const fn cast<U>(self) -> AllocPtr<U, Alloc> {
+        AllocPtr {
+            ptr: self.ptr.cast(),
+            marker: PhantomData,
+        }
+    }
     /// The offset between `self.ptr` and the prefix.
     pub const fn prefix_skip() -> usize {
         AllocPrefix::<Alloc>::skip_to::<T>()

stabby-abi/src/enums/err_non_empty.rs

@@ -26,7 +26,7 @@ pub struct Layout<
     ErrFv: IForbiddenValues,
     ErrUb: IBitMask,
     ErrSize: Unsigned,
-    ErrAlign: PowerOf2,
+    ErrAlign: Alignment,
     ErrOffset: Unsigned,
 >(
     core::marker::PhantomData<(
@@ -89,7 +89,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
     > IDeterminantProviderInner
     for Layout<UnionSize, H, OkFv, OkUb, ErrFv, ErrUb, ErrSize, ErrAlign, ErrOffset>
@@ -108,7 +108,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
     > IDeterminantProviderInner
     for Layout<UnionSize, T<Budget>, OkFv, OkUb, ErrFv, ErrUb, ErrSize, ErrAlign, ErrOffset>
@@ -145,7 +145,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
         Offset: Unsigned,
         V: Unsigned,
@@ -179,7 +179,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
         Offset: Unsigned,
         V: Unsigned,
@@ -211,7 +211,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
         Offset: Unsigned,
         V: NonZero,
@@ -242,7 +242,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
     > IDeterminantProviderInner
     for DeterminantProvider<
@@ -271,7 +271,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
     > IDeterminantProviderInner
     for (
@@ -291,7 +291,7 @@ impl<
         ErrFv: IForbiddenValues,
         ErrUb: IBitMask,
         ErrSize: Unsigned,
-        ErrAlign: PowerOf2,
+        ErrAlign: Alignment,
         ErrOffset: Unsigned,
     > IDeterminantProviderInner
     for (

stabby-abi/src/fatptr.rs

@@ -140,7 +140,7 @@ impl<'a, Vt: Copy + 'a> DynRef<'a, Vt> {
     where
         Vt: PartialEq + IConstConstructor<'a, T>,
     {
-        (self.vtable == Vt::VTABLE).then(|| unsafe { self.ptr.as_ref() })
+        (self.vtable == Vt::vtable()).then(|| unsafe { self.ptr.as_ref() })
     }
     /// Downcasts the reference based on its reflection report.
     pub fn stable_downcast<T: crate::IStable, Path>(&self) -> Option<&T>
@@ -267,6 +267,12 @@ impl<'a, P: IPtrOwned + IPtr, Vt: HasDropVt + 'a> Dyn<'a, P, Vt> {
     /// This implies that this downcast will always yield `None` when attempting to downcast
     /// values constructed accross an FFI.
     ///
+    /// Note that the compiler may chose to have multiple copies of the vtable, notably in optimized builds.
+    /// This means that even within a same compile unit, this function may fail to downcast a value even if
+    /// the type should have matched.
+    ///
+    /// In general, you should prefer [`Self::stable_downcast_ref`]
+    ///
     /// # Safety
     /// This may have false positives if all of the following applies:
     /// - `self` was built from `&U`, within the same FFI-boundary,
@@ -280,20 +286,26 @@ impl<'a, P: IPtrOwned + IPtr, Vt: HasDropVt + 'a> Dyn<'a, P, Vt> {
     where
         Vt: PartialEq + Copy + IConstConstructor<'a, T>,
     {
-        (self.vtable == Vt::VTABLE).then(|| unsafe { self.ptr.as_ref() })
+        (self.vtable == Vt::vtable()).then(|| unsafe { self.ptr.as_ref() })
     }
     /// Downcasts the reference based on its reflection report.
     pub fn stable_downcast_ref<T: crate::IStable, Path>(&self) -> Option<&T>
     where
         Vt: TransitiveDeref<crate::vtable::StabbyVtableAny, Path> + IConstConstructor<'a, T>,
     {
-        (self.report() == T::REPORT).then(|| unsafe { self.ptr.as_ref() })
+        (self.id() == T::ID && self.report() == T::REPORT).then(|| unsafe { self.ptr.as_ref() })
     }
     /// Downcasts the mutable reference based on vtable equality.
     ///
     /// This implies that this downcast will always yield `None` when attempting to downcast
     /// values constructed accross an FFI.
     ///
+    /// Note that the compiler may chose to have multiple copies of the vtable, notably in optimized builds.
+    /// This means that even within a same compile unit, this function may fail to downcast a value even if
+    /// the type should have matched.
+    ///
+    /// In general, you should prefer [`Self::stable_downcast_mut`]
+    ///
     /// # Safety
     /// This may have false positives if all of the following applies:
     /// - `self` was built from `&U`, within the same FFI-boundary,
@@ -308,15 +320,15 @@ impl<'a, P: IPtrOwned + IPtr, Vt: HasDropVt + 'a> Dyn<'a, P, Vt> {
         Vt: PartialEq + Copy + IConstConstructor<'a, T>,
         P: IPtrMut,
     {
-        (self.vtable == Vt::VTABLE).then(|| unsafe { self.ptr.as_mut() })
+        (self.vtable == Vt::vtable()).then(|| unsafe { self.ptr.as_mut() })
     }
-    /// Downcasts the reference based on its reflection report.
+    /// Downcasts the mutable reference based on its reflection report.
     pub fn stable_downcast_mut<T: crate::IStable, Path>(&mut self) -> Option<&mut T>
     where
         Vt: TransitiveDeref<crate::vtable::StabbyVtableAny, Path> + IConstConstructor<'a, T>,
         P: IPtrMut,
     {
-        (self.report() == T::REPORT).then(|| unsafe { self.ptr.as_mut() })
+        (self.id() == T::ID && self.report() == T::REPORT).then(|| unsafe { self.ptr.as_mut() })
     }
 }
 
@@ -331,7 +343,7 @@ where
     fn from(value: P) -> Self {
         Self {
             ptr: core::mem::ManuallyDrop::new(value.anonimize()),
-            vtable: Vt::VTABLE,
+            vtable: Vt::vtable(),
             unsend: core::marker::PhantomData,
         }
     }
@@ -350,7 +362,7 @@ impl<'a, T, Vt: Copy + IConstConstructor<'a, T>> From<&'a T> for DynRef<'a, Vt>
         unsafe {
             DynRef {
                 ptr: core::mem::transmute(value),
-                vtable: Vt::VTABLE,
+                vtable: Vt::vtable(),
                 unsend: core::marker::PhantomData,
             }
         }