Replace MemCase's unsound impl of Deref/AsMut with a borrow() method

This code compiled fine, but raised a segfault on the last println:

```rust
use epserde::deser::Deserialize;
use epserde::prelude::Flags;
use epserde::prelude::MemCase;
use epserde::ser::Serialize;
use std::fs::File;
use std::io::BufWriter;

fn main() {
    let v = vec![0u64, 10, 20, 30, 40];

    let path = std::path::PathBuf::from("/tmp/test.vector");

    println!("Writing vector...");
    let mut writer = BufWriter::new(File::create(&path).expect("Could not create temp file"));
    unsafe { v.serialize(&mut writer) }.expect("Could not write vector");
    drop(v);

    println!("mmapping vector...");
    let v =
        unsafe { <Vec<u64>>::mmap(&path, Flags::RANDOM_ACCESS) }.expect("Could not mmap vector");
    let v2: &[u64] = *v;

    println!("Vector value:");
    println!("{:?}", v2);

    println!("Dropping vector...");
    drop(v);

    println!("Vector value:");
    println!("{:?}", v2);
}
```

This is because the `Deref` implementation of `MemCase<&[u64]>`
effectively yields a `&&'static [u64]` which can itself be dereferenced
to a `&'static [u64]`, which outlives the `MemCase`.

There does not seem to be a way to implement `Deref` or `AsRef` (or
`std::borrow::Borrow`) without yielding some sort of `'static` here.
So as far as I can tell, we have to take this ergonomic blow in order to
avoid segfaults that are very easy for users to trigger unknowingly.

Author: progval

README.md

@@ -76,14 +76,12 @@ These are the main limitations you should be aware of before choosing to use
   structure you will need to couple permanently the deserialized structure with
   its serialized support, which is obtained by putting it in a [`MemCase`] using
   the convenience methods [`Deserialize::load_mem`], [`Deserialize::load_mmap`],
-  and [`Deserialize::mmap`]. A [`MemCase`] will deref to its contained type, so it
-  can be used transparently as long as fields and methods are concerned, but if
-  your original type is `T` the field of the new structure will have to be of type
-  `MemCase<DeserType<'static, T>>`, not `T`.
+  and [`Deserialize::mmap`]. A [`MemCase`] provides a method that yields references
+  to the deserialized type associated to its contained type.
 
 - No validation or padding cleaning is performed on zero-copy types. If you plan
   to serialize data and distribute it, you must take care of these issues.
-  
+
 ## Pros
 
 - Almost instant deserialization with minimal allocation provided that you
@@ -140,7 +138,7 @@ let t: DeserType<'_, [usize; 1000]> =
 assert_eq!(s, *t);
 
 // This is a traditional deserialization instead
-let t: [usize; 1000] = 
+let t: [usize; 1000] =
     unsafe { <[usize; 1000]>::deserialize_full(
         &mut std::fs::File::open(&file)?
     )? };
@@ -149,25 +147,19 @@ assert_eq!(s, t);
 // In this case we map the data structure into memory
 //
 // Note: requires the `mmap` feature.
-let u: MemCase<&[usize; 1000]> = 
-    unsafe { <[usize; 1000]>::mmap(&file, Flags::empty())? };
-
-assert_eq!(s, **u);
-
-// When using a MemCase, the lifetime of the derived deserialization type is 'static
-let u: MemCase<DeserType<'static, [usize; 1000]>> = 
+let u: MemCase<[usize; 1000]> =
     unsafe { <[usize; 1000]>::mmap(&file, Flags::empty())? };
 
