…i.e. never forget the existence of mem::swap. Here’s a problematic code example

use soapy::{Soa, Soapy};

#[derive(Soapy)]
struct Foo(u8);

fn main() {
    let mut x = Soa::<Foo>::new();
    x.push(Foo(0));
    let mut y = Soa::<Foo>::new();

    std::mem::swap(x.as_mut_slice(), y.as_mut_slice());
    x.push(Foo(0)); // segfault
}

Unless of course Soa was somehow made to deal with blatantly incorrect capacity values… which seems impossible because capacity is safety-critical information, e.g. instead of out-of-bounds access like above (which can be checked by looking at the len), access of uninitialized data is the whole point of what capacity tries to prevent.

This unfortunately kills the deref-based API, at least as far as DerefMut is concerned, at least you can’t move out of a shared reference so the Deref part does still stand?

Nevermind. As I’m writing this, I’m seeing the next problem:

use soapy::{Soa, Soapy};

#[derive(Soapy)]
struct Foo(Box<&'static &'static usize>);

fn main() {
    let mut x = Soa::<Foo>::new();
    x.push(Foo(Box::new(&&0)));
    let slice = *x.as_slice();

    drop(x);
    println!("{:?}", slice.get(0).unwrap().0); // segfault
}

the type Slice<T> is Copy, and thus can just be copied out from behind even a shared immutable reference, kept without a lifetime, and lead to use-after free.

Using version 0.3.1 of soa-rs I cannot compile anything because a private type is being leaked from soa-rs-derive.

For me, to reproduce the issue it's as simple as

cargo init test_soa_rs;
cd test_soa_rs;
cargo add soa-rs;
cargo check;

I get the following error message

   Compiling soa-rs-derive v0.3.1
error[E0446]: private type `SoaDerive` in public interface
  --> C:\Users\zane\.cargo\registry\src\index.crates.io-6f17d22bba15001f\soa-rs-derive-0.3.1\src\fields.rs:9:1
   |
