Fix unsoundness in conversion functions

- Fix unsoundness in `project`, `from_(a)rc` and `try_from_(a)rc`
  by adding the missing lifetime constraints.
- Add doctests that will test that the offending examples don't compile
- Bump version to 0.4.0 due to this breaking the public API
- Fix new warnings with newer Rust versions

Author: radekvit

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 0.4.0
+- Fix unsoundness in `Parc::project` and `Prc::project` due to lifetime coersion and a missing
+  lifetime constraint.
+- Fix unsoudness in `Parc::from_arc`, `Parc::try_from_arc`, `Prc::from_rc`, and `Prc::try_from_rc`
+  due to a missing lifetime constraint on the `Arc`/`Rc` parameter.
+
 ## 0.3.0
 - Change the API of `try_*` functions on `Parc` and `Prc` to take functions returning `Result` instead of `Option` and make them return `Result` instead of `Option`.
 

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "pared"
-version = "0.3.0"
+version = "0.4.0"
 authors = ["Radek Vít <[email protected]>"]
 edition = "2021"
 rust-version = "1.56"
@@ -14,6 +14,9 @@ categories = ["data-structures", "memory-management", "no-std", "rust-patterns"]
 default = ["std"]
 std = []
 
+[lints.rust]
+unexpected_cfgs = { level = "warn", check-cfg = ['cfg(coverage_nightly)'] }
+
 [dependencies]
 
 [dev-dependencies]

Cargo.toml