-assert_eq!(s, **u);
+assert_eq!(s, **u.borrow());
 #     Ok(())
 # }
 ```
 
 Note how we serialize an array, but we deserialize a reference. The reference
 points inside `b`, so there is no copy performed. The call to
 [`deserialize_full`] creates a new array instead. The third call maps the data
-structure into memory and returns a [`MemCase`] that can be used transparently
-as a reference to the array; moreover, the [`MemCase`] can be passed to other
+structure into memory and returns a [`MemCase`] that can be used to get
+a reference to the array; moreover, the [`MemCase`] can be passed to other
 functions or stored in a structure field, as it contains both the structure and
 the memory-mapped region that supports it.
 
@@ -205,14 +197,14 @@ let t: DeserType<'_, Vec<usize>> =
 assert_eq!(s, *t);
 
 // This is a traditional deserialization instead
-let t: Vec<usize> = 
+let t: Vec<usize> =
     unsafe { <Vec<usize>>::load_full(&file)? };
 assert_eq!(s, t);
 
 // In this case we map the data structure into memory
-let u: MemCase<DeserType<'static, Vec<usize>>> = 
+let u: MemCase<Vec<usize>> =
     unsafe { <Vec<usize>>::mmap(&file, Flags::empty())? };
-assert_eq!(s, **u);
+assert_eq!(s, **u.borrow());
 #     Ok(())
 # }
 ```
@@ -264,14 +256,14 @@ let t: DeserType<'_, Vec<Data>> =
 assert_eq!(s, *t);
 
 // This is a traditional deserialization instead
-let t: Vec<Data> = 
+let t: Vec<Data> =
     unsafe { <Vec<Data>>::load_full(&file)? };
 assert_eq!(s, t);
 
 // In this case we map the data structure into memory
-let u: MemCase<DeserType<'static, Vec<Data>>> = 
+let u: MemCase<Vec<Data>> =
     unsafe { <Vec<Data>>::mmap(&file, Flags::empty())? };
-assert_eq!(s, **u);
+assert_eq!(s, **u.borrow());
 #     Ok(())
 # }
 ```
@@ -309,20 +301,21 @@ unsafe { s.store(&file) };
 let b = std::fs::read(&file)?;
 
 // The type of t will be inferred--it is shown here only for clarity
-let t: MyStruct<&[isize]> = 
+let t: MyStruct<&[isize]> =
     unsafe { <MyStruct<Vec<isize>>>::deserialize_eps(b.as_ref())? };
 
 assert_eq!(s.id, t.id);
 assert_eq!(s.data, Vec::from(t.data));
 
 // This is a traditional deserialization instead
-let t: MyStruct<Vec<isize>> = 
+let t: MyStruct<Vec<isize>> =
     unsafe { <MyStruct<Vec<isize>>>::load_full(&file)? };
 assert_eq!(s, t);
 
 // In this case we map the data structure into memory
-let u: MemCase<MyStruct<&[isize]>> = 
+let u: MemCase<MyStruct<Vec<isize>>> =
     unsafe { <MyStruct<Vec<isize>>>::mmap(&file, Flags::empty())? };
+let u: &MyStruct<&[isize]> = u.borrow();
 assert_eq!(s.id, u.id);
 assert_eq!(s.data, u.data.as_ref());
 #     Ok(())
@@ -371,8 +364,9 @@ let t = unsafe { MyStruct::deserialize_eps(b.as_ref())? };
 assert_eq!(s.sum(), t.sum());
 
 let t = unsafe { <MyStruct>::mmap(&file, Flags::empty())? };
+let t: &MyStructParam<&[isize]> = t.borrow();
 
-// t works transparently as a MyStructParam<&[isize]>
+// t works transparently as a &MyStructParam<&[isize]>
 assert_eq!(s.id, t.id);
 assert_eq!(s.data, t.data.as_ref());
 assert_eq!(s.sum(), t.sum());
@@ -407,7 +401,7 @@ unsafe { s.store(&file) };
 let b = std::fs::read(&file)?;
 
 // The type of t is unchanged
-let t: MyStruct<Vec<isize>> = 
+let t: MyStruct<Vec<isize>> =
     unsafe { <MyStruct<Vec<isize>>>::deserialize_eps(b.as_ref())? };
 #     Ok(())
 # }
@@ -445,7 +439,7 @@ unsafe { s.store(&file) };
 let b = std::fs::read(&file)?;
 
 // The type of t is unchanged
-let t: &MyStruct<i32> = 
+let t: &MyStruct<i32> =
     unsafe { <MyStruct<i32>>::deserialize_eps(b.as_ref())? };
 #     Ok(())
 # }
@@ -480,7 +474,7 @@ let e = Enum::B(vec![0, 1, 2, 3]);
 let mut file = std::env::temp_dir();
 file.push("serialized7");
 unsafe { e.store(&file) };
-// Deserializing using just Enum will fail, as the type parameter 
+// Deserializing using just Enum will fail, as the type parameter
 // by default is Vec<usize>
 assert!(unsafe { <Enum>::load_full(&file) }.is_err());
 #     Ok(())
@@ -513,7 +507,7 @@ let t: &[i32] = unsafe { <Vec<i32>>::deserialize_eps(b.as_ref())? };
 let t: Vec<i32> = unsafe { <Vec<i32>>::deserialize_full(
         &mut std::fs::File::open(&file)?
     )? };
-let t: MemCase<&[i32]> = unsafe { <Vec<i32>>::mmap(&file, Flags::empty())? };
+let t: MemCase<Vec<i32>> = unsafe { <Vec<i32>>::mmap(&file, Flags::empty())? };
 
 // Within a structure
 #[derive(Epserde, Debug, PartialEq, Eq, Default, Clone)]
@@ -532,7 +526,7 @@ let t: Data<&[i32]> = unsafe { <Data<Vec<i32>>>::deserialize_eps(b.as_ref())? };
 let t: Data<Vec<i32>> = unsafe { <Data<Vec<i32>>>::deserialize_full(
         &mut std::fs::File::open(&file)?
     )? };
-let t: MemCase<Data<&[i32]>> = unsafe { <Data<Vec<i32>>>::mmap(&file, Flags::empty())? };
+let t: MemCase<Data<Vec<i32>>> = unsafe { <Data<Vec<i32>>>::mmap(&file, Flags::empty())? };
 #     Ok(())
 # }
 ```
