From 37a3157534e88191ebd014a9aac9c67f8b1fd12b Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Thu, 9 Nov 2023 18:06:46 +0100 Subject: [PATCH 1/3] Fix insufficient covariance detection --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 1e49e3f..567ebb4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -577,7 +577,7 @@ macro_rules! _covariant_access { (covariant, $Vis:vis, $Dependent:ident) => { /// Borrows dependent. $Vis fn borrow_dependent<'_q>(&'_q self) -> &'_q $Dependent<'_q> { - fn _assert_covariance<'x: 'y, 'y>(x: $Dependent<'x>) -> $Dependent<'y> { + fn _assert_covariance<'x: 'y, 'y>(x: &'y $Dependent<'x>) -> &'y $Dependent<'y> { // This function only compiles for covariant types. x // Change the macro invocation to not_covariant. } From f82025e9f11221cb853a204645268d6d61376fe0 Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Thu, 9 Nov 2023 18:57:28 +0100 Subject: [PATCH 2/3] Adjust wrong_covariance invalid program output Also adjusts doc comment. --- src/lib.rs | 2 +- tests-extra/invalid/wrong_covariance.stderr | 31 ++++----------------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 567ebb4..4f764df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,7 +263,7 @@ pub mod unsafe_self_cell; /// /// * **covariant**: This generates the direct reference accessor function /// `borrow_dependent`. This is only safe to do if this compiles `fn -/// _assert_covariance<'x: 'y, 'y>(x: $Dependent<'x>) -> $Dependent<'y> +/// _assert_covariance<'x: 'y, 'y>(x: &'y $Dependent<'x>) -> &'y $Dependent<'y> /// {x}`. Otherwise you could choose a lifetime that is too short for types /// with interior mutability like `Cell`, which can lead to UB in safe code. /// Which would violate the promise of this library that it is safe-to-use. diff --git a/tests-extra/invalid/wrong_covariance.stderr b/tests-extra/invalid/wrong_covariance.stderr index 2a697b8..96fb5f2 100644 --- a/tests-extra/invalid/wrong_covariance.stderr +++ b/tests-extra/invalid/wrong_covariance.stderr @@ -1,4 +1,4 @@ -error[E0308]: mismatched types +error[E0623]: lifetime mismatch --> $DIR/wrong_covariance.rs:7:1 | 7 | / self_cell!( @@ -8,30 +8,9 @@ error[E0308]: mismatched types ... | 13 | | } 14 | | ); - | |__^ lifetime mismatch + | | ^ + | | | + | |__these two types are declared with different lifetimes... + | ...but data from `x` flows into `x` here | - = note: expected struct `Cell<&'y String>` - found struct `Cell<&'x String>` -note: the lifetime `'y` as defined on the function body at 580:43... - --> $DIR/wrong_covariance.rs:7:1 - | -7 | / self_cell!( -8 | | struct NoCov { -9 | | owner: String, -10 | | -... | -13 | | } -14 | | ); - | |__^ -note: ...does not necessarily outlive the lifetime `'x` as defined on the function body at 580:35 - --> $DIR/wrong_covariance.rs:7:1 - | -7 | / self_cell!( -8 | | struct NoCov { -9 | | owner: String, -10 | | -... | -13 | | } -14 | | ); - | |__^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) From e418be1c132a57d3b9ccc56847443b83457cbe83 Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Thu, 9 Nov 2023 18:57:52 +0100 Subject: [PATCH 3/3] Add wrong_covariance_unsize_coercion invalid program test --- .../wrong_covariance_unsize_coercion.rs | 32 +++++++++++++++++++ .../wrong_covariance_unsize_coercion.stderr | 16 ++++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests-extra/invalid/wrong_covariance_unsize_coercion.rs create mode 100644 tests-extra/invalid/wrong_covariance_unsize_coercion.stderr diff --git a/tests-extra/invalid/wrong_covariance_unsize_coercion.rs b/tests-extra/invalid/wrong_covariance_unsize_coercion.rs new file mode 100644 index 0000000..44e7a32 --- /dev/null +++ b/tests-extra/invalid/wrong_covariance_unsize_coercion.rs @@ -0,0 +1,32 @@ +use std::cell::RefCell; +use std::fmt; + +use self_cell::self_cell; + +self_cell! { + struct WrongVarianceExample { + owner: (), + + #[covariant] + dependent: Dependent, + } +} + +// this type is not covariant +type Dependent<'a> = RefCell>; + +fn main() { + let cell = WrongVarianceExample::new((), |_| RefCell::new(Box::new(""))); + let s = String::from("Hello World"); + + // borrow_dependent unsound due to incorrectly checked variance + *cell.borrow_dependent().borrow_mut() = Box::new(s.as_str()); + + // s still exists + cell.with_dependent(|_, d| println!("{}", d.borrow())); + + drop(s); + + // s is gone + cell.with_dependent(|_, d| println!("{}", d.borrow())); +} diff --git a/tests-extra/invalid/wrong_covariance_unsize_coercion.stderr b/tests-extra/invalid/wrong_covariance_unsize_coercion.stderr new file mode 100644 index 0000000..784ff7d --- /dev/null +++ b/tests-extra/invalid/wrong_covariance_unsize_coercion.stderr @@ -0,0 +1,16 @@ +error[E0623]: lifetime mismatch + --> $DIR/wrong_covariance_unsize_coercion.rs:6:1 + | +6 | / self_cell! { +7 | | struct WrongVarianceExample { +8 | | owner: (), +9 | | +... | +12 | | } +13 | | } + | | ^ + | | | + | |_these two types are declared with different lifetimes... + | ...but data from `x` flows into `x` here + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)