Skip to content
This repository has been archived by the owner on Mar 27, 2024. It is now read-only.

Undefined behavior: zero-initialization of Event #131

Closed
askeksa opened this issue Sep 3, 2020 · 4 comments · Fixed by #138
Closed

Undefined behavior: zero-initialization of Event #131

askeksa opened this issue Sep 3, 2020 · 4 comments · Fixed by #138
Assignees

Comments

@askeksa
Copy link
Contributor

askeksa commented Sep 3, 2020

As I understand it, we need to fix this to prevent this code from panicking in a future version of Rust.

Quoting @RalfJung from the discussion of #112:

The bug is here:

/// # let mut event: *mut Event = &mut unsafe { std::mem::zeroed() };

and here:

/// # let mut event: *mut Event = &mut unsafe { std::mem::zeroed() };

EventType contains no variant with tag discriminant 0, so creating one via mem::zeroed() is Undefined Behavior.

@RalfJung
Copy link

RalfJung commented Sep 3, 2020

Yes, thanks for opening an issue. :)

The easiest fix is probably to have some variant with discriminant 0, that would then be the one that is used by a std::mem::zeroed() event. Alternatively, maybe you could avoid unsafe code entirely and derive(Default) for Event, and then impl Default for EventType to determine what the default event type should be.

@Boscop
Copy link
Member

Boscop commented Oct 23, 2020

I just ran into this, my plugin panics when I try to load it, e.g. in the simple_host that's part of the examples.
It crashes in this line:

let api_events = vec![unsafe { mem::zeroed::<PlaceholderEvent>() }; capacity];

because SysExEvent contains an EventType.

Could we please fix this soon? :)
(I really need this VST to function..)

thread '<unnamed>' panicked at 'attempted to zero-initialize type `api::SysExEve
nt`, which is invalid', /rustc/b1496c6e606dd908dd651ac2cce89815e10d7fc5\library\
core\src\mem\mod.rs:623:9
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/b1496c6e606dd908dd651ac2cce89815e10d7fc5\/library\std\src
\panicking.rs:483
   1: core::panicking::panic_fmt
             at /rustc/b1496c6e606dd908dd651ac2cce89815e10d7fc5\/library\core\sr
c\panicking.rs:85
   2: core::panicking::panic
             at /rustc/b1496c6e606dd908dd651ac2cce89815e10d7fc5\/library\core\sr
c\panicking.rs:50
   3: core::mem::zeroed
             at /rustc/b1496c6e606dd908dd651ac2cce89815e10d7fc5\library\core\src
\mem\mod.rs:623
   4: vst::buffer::SendEventBuffer::new
             at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\vst-
0.2.0\src\buffer.rs:397
   5: vst::buffer::{{impl}}::default
             at C:\Users\me\.cargo\registry\src\github.com-1ecc6299db9ec823\vst-
0.2.0\src\buffer.rs:386

@RalfJung
Copy link

Indeed, the UB check that causes a panic here has landed in the mean time. Cc rust-lang/rust#66151

@askeksa
Copy link
Contributor Author

askeksa commented Oct 23, 2020

I will take a look at this over the weekend.

@askeksa askeksa self-assigned this Oct 23, 2020
askeksa added a commit that referenced this issue Nov 7, 2020
#138)

This avoids panicking on Rust 1.48 and later.

Fixes #131
askeksa added a commit to askeksa/vst-rs that referenced this issue Nov 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants