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

stb_vorbis: undefined behaviour causes audio distortions when ubsan is enabled #1563

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Seb-degraff
Copy link

@Seb-degraff Seb-degraff commented Oct 22, 2023

Fixes distorted decompressed audio when UBSan was enabled.

I found an audio distortion issue while working on a game. This bug seems petty harmless since it shows up only when ubsan is enabled (for me at least). However – since I was not listening to audio at the time at enabled ubsan – it took me a little while to link the audio distortion bug to the fact that ubsan was enabled. Fixing this might save others the trouble!

Distorted audio (ubsan enabled): https://github.com/nothings/stb/assets/6556843/a92daa96-4ac0-4c3f-aa03-4e58d161dd78
Correct audio (ubsan disabled): https://github.com/nothings/stb/assets/6556843/a932be22-d25d-4158-8f16-e3effc583416

Minimal repro: stb_vorbis_ubsan_bug_repro.zip

Running run_repro.sh on my arm mac results in this input showing that the decoded data with and without ubsan don't match:

> Compiling and running without ubsan
Decoded 3309167 samples.
> done

> Compiling and running with ubsan
stb_vorbis.c:2078:10: runtime error: index -128 out of bounds for type 'float[256]'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior stb_vorbis.c:2078:10 in 
stb_vorbis.c:2070:7: runtime error: index -120 out of bounds for type 'float[256]'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior stb_vorbis.c:2070:7 in 
Decoded 3309167 samples.
> done

> Comparing the two decoded files
decoded.data decoded_ubsan.data differ: char 57, line 1
> done

> Compiling and running the patched version without ubsan
Decoded 3309167 samples.
> done

> Compiling and running the patched version with ubsan
Decoded 3309167 samples.
> done

> Comparing the two decoded files
Files matched!
> done

Fixes distorted decompressed audio when UBSan was enabled.
@Seb-degraff Seb-degraff changed the title [stb_vorbis.c] undefined behaviour stb_vorbis: undefined behaviour causes audio distortions when ubsan is enabled Oct 22, 2023
sezero added a commit to sezero/libxmp that referenced this pull request Nov 7, 2023
Patch by Seb de Graffenried (@Seb-degraff): nothings/stb#1563
Fixes distorted decompressed audio when UBSan was enabled.
sezero added a commit to icculus/SDL_sound that referenced this pull request Nov 7, 2023
Patch by Seb de Graffenried (@Seb-degraff): nothings/stb#1563
Fixes distorted decompressed audio when UBSan was enabled.
sezero added a commit to libsdl-org/SDL_mixer that referenced this pull request Nov 7, 2023
Patch by Seb de Graffenried (@Seb-degraff): nothings/stb#1563
Fixes distorted decompressed audio when UBSan was enabled.
(cherry picked from commit c015a42)
sezero added a commit to libsdl-org/SDL_mixer that referenced this pull request Nov 7, 2023
Patch by Seb de Graffenried (@Seb-degraff): nothings/stb#1563
Fixes distorted decompressed audio when UBSan was enabled.
AliceLR pushed a commit to libxmp/libxmp that referenced this pull request Nov 7, 2023
Patch by Seb de Graffenried (@Seb-degraff): nothings/stb#1563
Fixes distorted decompressed audio when UBSan was enabled.
Wohlstand added a commit to WohlSoft/SDL-Mixer-X that referenced this pull request Nov 8, 2023
Patch by Seb de Graffenried (@Seb-degraff): nothings/stb#1563
Fixes distorted decompressed audio when UBSan was enabled.

Backported from mainstream: libsdl-org/SDL_mixer@c015a42
sezero pushed a commit to sezero/stb that referenced this pull request Dec 12, 2023
@DanielGibson
Copy link
Contributor

DanielGibson commented Feb 3, 2024

This is weird - I don't see how y&255 (with int y) should be undefined behavior, or return negative results. Regardless of the value of y, it should return a value between 0 and 255. Is it possible that your compiler has a bug?

(and FWIW, on my Linux amd64 system, with clang 14, your repro always says Files matched! - of course there could be platform-specific differences to ARM64 and/or Mac, but as I wrote, I don't see how the original code could possibly be wrong)

@Seb-degraff
Copy link
Author

Oh interesting! Thank you for looking into it. I’ll do more testing on my side

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

Successfully merging this pull request may close these issues.

None yet

2 participants