ZDoom / ZMusic

GZDoom's music system as a standalone library
https://forum.zdoom.org/index.php
60 stars 32 forks source link

Build fails with Android libc #23

Closed Cacodemon345 closed 3 years ago

Cacodemon345 commented 3 years ago

This is the error I get when I build ZMusic with Android NDK.

[ 76%] Building CXX object source/CMakeFiles/zmusic.dir/mididevices/music_wavewriter_mididevice.cpp.o
/home/caco345/ZMusic/source/mididevices/music_wavewriter_mididevice.cpp:155:39: error: in call to 'fwrite', size * count is too large for the given buffer
                                        if (4 == fwrite(&size, 1, 5, File))
                                                                         ^
/home/caco345/Android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/stdio.h:116:9: note: from 'diagnose_if' attribute on 'fwrite':
        __clang_error_if(__bos_unevaluated_lt(__bos0(buf), size * count),
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/caco345/Android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/sys/cdefs.h:134:52: note: expanded from macro '__clang_error_if'
#define __clang_error_if(cond, msg) __attribute__((diagnose_if(cond, msg, "error")))
                                                   ^           ~~~~
1 error generated.
coelckers commented 3 years ago

Where does that 5 come from?

alexey-lysiuk commented 3 years ago

https://github.com/coelckers/ZMusic/blob/a192e660493b9efaa5268fd1598f7cbe1465238b/source/mididevices/music_wavewriter_mididevice.cpp#L155 Seems to be a typo.

Cacodemon345 commented 3 years ago

This seems to be a buffer overflow bug too. Needs quick fixing.

alexey-lysiuk commented 3 years ago

Not really. The given line will write four intended bytes and one junk byte to a file. It's impossible to alter stack or heap. The most severe effect would be a crash if the next byte after size variable cannot be read.

alexey-lysiuk commented 3 years ago

Does it compile with ba9e9da?

Cacodemon345 commented 3 years ago

Yep. But I am getting this warning:

/home/caco345/ZMusic/source/zmusic/zmusic.cpp:506:2: warning: deleting pointer to incomplete type '__sFILE' may cause undefined behavior [-Wdelete-incomplete]
        delete f;
        ^      ~
/home/caco345/Android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/stdio.h:58:8: note: forward declaration of '__sFILE'
struct __sFILE;
       ^
1 warning generated.

The FILE struct is an incomplete type when building for Android Nougat and later. delete f should be changed to fclose(f).

alexey-lysiuk commented 3 years ago

Done.