9  | / pub fn fields_struct(
10 | |     ident: Ident,
11 | |     vis: Visibility,
12 | |     fields: Punctuated<Field, Comma>,
13 | |     kind: FieldKind,
14 | |     soa_derive: SoaDerive,
15 | | ) -> Result<TokenStream, syn::Error> {
   | |____________________________________^ can't leak private type
   |
  ::: C:\Users\zane\.cargo\registry\src\index.crates.io-6f17d22bba15001f\soa-rs-derive-0.3.1\src\lib.rs:75:1
   |
75 |   struct SoaDerive {
   |   ---------------- `SoaDerive` declared as private

For more information about this error, try `rustc --explain E0446`.
error: could not compile `soa-rs-derive` (lib) due to previous error

I know you are probably busy with the soundness issues right now, so this is going to to be low priority.

I recently ran into an issue where I had wildly varying sizes for different enum members. I could of course box one of them if it is rare, but I had a pretty even distribution in my case. Could we derive a "array of enum variants" similar to this that simply puts all instances of different enum types in different arrays internally, thus reducing memory usage and improving cache locality? I don't know what a good API would look like, some thought is required here.

Support serde serialization and deserialization of soa_rs::Soa.

Implementation could be added if #[soa_serde] attribute is added to the struct. It would require that all fields are serializable, deserializable.

This would be useful when data needs to be passed around and no conversion between Vec and soa_rs::Soa would be needed.

if you try to use let's say a mutex within a soar struct, the compiler will complain.

In other words, you can’t just copy a value to a different place, and then expose that copy by-immutable-reference to untrusted (safe) user code, because immutable references could have mutable access behind interior mutability.

use soapy::{Soa, Soapy, WithRef};
use std::cell::RefCell;

#[derive(Soapy)]
struct Foo(RefCell<String>);

fn main() {
    let mut x = Soa::<Foo>::new();
    x.push(Foo(RefCell::new(String::from("Hello World"))));

    let r = x.get(0).unwrap();

    // mem::take gets ownership and modifies the `String`, however these modifications are only happening
    // to the local copy via `ptr::read` in the `unsafe` implementation of `WithRef`, so the original `String`
    // keeps ownership, too
    let s1: String =
        r.with_ref(|copy_of_refcell| std::mem::take::<String>(&mut copy_of_refcell.0.borrow_mut()));

    // let's just do it again, because copy-paste is easy
    let s2: String =
        r.with_ref(|copy_of_refcell| std::mem::take::<String>(&mut copy_of_refcell.0.borrow_mut()));

    println!("{s1} and {s2} again?"); // ==>> "Hello World and Hello World again?"
    drop(s1);
    println!("freed!");
    drop(s2); // double-free
    println!("freed again!");

    // (triple-free, once `x` is dropped)
}

a workaround of “just write back the changes” of course can’t work either, as that isn’t thread-safe, whilst Soa promises Sync.

I don’t have any immediate ideas on how to keep the nice implementations of Hash and Eq and the like.

Test that doesn't pass:

#[test]
fn field_type_without_partial_eq() {
    // Type that doesn't implement PartialEq
    struct TypeWithoutPartialEq {
        foo: u8,
    }

    #[derive(Soars)]
    struct Example {
        a: TypeWithoutPartialEq,
    }
    let arr = soa![Example {
        a: TypeWithoutPartialEq { foo: 1 }
    }];
    assert_eq!(arr.get(0).unwrap().a.foo, 1);
}

If PartialEq derives are added, then this test passes. It shouldn't be required.

Tried to fix this in https://github.com/tim-harding/soa-rs/pull/8/files , but pull request was closed even if the existing PartialEq implementation in derive macro for #itemRef was made conditional (kept the same) based on soa_derive.

This heavily limits what types can be stored in Soa.

use soa_rs::{SoaArray, Soars};

fn main() {
    let foo = Box::new(42_u8);
    let x = ExampleArray::from_array([Example { foo }]);
    let s = x.as_slice();
    let v: &Box<u8> = s.get(0).unwrap().foo;
    dbg!(v);
}

#[derive(Soars)]
#[soa_derive(PartialEq)] // <- unrelated issue: I get a compile error without this
struct Example {
    foo: Box<u8>,
}

$ cargo +nightly miri run
Preparing a sysroot for Miri (target: aarch64-unknown-linux-gnu)... done
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.23s
     Running `/home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/bin/cargo-miri runner target/miri/aarch64-unknown-linux-gnu/debug/play`
error: Undefined Behavior: trying to retag from <1690> for Unique permission at alloc752[0x0], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:11:10
   |
11 | #[derive(Soars)]
   |          ^^^^^
   |          |
   |          trying to retag from <1690> for Unique permission at alloc752[0x0], but that tag does not exist in the borrow stack for this location
   |          this error occurs as part of retag (of a reference/box inside this compound value) at alloc752[0x0..0x1]
   |          errors for retagging in fields are fairly new; please reach out to us (e.g. at <https://rust-lang.zulipchat.com/#narrow/stream/269128-miri>) if you find this error troubling
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1690> was created by a Unique retag at offsets [0x0..0x1]
  --> src/main.rs:11:10
   |
11 | #[derive(Soars)]
   |          ^^^^^
help: <1690> was later invalidated at offsets [0x0..0x1] by a Unique retag (of a reference/box inside this compound value)
  --> src/main.rs:11:10
   |
11 | #[derive(Soars)]
   |          ^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `ExampleArray::<1>::from_array` at src/main.rs:11:10: 11:15
note: inside `main`
  --> src/main.rs:5:13
   |
5  |     let x = ExampleArray::from_array([Example { foo }]);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: this error originates in the derive macro `Soars` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

use soa_rs::{SoaArray, Soars};

fn main() {
    let foo = Box::new(42_u8);
    let x = ExampleArray::from_array_([Example { foo }]);
    let s = x.as_slice();
    let v: &Box<u8> = s.get(0).unwrap().foo;
    dbg!(v);
}

#[derive(Soars)]
#[soa_derive(PartialEq)] // <- unrelated issue: I get a compile error without this
struct Example {
    foo: Box<u8>,
}

impl<const N: usize> ExampleArray<N> {
    const fn from_array_(array: [Example; N]) -> Self {
        struct Uninit<const K: usize> {
            #[automatically_derived]
            foo: [::std::mem::MaybeUninit<Box<u8>>; K],
        };
        let mut uninit: Uninit<N> = Uninit {
            foo: unsafe { ::std::mem::MaybeUninit::uninit().assume_init() },
        };
        let mut i = 0;
        while i < N {
            let src = &array[i].foo as *const Box<u8>;
            unsafe {
                uninit.foo[i] = ::std::mem::MaybeUninit::new(src.read());
            }
            i += 1;
        }
        ::std::mem::forget(array);
        Self {
            foo: unsafe { ::std::mem::transmute_copy(&::std::mem::ManuallyDrop::new(uninit.foo)) },
        }
    }
}

cargo +nightly miri run
Preparing a sysroot for Miri (target: aarch64-unknown-linux-gnu)... done
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.14s
     Running `/home/frank/.rustup/toolchains/nightly-aarch64-unknown-linux-gnu/bin/cargo-miri runner target/miri/aarch64-unknown-linux-gnu/debug/play`
warning: unnecessary trailing semicolon
  --> src/main.rs:22:10
   |
22 |         };
   |          ^ help: remove this semicolon
   |
   = note: `#[warn(redundant_semicolons)]` on by default

warning: `#[automatically_derived]` only has an effect on implementation blocks
  --> src/main.rs:20:13
   |
20 |             #[automatically_derived]
   |             ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_attributes)]` on by default

error: Undefined Behavior: trying to retag from <1690> for Unique permission at alloc752[0x0], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:36:27
   |
36 |             foo: unsafe { ::std::mem::transmute_copy(&::std::mem::ManuallyDrop::new(uninit.foo)) },
   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |                           |
   |                           trying to retag from <1690> for Unique permission at alloc752[0x0], but that tag does not exist in the borrow stack for this location
   |                           this error occurs as part of retag (of a reference/box inside this compound value) at alloc752[0x0..0x1]
   |                           errors for retagging in fields are fairly new; please reach out to us (e.g. at <https://rust-lang.zulipchat.com/#narrow/stream/269128-miri>) if you find this error troubling
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1690> was created by a Unique retag at offsets [0x0..0x1]
  --> src/main.rs:30:33
   |
30 |                 uninit.foo[i] = ::std::mem::MaybeUninit::new(src.read());
   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <1690> was later invalidated at offsets [0x0..0x1] by a Unique retag (of a reference/box inside this compound value)
  --> src/main.rs:34:28
   |
34 |         ::std::mem::forget(array);
   |                            ^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `ExampleArray::<1>::from_array_` at src/main.rs:36:27: 36:97
note: inside `main`
  --> src/main.rs:5:13
   |
5  |     let x = ExampleArray::from_array_([Example { foo }]);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error; 2 warnings emitted

The problem is the usage of mem::forget in ExampleArray::from_array. This moves a value of type Example, and when you move / pass a Box<T> or &mut T in Rust (under Stacked Borrows at least) by value (even in a field), it asserts unique access to that reference. But at that point, the copy via src.read() is already done.

This should be easy to fix by using ManuallyDrop instead.

It is generally preferrable to use ManuallyDrop instead of mem::forget. (Looking at the code, this seems to be the only mem::forget you use anywhere though.) Wrap the struct you want to not drop (like array in the derive-generated fn from_array) into ManuallyDrop in the beginning of the function (i.e. in general: as soon as it’s clear you’re not gonna drop it, and before you copied any contents out of it), and then read the contents piece-by-piece through ManuallyDrop’s Deref/…Mut interface.

This also has the additional benefit that any accidentally overlooked panic path cannot result in doubly-frees (by skipping the mem::forget).