UB with std::mem::zeroed

[nico-abram]

I think this line

vorbis-rs/src/lib.rs

Line 150 in ef8f99a

let mut callbacks: vorbisfile_sys::ov_callbacks = unsafe { std::mem::zeroed() };

is invoking undefined behaviour

zero is a trap/niche value for fn function pointers (afaik even extern "C" ones, and even for repr(C)). ov_callbacks is a struct of function pointers https://github.com/tomaka/vorbisfile-sys/blob/master/src/lib.rs#L7-L14

The line in queston is initializing an instance of said struct with mem::zeroed, which as far as I understand, is insta-UB

I originally found this because of a runtime panic from rustc 1.49.0-nightly (ffa2e7ae8 2020-10-24) while running the lewton benchmarks

I think the fix would be making the fields in the struct in vorbisfile Option's, and then making the appropiate changes in vorbis-rs

I opened tomaka/vorbisfile-sys#2 to begin that process

[roderickvd]

Besides the route you have been taking, a quick fix may be to change std::mem::zeroed() to std::mem::uninitialized(). This still is not considered as safe as it should be, better Rust parlance seems to be like so: https://doc.rust-lang.org/std/mem/union.MaybeUninit.html

[nico-abram]

Changing it to uninitialized might help get rid of the panic, but it would still be UB AIUI if the struct has any niche values (Like it currently does with 0), and whether or not it is UB if all bit patterns are valid is still in discussion iirc. As you say, MaybeUninit is the preferred alternative

[Johannesd3]

Please take a look at librespot-org/librespot-tremor#2 which was basically the same issue. As a matter of fact, it seems as if the code of that crate was "stolen" from this repository (?).