@@ -565,7 +559,7 @@ let t: &[i32] = unsafe { <Vec<i32>>::deserialize_eps(b.as_ref())? };
 let t: Vec<i32> = unsafe { <Vec<i32>>::deserialize_full(
         &mut std::fs::File::open(&file)?
     )? };
-let t: MemCase<&[i32]> = unsafe { <Vec<i32>>::mmap(&file, Flags::empty())? };
+let t: MemCase<Vec<i32>> = unsafe { <Vec<i32>>::mmap(&file, Flags::empty())? };
 
 // Within a structure
 #[derive(Epserde, Debug, PartialEq, Eq, Default, Clone)]
@@ -584,7 +578,7 @@ let t: Data<&[i32]> = unsafe { <Data<Vec<i32>>>::deserialize_eps(b.as_ref())? };
 let t: Data<Vec<i32>> = unsafe { <Data<Vec<i32>>>::deserialize_full(
         &mut std::fs::File::open(&file)?
     )? };
-let t: MemCase<Data<&[i32]>> = unsafe { <Data<Vec<i32>>>::mmap(&file, Flags::empty())? };
+let t: MemCase<Data<Vec<i32>>> = unsafe { <Data<Vec<i32>>>::mmap(&file, Flags::empty())? };
 #     Ok(())
 # }
 ```

epserde/src/deser/mem_case.rs

@@ -4,8 +4,9 @@
  * SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
  */
 
+use crate::DeserializeInner;
 use bitflags::bitflags;
-use core::{mem::size_of, ops::Deref};
+use core::{fmt, mem::size_of};
 use maligned::A64;
 use mem_dbg::{MemDbg, MemSize};
 
@@ -111,36 +112,42 @@ impl MemBackend {
 /// wrapped type, using the no-op [`None`](`MemBackend#variant.None`) variant
 /// of [`MemBackend`], so a structure can be [encased](MemCase::encase)
 /// almost transparently.
