Optiroc / SuperFamiconv

Flexible and composable tile graphics converter supporting Super Nintendo, Game Boy, Game Boy Color, Game Boy Advance, Mega Drive, PC Engine and WonderSwan formats.
MIT License
145 stars 20 forks source link

palette and tiles commands segfault/abort #1

Closed ARM9 closed 7 years ago

ARM9 commented 7 years ago
*** Error in `./superfamiconv': double free or corruption (fasttop): 0x000000000065fc20 ***
======= Backtrace: =========
...
Program received signal SIGABRT, Aborted.
0x00007ffff7a43428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54

when running $ superfamiconv tiles and $ superfamiconv palette, but not superfamiconv or superfamiconv map, it crashes regardless of which combination of flags I use. If I run with -v I'll get something like:

$ superfamiconv palette -i bg.png -d out.pal -v
Loaded image from "bg.png" (128x128, indexed color)
Mapping optimized palette (8x16 color palettes, 8x8 tiles)
Setting color zero to #080417
Generated palette with [16] colors, 16 total
Saved native palette data to "out.pal"
Segmentation fault

Only tested on lubuntu 16.04 yet, if it's not a bug in one of your libraries it could have something to do with the way they handle c++ exceptions.

Optiroc commented 7 years ago

Interesting! In the last log it seems to have already carried out all the work when crashing... I'm guessing it segfaults when some destructor gets called. Did out.pal get written? The tool has really only been tested with osx/clang/libc++ so far, so I really appreciate the crash report. Could you send the input data you're testing with?

I'll try compiling with g++/libstdc++ and see if I can provoke some bad behaviour...

Does it crash even when only invoked with for example superfamiconv palette -h?

ARM9 commented 7 years ago

superfamiconv (palette|tiles) with or without -h crashes as demonstrated in the first log, or any input that leads to a throw statement it seems. However if I make it do work, without reaching a throw statement, it segfaults when done sans stacktrace. You're likely correct about it happening in some destructor.

I forgot to mention that the output is generated just fine for both palette and tiles modes, it seems to segfault regardless of what I throw at it but here's one.

Optiroc commented 7 years ago

OK. Hmm. Can you try building with make DEBUG=1 and see if something helpful is logged upon stack unwinding?

ARM9 commented 7 years ago

I was able to track down the cause:

g++  -D SFC_MONOLITH .build/superfamiconv.o .build/sfc_palette.om .build/sfc_tiles.om .build/sfc_map.om .build/Image.o .build/Palette.o .build/Tiles.o .build/Map.o .build/LodePNG/lodepng.o -o bin/superfamiconv
src/superfamiconv.cpp:16:8: warning: type ‘struct Settings’ violates one definition rule [-Wodr]
 struct Settings {
        ^
src/sfc_tiles.cpp:12:8: note: a different type is defined in another translation unit
 struct Settings {

When I debugged it I found that it crashed on the first item in Settings::~Settings(). sfc_tiles, sfc_palette and sfc_map all duplicated struct Settings, but sfc_map was the only ones default destructor which accidentally matched the one defined in superfamiconv.cpp (6 std::strings total). Thus Settings::~Settings() was trying to delete the 6th string which was absent in sfc_tiles and sfc_palette. Curious that clang++ didn't report this despite the compiled binary segfaulting, only got the warning when compiling with g++.

Submitting a PR.

Optiroc commented 7 years ago

Doh! Nice find. Quite surprising that clang didn't catch that, indeed..

-david

On 10 Feb 2017, at 18:32, ARM9 notifications@github.com wrote:

I was able to track down the cause:

g++ -D SFC_MONOLITH .build/superfamiconv.o .build/sfc_palette.om .build/sfc_tiles.om .build/sfc_map.om .build/Image.o .build/Palette.o .build/Tiles.o .build/Map.o .build/LodePNG/lodepng.o -o bin/superfamiconv src/superfamiconv.cpp:16:8: warning: type ‘struct Settings’ violates one definition rule [-Wodr] struct Settings { ^ src/sfc_tiles.cpp:12:8: note: a different type is defined in another translation unit struct Settings { When I debugged it I found that it crashed on the first item in Settings::~Settings(). sfc_tiles, sfc_palette and sfc_map all duplicated struct Settings, but sfc_map was the only ones default destructor which accidentally matched the one defined in superfamiconv.cpp (6 std::strings total). Thus Settings::~Settings() was trying to delete the 6th string which was absent in sfc_tiles and sfc_palette. Curious that clang++ didn't report this despite the compiled binary segfaulting, only got the warning when compiling with g++.

Submitting a PR.

― You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.