Unsound uninitialized array of `Voice`

[shinmao]

Hi, I found a potential unsound use of uninit() in the library. Could you check about it?

In following code, the safe new() will call uninit() to allocate a uninitialized struct (Voice) array.

https://github.com/Ameobea/web-synth/blob/556522b7e7bb168f57e27e0de22c9948bd4b5115/engine/polysynth/src/lib.rs#L108

However, it is UB to have an uninitialized Voice, isn't it? I think the code should be changed like:

let mut voices: [MaybeUninit<Voice>; POLY_SYNTH_VOICE_COUNT] = uninit();

Please let me know if I mistake for something? Thanks.

[Ameobea]

If you look a little bit further down in the code, that voices array is immediately initialized by writing voices in using std::ptr::write. This prevents drop() from getting called on the uninitialized voices and thus safely initializes the voices array. This is just a small optimization I suppose, since I could fill them with default voices or whatever and then just overwrite them, but I prefer doing it this way.

I know that Rust has the newer std::mem::MaybeUninit which they recommend over dealing with uninitialized memory directly. I dislike that interface and find it cumbersome.

The definitions of UB are broad; pretty much everything is UB if you look at it under the right light it seems. However, I've noticed no issues with this pattern and can't imagine how it could lead to issues with the program if implemented correctly.

[shinmao]

Yes, I saw that at the line 111 you immediately initialized with ptr::write. I also think this would not be a big issue. However, there should be no any UB in safe function. UB here is clearly defined in initialized invariant.

The issue occurs with assume_init which could become UB if the content (Voice) is not initialized actually just like mentioned in the documentation.

There is also an example which initialize struct in documentation. You still can use as_mut_ptr to *mut Voice and use ptr::write to initialize values. But only using assume_init after you initialize the struct.

[Ameobea]

This is one of the great benefits of being the sole developer on a project: I'm able to take liberties that I wouldn't/shouldn't take if working on a team of other developers or shipping a publicly available library or something similar.

For my use case, there is no issues caused by this pattern. I have complete control of my toolchain, production environment, and dependencies. I know that Rust is a language that holds safety as one of its core principals, and I appreciate that. The safety by default makes it much easier to write code that works well and avoids hard to track down bugs.

However, I equally cherish my ability to bypass those guard rails and write code in a manner that suits me. In this project, I make extensive use of unsafe code for things like accessing raw pointers, reading/writing mutable statics, transmuting pointers to usize and back again, and tons of other things. It's required to do what I want to do in many cases, it's more efficient in others, and it's often just plain easier as well.

Anyway, what I'm trying to say is that you're right - there are more idiomatic and correct ways of dealing with uninitialized memory in Rust. If I were working on a team with other devs or shipping a library, I'd most certainly make efforts to adhere to them. However, in my case, what I have works just fine and the patterns I use make development easier, faster, and more enjoyable for me.

I do appreciate your concern for my project, though. I'm curious - how did you find this concern? Github code search, or just interested in my code and reading through?

[shinmao]

It is really not a big deal because this is not an exploitable API which is exposed to users.

I found it by code search. I am also learning how to deal with uninitialized memory, and exploring the use cases on GitHub. It's great to have a discussion with you.