-#[derive(Debug, MemDbg, MemSize)]
-pub struct MemCase<S>(pub(crate) S, pub(crate) MemBackend);
+#[derive(MemDbg, MemSize)]
+pub struct MemCase<'a, S: DeserializeInner>(
+    pub(crate) <S as DeserializeInner>::DeserType<'a>,
+    pub(crate) MemBackend,
+);
 
-impl<S> MemCase<S> {
-    /// Encases a data structure in a [`MemCase`] with no backend.
-    pub fn encase(s: S) -> MemCase<S> {
-        MemCase(s, MemBackend::None)
+impl<'a, S: DeserializeInner> fmt::Debug for MemCase<'a, S>
+where
+    <S as DeserializeInner>::DeserType<'a>: fmt::Debug,
+{
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_tuple("MemBackend")
+            .field(&self.0)
+            .field(&self.1)
+            .finish()
     }
 }
 
-unsafe impl<S: Send> Send for MemCase<S> {}
-unsafe impl<S: Sync> Sync for MemCase<S> {}
-
-impl<S> Deref for MemCase<S> {
-    type Target = S;
-    #[inline(always)]
-    fn deref(&self) -> &Self::Target {
-        &self.0
+impl<'a, S: DeserializeInner> MemCase<'a, S> {
+    /// Encases a data structure in a [`MemCase`] with no backend.
+    pub fn encase(s: <S as DeserializeInner>::DeserType<'a>) -> Self {
+        MemCase(s, MemBackend::None)
     }
-}
 
-impl<S> AsRef<S> for MemCase<S> {
-    #[inline(always)]
-    fn as_ref(&self) -> &S {
-        &self.0
+    pub fn borrow<'this>(&'this self) -> &'this <S as DeserializeInner>::DeserType<'this> {
+        // SAFETY: 'a outlives 'this, and <S as DeserializeInner>::DeserType is required to be
+        // covariant (ie. it's a normal structure and not, say, a closure with 'this as argument)
+        // TODO: document in DeserializeInner that its DeserType must be covariant
+        unsafe {
+            core::mem::transmute::<
+                &'this <S as DeserializeInner>::DeserType<'a>,
+                &'this <S as DeserializeInner>::DeserType<'this>,
+            >(&self.0)
+        }
     }
 }
 
-impl<S: Send + Sync> From<S> for MemCase<S> {
-    fn from(s: S) -> Self {
-        MemCase::encase(s)
-    }
-}
+unsafe impl<'a, S: DeserializeInner + Send> Send for MemCase<'a, S> {}
+unsafe impl<'a, S: DeserializeInner + Sync> Sync for MemCase<'a, S> {}

epserde/src/deser/mod.rs

@@ -99,9 +99,7 @@ pub trait Deserialize: DeserializeInner {
     /// # Safety
     ///
     /// See the [trait documentation](Deserialize).
-    unsafe fn load_mem<'a>(
-        path: impl AsRef<Path>,
-    ) -> anyhow::Result<MemCase<<Self as DeserializeInner>::DeserType<'a>>> {
+    unsafe fn load_mem<'a>(path: impl AsRef<Path>) -> anyhow::Result<MemCase<'a, Self>> {
         let align_to = align_of::<MemoryAlignment>();
         if align_of::<Self>() > align_to {
             return Err(Error::AlignmentError.into());
@@ -111,8 +109,7 @@ pub trait Deserialize: DeserializeInner {
         // Round up to u128 size
         let capacity = file_len + crate::pad_align_to(file_len, align_to);
 
-        let mut uninit: MaybeUninit<MemCase<<Self as DeserializeInner>::DeserType<'_>>> =
-            MaybeUninit::uninit();
+        let mut uninit: MaybeUninit<MemCase<'_, Self>> = MaybeUninit::uninit();
         let ptr = uninit.as_mut_ptr();
 
         // SAFETY: the entire vector will be filled with data read from the file,
@@ -170,13 +167,12 @@ pub trait Deserialize: DeserializeInner {
     unsafe fn load_mmap<'a>(
         path: impl AsRef<Path>,
         flags: Flags,
-    ) -> anyhow::Result<MemCase<<Self as DeserializeInner>::DeserType<'a>>> {
+    ) -> anyhow::Result<MemCase<'a, Self>> {
         let file_len = path.as_ref().metadata()?.len() as usize;
         let mut file = std::fs::File::open(path)?;
         let capacity = file_len + crate::pad_align_to(file_len, 16);
 
-        let mut uninit: MaybeUninit<MemCase<<Self as DeserializeInner>::DeserType<'_>>> =
-            MaybeUninit::uninit();
+        let mut uninit: MaybeUninit<MemCase<'_, Self>> = MaybeUninit::uninit();
         let ptr = uninit.as_mut_ptr();
 
         let mut mmap = mmap_rs::MmapOptions::new(capacity)?
@@ -217,15 +213,11 @@ pub trait Deserialize: DeserializeInner {
     ///
     /// See the [trait documentation](Deserialize) and [mmap's `with_file`'s documentation](mmap_rs::MmapOptions::with_file).
     #[cfg(feature = "mmap")]
-    unsafe fn mmap<'a>(
-        path: impl AsRef<Path>,
-        flags: Flags,
-    ) -> anyhow::Result<MemCase<<Self as DeserializeInner>::DeserType<'a>>> {
+    unsafe fn mmap<'a>(path: impl AsRef<Path>, flags: Flags) -> anyhow::Result<MemCase<'a, Self>> {
         let file_len = path.as_ref().metadata()?.len();
         let file = std::fs::File::open(path)?;
 
-        let mut uninit: MaybeUninit<MemCase<<Self as DeserializeInner>::DeserType<'_>>> =
-            MaybeUninit::uninit();
+        let mut uninit: MaybeUninit<MemCase<'_, Self>> = MaybeUninit::uninit();
         let ptr = uninit.as_mut_ptr();
 
         let mmap = unsafe {

epserde/tests/test_memcase.rs

@@ -37,18 +37,21 @@ fn test_mem_case() {
     unsafe { person.store("test.bin").unwrap() };
 
     let res = unsafe { Person::load_mem("test.bin").unwrap() };
+    let res = res.borrow();
     assert_eq!(person.test, res.test);
     assert_eq!(person.a, res.a);
     assert_eq!(person.b.a, res.b.a);
     assert_eq!(person.b.b, res.b.b);
 
     let res = unsafe { Person::load_mmap("test.bin", Flags::empty()).unwrap() };
+    let res = res.borrow();
     assert_eq!(person.test, res.test);
     assert_eq!(person.a, res.a);
     assert_eq!(person.b.a, res.b.a);
     assert_eq!(person.b.b, res.b.b);
 
     let res = unsafe { Person::load_mem("test.bin").unwrap() };
+    let res = res.borrow();
     assert_eq!(person.test, res.test);
     assert_eq!(person.a, res.a);
     assert_eq!(person.b.a, res.b.a);
@@ -61,18 +64,21 @@ fn test_mem_case() {
     assert_eq!(person.b.b, res.b.b);
 
     let res = unsafe { Person::mmap("test.bin", Flags::empty()).unwrap() };
+    let res = res.borrow();
     assert_eq!(person.test, res.test);
     assert_eq!(person.a, res.a);
     assert_eq!(person.b.a, res.b.a);
     assert_eq!(person.b.b, res.b.b);
 
     let res = unsafe { Person::mmap("test.bin", Flags::TRANSPARENT_HUGE_PAGES).unwrap() };
+    let res = res.borrow();
     assert_eq!(person.test, res.test);
     assert_eq!(person.a, res.a);
     assert_eq!(person.b.a, res.b.a);
     assert_eq!(person.b.b, res.b.b);
 
     let res = unsafe { Person::mmap("test.bin", Flags::empty()).unwrap() };
+    let res = res.borrow();
     assert_eq!(person.test, res.test);
     assert_eq!(person.a, res.a);
     assert_eq!(person.b.a, res.b.a);