CpuAccessibleBuffer methods read and write are unsound

[Rua]

These two methods give you an immutable and a mutable reference to the buffer contents, respectively. The reference is typed according to the generic type T of the CpuAccessibleBuffer. There are a few soundness issues with this:

1. A buffer starts off uninitialised, and getting a reference to uninitialised memory is UB.
2. A buffer is treated as a block of bytes by Vulkan, so it can contain a byte representation that is not valid for T.
3. What if T implements Drop?

The second issue can be solved by returning a byte slice reference from these two methods instead, but I'm not sure what to do about the others. Issue 1 may not be an issue as long as Rust doesn't "know" that the bytes aren't initialised, so it doesn't do weird optimisation tricks.

[Amjad50]

Regarding the first point, if I understand how the buffer is created, it should allocate a buffer without issues. So, it should be no problem to reference the buffer after creating it (And we do checks for the return code for memory mapping, so it should not give us invalid reference if it couldn't map the memory).

But another concern is the content of the buffer, which relate to the second point. On my machine, the content of uninitialized buffer is all zeros, but this can be different depending on implementation.

Correct me if I have misunderstood your point

[Rua]

All zeros is relatively safe, but could still be an invalid byte representation, depending on how T is defined.

[Amjad50]

Yes, so this is the problem in point 2.
Which seem to me the most serious (if we still want to keep using T)

I'll try to test point 3.

[Amjad50]

After some testing, I don't think we can trigger "Drop" of any data held by "Write/ReadLock", since only a reference is returned.
Also when the lock is dropped, the inner type Drop is not called.

[Rua]

While musing about this I wondered if it would be better if Vulkano exposed buffers only as bytes, and got rid of the type parameter. Vulkano isn't able to enforce the buffer contents anyway, since GPU operations can always write to buffers behind Vulkano's back. This would place the burden of ensuring that the contents of the buffer is valid on the user, but this is less of an issue than it might seem. As far as I can tell, if you have bogus data in a buffer you just read bogus data in the shader, but there is no UB associated with it. Things just don't work the way they should and that's about it.

As for Rust data being read from or written to a buffer, which is the point where CpuAccessibleBuffer currently has issues, I believe this is a good thing to leave to the user because there are many possible ways to do this. There's the excellent bytemuck and zerocopy crates that help the user much better than Vulkano ever could. It gives the user the freedom to use buffers however they wish.

[ShadowJonathan]

Sounds good, maybe refer to those crates in the documentation relevant to CpuAccessibleBuffer