@@ -81,7 +81,7 @@ mod tests {
     #[cfg_attr(coverage_nightly, coverage(off))]
     fn dyn_ptr() {
         // We want to check that the pointers actually ARE compatible
-        #![allow(clippy::vtable_address_comparisons)]
+        #![allow(ambiguous_wide_pointer_comparisons)]
 
         let debug: &dyn core::fmt::Debug = &"Hello!";
         let ptr = TypeErasedPtr::new(debug as *const dyn core::fmt::Debug);
@@ -103,6 +103,6 @@ mod tests {
     #[cfg_attr(coverage_nightly, coverage(off))]
     fn debug() {
         let ptr = TypeErasedPtr::new(&1);
-        format!("{:?}", ptr);
+        let _msg = format!("{:?}", ptr);
     }
 }

src/erased_ptr.rs

@@ -159,10 +159,32 @@ impl<T: ?Sized> Prc<T> {
     /// let local = 5;
     /// let prc = Prc::from_rc(&rc, |_| &local);
     /// ```
+    ///
+    /// Do not allow calling `from_rc` on `Rc`s that don't own their data.
+    /// If this is allowed to happen, we could create a `Prc` with a longer lifetime than the
+    /// original `Rc`.
+    /// ```compile_fail,E0597
+    /// use pared::prc::Prc;
+    /// use std::rc::Rc;
+    ///
+    /// struct PrintOnDrop<'a>(&'a str);
+    /// impl Drop for PrintOnDrop<'_> {
+    ///     fn drop(&mut self) {
+    ///         println!("dropping: {:?}", self.0);
+    ///     }
+    /// }
+    ///
+    /// let s = "Hello World!".to_owned();
+    /// let rc = Rc::new(PrintOnDrop(&s));
+    /// let p = Prc::from_rc(&rc, |_| &());
+    /// drop(rc);
+    /// drop(s);
+    /// drop(p); // garbage / use-after-free if `from_rc` compiles
+    /// ```
     #[inline]
     pub fn from_rc<U, F>(rc: &Rc<U>, project: F) -> Self
     where
-        U: ?Sized,
+        U: ?Sized + 'static,
         T: 'static,
         F: FnOnce(&U) -> &T,
     {
@@ -201,10 +223,32 @@ impl<T: ?Sized> Prc<T> {
     ///
     /// assert!(matches!(prc, Ok(prc) if *prc == 5 ));
     /// ```
+    ///
+    /// Do not allow calling `from_rc` on `Rc`s that don't own their data.
+    /// If this is allowed to happen, we could create a `Prc` with a longer lifetime than the
+    /// original `Rc`.
+    /// ```compile_fail,E0597
+    /// use pared::prc::Prc;
+    /// use std::rc::Rc;
+    ///
+    /// struct PrintOnDrop<'a>(&'a str);
+    /// impl Drop for PrintOnDrop<'_> {
+    ///     fn drop(&mut self) {
+    ///         println!("dropping: {:?}", self.0);
+    ///     }
+    /// }
+    ///
+    /// let s = "Hello World!".to_owned();
+    /// let rc = Rc::new(PrintOnDrop(&s));
+    /// let p = Prc::try_from_rc(&rc, |_| Ok::<&'static(), ()>(&()));
+    /// drop(rc);
+    /// drop(s);
+    /// drop(p); // garbage / use-after-free if `try_from_rc` compiles
+    /// ```
     #[inline]
     pub fn try_from_rc<U, E, F>(rc: &Rc<U>, project: F) -> Result<Self, E>
     where
-        U: ?Sized,
+        U: ?Sized + 'static,
         T: 'static,
         F: FnOnce(&U) -> Result<&T, E>,
     {
@@ -237,10 +281,26 @@ impl<T: ?Sized> Prc<T> {
     /// let local = 5;
     /// let projected = prc.project(|_| &local);
     /// ```
+    ///
+    /// /// Do not allow coercing `Prc<&'static T>` to `Prc<&'short T>` when projecting:
+    /// ```compile_fail,E0597
+    /// use pared::prc::Prc;
+    ///
+    /// let s = "Hello World!".to_owned();
+    /// // x: Prc<&'static ()>
+    /// let x = Prc::new(&());
+    /// // This must fail
+    /// let x = x.project(|_| s.as_str());
+    ///
+    /// println!("{:?}", &*x); // "Hello World!"
+    /// drop(s);
+    /// println!("{:?}", &*x); // garbage / use-after-free if the above doesn't fail
+    /// ```
     #[inline]
     pub fn project<U, F>(&self, project: F) -> Prc<U>
     where
         U: ?Sized + 'static,
+        T: 'static,
         F: FnOnce(&T) -> &U,
     {
         let projected = project(self);

src/prc.rs

@@ -204,12 +204,34 @@ impl<T: ?Sized> Parc<T> {
     /// let local = 5;
     /// let parc = Parc::from_arc(&arc, |tuple| &local);
     /// ```
+    ///
+    /// Do not allow calling `from_arc` on `Arc`s that don't own their data.
+    /// If this is allowed to happen, we could create a `Parc` with a longer lifetime than the
+    /// original `Arc`.
+    /// ```compile_fail,E0597
+    /// use pared::sync::Parc;
+    /// use std::sync::Arc;
+    ///
+    /// struct PrintOnDrop<'a>(&'a str);
+    /// impl Drop for PrintOnDrop<'_> {
+    ///     fn drop(&mut self) {
+    ///         println!("dropping: {:?}", self.0);
+    ///     }
+    /// }
+    ///
+    /// let s = "Hello World!".to_owned();
+    /// let arc = Arc::new(PrintOnDrop(&s));
+    /// let p = Parc::from_arc(&arc, |_| &());
+    /// drop(arc);
+    /// drop(s);
+    /// drop(p); // garbage / use-after-free if `from_arc` compiles
+    /// ```
     #[inline]
     pub fn from_arc<U, F>(arc: &Arc<U>, project: F) -> Self
     where
-        T: 'static,
-        U: ?Sized + Send + Sync,
+        U: ?Sized + Send + Sync + 'static,
         F: FnOnce(&U) -> &T,
+        T: 'static,
     {
         let projected = project(arc);
         // SAFETY: the returned reference always converts to a non-null pointer.
@@ -249,10 +271,32 @@ impl<T: ?Sized> Parc<T> {
     ///
     /// assert!(matches!(parc, Ok(parc) if *parc == 5 ));
     /// ```
+    ///
+    /// Do not allow calling `try_from_arc` on `Arc`s that don't own their data.
+    /// If this is allowed to happen, we could create a `Parc` with a longer lifetime than the
+    /// original `Arc`.
+    /// ```compile_fail,E0597
+    /// use pared::sync::Parc;
+    /// use std::sync::Arc;
+    ///
+    /// struct PrintOnDrop<'a>(&'a str);
+    /// impl Drop for PrintOnDrop<'_> {
+    ///     fn drop(&mut self) {
+    ///         println!("dropping: {:?}", self.0);
+    ///     }
+    /// }
+    ///
+    /// let s = "Hello World!".to_owned();
+    /// let arc = Arc::new(PrintOnDrop(&s));
+    /// let p = Parc::try_from_arc(&arc, |_| Ok::<&'static (), ()>(&()));
+    /// drop(arc);
+    /// drop(s);
+    /// drop(p); // garbage / use-after-free if `try_from_arc` compiles
+    /// ```
     #[inline]
     pub fn try_from_arc<U, E, F>(arc: &Arc<U>, project: F) -> Result<Self, E>
     where
-        U: ?Sized + Sync + Send,
+        U: ?Sized + Sync + Send + 'static,
         T: 'static,
         F: FnOnce(&U) -> Result<&T, E>,
     {
@@ -285,10 +329,25 @@ impl<T: ?Sized> Parc<T> {
     /// let local = 5;
     /// let projected = parc.project(|tuple| &local);
     /// ```
+    ///
+    /// Do not allow coercing `Parc<&'static T>` to `Parc<&'short T>` when projecting:
+    /// ```compile_fail,E0597
+    /// use pared::sync::Parc;
+    ///
+    /// let s = "Hello World!".to_owned();
+    /// // x: Parc<&'static ()>
+    /// let x = Parc::new(&());
+    /// // This must fail
+    /// let x = x.project(|_| s.as_str());
+    ///
+    /// println!("{:?}", &*x); // "Hello World!"
+    /// drop(s);
+    /// println!("{:?}", &*x); // garbage / use-after-free if the above doesn't fail
+    /// ```
     #[inline]
     pub fn project<U, F>(&self, project: F) -> Parc<U>
     where
-        T: Send + Sync,
+        T: Send + Sync + 'static,
         U: ?Sized + 'static,
         F: FnOnce(&T) -> &U,
     {

src/sync.rs

@@ -55,6 +55,6 @@ mod tests {
             strong_count_weak: c,
             weak_count_weak: c,
         };
-        format!("{:?}", vtable);
+        let _msg = format!("{:?}", vtable);
     }
 }

src/vtable.rs

@@ -217,11 +217,11 @@ fn borrows() {
 fn fmt() {
     let parc = Parc::new(5);
 
-    format!("{} {:?} {:p}", parc, parc, parc);
+    let _msg = format!("{} {:?} {:p}", parc, parc, parc);
 
     let weak = Parc::downgrade(&parc);
 
-    format!("{:?}", weak);
+    let _msg = format!("{:?}", weak);
 }
 
 #[test]

tests/parc.rs

@@ -218,11 +218,11 @@ fn borrows() {
 fn fmt() {
     let prc = Prc::new(5);
 
-    format!("{} {:?} {:p}", prc, prc, prc);
+    let _msg = format!("{} {:?} {:p}", prc, prc, prc);
 
     let weak = Prc::downgrade(&prc);
 
-    format!("{:?}", weak);
+    let _msg = format!("{:?}", weak);
 }
 
 #[test]