Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: undefined behavior when transmuting u32 to various enums #1388

Open
antonilol opened this issue Apr 23, 2024 · 7 comments
Open

bug: undefined behavior when transmuting u32 to various enums #1388

antonilol opened this issue Apr 23, 2024 · 7 comments
Labels

Comments

@antonilol
Copy link
Contributor

antonilol commented Apr 23, 2024

in several places in the code a u32 is transmuted to enums without any checks. examples include

Ok(match unsafe { transmute(n) } {
,
Ok(match unsafe { transmute(n) } {
and
Ok(match unsafe { transmute(n) } {
.

these TryInto implementations are public so this can be easily demonstrated by using miri with the code let b: BlendMode = 3.try_into() (this calls <BlendMode as TryFrom<u32>>::try_from(3) with the invalid value 3) for example:

error: Undefined Behavior: constructing invalid value at .<enum-tag>: encountered 0x00000003, but expected a valid enum tag
   --> /home/antoni/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sdl2-0.36.0/src/sdl2/render.rs:172:27
    |
172 |         Ok(match unsafe { transmute(n) } {
    |                           ^^^^^^^^^^^^ constructing invalid value at .<enum-tag>: encountered 0x00000003, but expected a valid enum tag
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
    = note: inside `<sdl2::render::BlendMode as std::convert::TryFrom<u32>>::try_from` at /home/antoni/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sdl2-0.36.0/src/sdl2/render.rs:172:27: 172:39
    = note: inside `<u32 as std::convert::TryInto<sdl2::render::BlendMode>>::try_into` at /home/antoni/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:798:9: 798:26
@Cobrand Cobrand added the bug label Apr 24, 2024
@jagprog5
Copy link

jagprog5 commented Feb 5, 2025

@antonilol this is fixed in my merge request: #1444

cc @Cobrand

@antonilol
Copy link
Contributor Author

@antonilol this is fixed in my merge request: #1444

Not completely, a quick search found (every line containing transmute and match):

  • src/sdl2/event.rs:420 (probably alright)
  • src/sdl2/audio.rs:340 (unsound)
  • src/sdl2/keyboard/scancode.rs:261 (unsound)
  • src/sdl2/keyboard/keycode.rs:486 (unsound)

There could be more, so please make sure you got them all before closing this issue.

@jagprog5
Copy link

jagprog5 commented Feb 7, 2025

@antonilol Thanks, I've updated my comment in the MR to reflect this.

I've decided not to amend the MR further:

  • scope creep
  • I think this repo is no longer active, which would be a shame. @Cobrand @AngryLawyer looking for maintainers?

@antonilol
Copy link
Contributor Author

  • I think this repo is no longer active, which would be a shame.

Yes, even if people switch to SDL3 (I don't know how stable the Rust bindings for SDL3 are already), people still use SDL2 so at least undefined behavior should be fixed here.
Also, the SDL3 bindings repo was forked from this repo, so fixes could potentially be reused there.

@Cobrand
Copy link
Member

Cobrand commented Feb 7, 2025

I think this repo is no longer active, which would be a shame.

It depends what you call maintained. I still look at PRs and merge them from time to time, although probably not as often as some of you would expect. If it was as easy as click "merge", I would do it more often, but unfortunately you have to read them, ask for changes, wait for them to answer, and if they don't take their changes to adapt them and merge them, sometimes fix merge conflicts, and that for every single PR.

Yes I do need maintainers or help in general, I have been calling it for years. Originally I just offered to take a bit of the burden, next thing I know I'm the only maintainer left and no one is stepping up. It has been this way for 8 years, almost 9 now. I think you can understand that it's not always easy to look at everything that's being thrown at you every week for that long while still keeping decent delays.

Sorry to be a bit pissy but it's a bit like the "no take, only throw" image, people expect it to be maintained, but no one is stepping up to help either.

Yes I will look at the issues/PRs at some point, no I don't have an estimate of when I will, yes help would be really appreciated. Thanks.

@jagprog5
Copy link

jagprog5 commented Feb 7, 2025

Yessir, I totally get it. Open source is thankless and this is a big repo, especially for one person. How can I help out? Can I be a maintainer?

@Cobrand
Copy link
Member

Cobrand commented Feb 8, 2025

Currently I'm trying to contact AngryLawyer (the original repo owner), so that I can add other members to the organization myself, once I can do that I'll definitely add new maintainers yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants