Closed Wohlstand closed 6 years ago
Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).
My system is Linux Mint 18.2, I using a custom-installed GCC 6.4, and system is 64-bit as build is also 64-bit. Also, at me sanitizer have produced some shared linking errors and lots of linking errors on attempt to link static build. Therefore I have disabled it by a CMake's flag. Maybe I need to try to build it as 32-bit? If that will work in 32-bits, then, you have some cross-architecture mistake related on a type sizes.
Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).
Okay, just now I have built the stuff on my very old computer but on modern Linux Mint with XFCE environment, I have used G++ 5.4 and built with default CMake config cmake ..
. Inability to play those files is still reproducable:
Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).
The full build log where are also two warnings have been found:
vitaly@whl-pc-002l ~/_git_repos/game-music-emu $ mkdir build
vitaly@whl-pc-002l ~/_git_repos/game-music-emu $ cd build/
vitaly@whl-pc-002l ~/_git_repos/game-music-emu/build $ cmake ..
-- The C compiler identification is GNU 5.4.0
-- The CXX compiler identification is GNU 5.4.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test __LIBGME_SWITCH_FALLTHROUGH_WARNINGS
-- Performing Test __LIBGME_SWITCH_FALLTHROUGH_WARNINGS - Failed
** ZLib library located, compressed file formats will be supported
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - not found
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE
SDL library not found, disabling player demo build
-- Configuring done
-- Generating done
-- Build files have been written to: /home/vitaly/_git_repos/game-music-emu/build
vitaly@whl-pc-002l ~/_git_repos/game-music-emu/build $ make
Scanning dependencies of target gme
[ 2%] Building CXX object gme/CMakeFiles/gme.dir/Blip_Buffer.cpp.o
[ 4%] Building CXX object gme/CMakeFiles/gme.dir/Classic_Emu.cpp.o
[ 6%] Building CXX object gme/CMakeFiles/gme.dir/Data_Reader.cpp.o
/home/vitaly/_git_repos/game-music-emu/gme/Data_Reader.cpp: In member function ‘virtual long int Std_File_Reader::read_avail(void*, long int)’:
/home/vitaly/_git_repos/game-music-emu/gme/Data_Reader.cpp:375:27: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
if ( file_ && s > 0 && s <= UINT_MAX ) {
^
In file included from /home/vitaly/_git_repos/game-music-emu/gme/Data_Reader.cpp:21:0:
/home/vitaly/_git_repos/game-music-emu/gme/Data_Reader.cpp: In member function ‘virtual const char* Std_File_Reader::read(void*, long int)’:
/home/vitaly/_git_repos/game-music-emu/gme/Data_Reader.cpp:389:36: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
RETURN_VALIDITY_CHECK( s > 0 && s <= UINT_MAX );
^
/home/vitaly/_git_repos/game-music-emu/gme/blargg_source.h:28:44: note: in definition of macro ‘unlikely’
#define unlikely( x ) __builtin_expect(x, 0)
^
/home/vitaly/_git_repos/game-music-emu/gme/Data_Reader.cpp:389:2: note: in expansion of macro ‘RETURN_VALIDITY_CHECK’
RETURN_VALIDITY_CHECK( s > 0 && s <= UINT_MAX );
^
[ 8%] Building CXX object gme/CMakeFiles/gme.dir/Dual_Resampler.cpp.o
[ 10%] Building CXX object gme/CMakeFiles/gme.dir/Effects_Buffer.cpp.o
[ 12%] Building CXX object gme/CMakeFiles/gme.dir/Fir_Resampler.cpp.o
[ 14%] Building CXX object gme/CMakeFiles/gme.dir/gme.cpp.o
[ 17%] Building CXX object gme/CMakeFiles/gme.dir/Gme_File.cpp.o
[ 19%] Building CXX object gme/CMakeFiles/gme.dir/M3u_Playlist.cpp.o
[ 21%] Building CXX object gme/CMakeFiles/gme.dir/Multi_Buffer.cpp.o
[ 23%] Building CXX object gme/CMakeFiles/gme.dir/Music_Emu.cpp.o
[ 25%] Building CXX object gme/CMakeFiles/gme.dir/Ay_Apu.cpp.o
[ 27%] Building CXX object gme/CMakeFiles/gme.dir/Ym2612_Emu.cpp.o
[ 29%] Building CXX object gme/CMakeFiles/gme.dir/Sms_Apu.cpp.o
[ 31%] Building CXX object gme/CMakeFiles/gme.dir/Ay_Cpu.cpp.o
[ 34%] Building CXX object gme/CMakeFiles/gme.dir/Ay_Emu.cpp.o
[ 36%] Building CXX object gme/CMakeFiles/gme.dir/Gb_Apu.cpp.o
[ 38%] Building CXX object gme/CMakeFiles/gme.dir/Gb_Cpu.cpp.o
[ 40%] Building CXX object gme/CMakeFiles/gme.dir/Gb_Oscs.cpp.o
[ 42%] Building CXX object gme/CMakeFiles/gme.dir/Gbs_Emu.cpp.o
[ 44%] Building CXX object gme/CMakeFiles/gme.dir/Gym_Emu.cpp.o
[ 46%] Building CXX object gme/CMakeFiles/gme.dir/Hes_Apu.cpp.o
[ 48%] Building CXX object gme/CMakeFiles/gme.dir/Hes_Cpu.cpp.o
[ 51%] Building CXX object gme/CMakeFiles/gme.dir/Hes_Emu.cpp.o
[ 53%] Building CXX object gme/CMakeFiles/gme.dir/Kss_Cpu.cpp.o
[ 55%] Building CXX object gme/CMakeFiles/gme.dir/Kss_Emu.cpp.o
[ 57%] Building CXX object gme/CMakeFiles/gme.dir/Kss_Scc_Apu.cpp.o
[ 59%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Apu.cpp.o
[ 61%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Cpu.cpp.o
[ 63%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Fme7_Apu.cpp.o
[ 65%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Namco_Apu.cpp.o
[ 68%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Oscs.cpp.o
[ 70%] Building CXX object gme/CMakeFiles/gme.dir/Nes_Vrc6_Apu.cpp.o
[ 72%] Building CXX object gme/CMakeFiles/gme.dir/Nsf_Emu.cpp.o
[ 74%] Building CXX object gme/CMakeFiles/gme.dir/Nsfe_Emu.cpp.o
[ 76%] Building CXX object gme/CMakeFiles/gme.dir/Sap_Apu.cpp.o
[ 78%] Building CXX object gme/CMakeFiles/gme.dir/Sap_Cpu.cpp.o
[ 80%] Building CXX object gme/CMakeFiles/gme.dir/Sap_Emu.cpp.o
[ 82%] Building CXX object gme/CMakeFiles/gme.dir/Snes_Spc.cpp.o
[ 85%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Cpu.cpp.o
[ 87%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Dsp.cpp.o
[ 89%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Emu.cpp.o
[ 91%] Building CXX object gme/CMakeFiles/gme.dir/Spc_Filter.cpp.o
[ 93%] Building CXX object gme/CMakeFiles/gme.dir/Vgm_Emu.cpp.o
[ 95%] Building CXX object gme/CMakeFiles/gme.dir/Vgm_Emu_Impl.cpp.o
[ 97%] Building CXX object gme/CMakeFiles/gme.dir/Ym2413_Emu.cpp.o
[100%] Linking CXX shared library libgme.so
[100%] Built target gme
Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).
I've reproduced the corrupt file message with current git. I'll see if I can bisect to the relevant commit.
As for the sanitizer flags, I believe those are only supported with shared libs anyways so I probably need to disable one or the other in CMake if both static libs and sanitizer is requested, or abort the build with a message saying to pick one or the other.
Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).
About of sanitizer, the corruption is reproducible with no matter to sanitizer flag state, and with no matter by which I built GME: by original CMake script, by my custom hand-make simplified CMake or by QMake I also made: https://github.com/WohlSoft/AudioCodecs/tree/master/libgme It's probably a mistake in a file reader or in the code that parses NSFE files. And, with no matter static or dynamic build.
Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).
Seems to be commit 205290614cdc057541b26adeea05a9d45993f860, which was applied back in Dec. 2017 to fix a crash and potential out-of-bounds memory access (see Issue #14). That's what git-bisect found and I can confirm that reverting that commit allows the player to operate.
I'm not sure if this means the a2xt-wandering.nsfe file is actually corrupt or whether we need to go and improve that sanity check as I'd mentioned in that commit though.
Original comment by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).
Let's check the NSFE specification and the actual hex data in both those files, there are both existing as-is at 3 years... I bet, when the size is given negative, needed some different logic, OR, that size must be an unsigned value...
Found this: http://wiki.nesdev.com/w/index.php/NSFe And yeah, that value must be unsigned
Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).
nsfe: Some blocks can have a 0-sized header, don't fail for those.
This fixes a regression introduced by commit 205290614cdc057541b26adeea05a9d45993f860, where we added a check for potentially malicious NSFE file dumps to avoid crashes or potential OOB mem access.
It seems some valid files can have 0-sized blocks, so let those through the first check. This allows the testcases for Issue #22 to pass while still rejecting the corrupt file on Issue #14.
Fixes Issue #22.
Original report by Wohlstand (Bitbucket: Wohlstand, GitHub: Wohlstand).
Recently I have found that two of my NSFE files are no more working with GME! However, there are worked fine in previous versions.
The error message I getting is the
Error: Corrupt file
.