search

nothings / stb

stb single-file public domain libraries for C/C++
https://twitter.com/nothings
Other
25.83k stars 7.66k forks source link

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

Open Seb-degraff opened 8 months ago

Seb-degraff commented 8 months ago

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
DanielGibson commented 5 months ago

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 commented 5 months ago

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