GNOME Bugzilla – Bug 704510
Gimp Git Segfaults on the new language module
Last modified: 2013-07-19 12:20:40 UTC
My recent (last built a few moment ago) git builds (2.9) have been instantly segfaulting on MBR running Mountain Lion. gdb backtrace (or commandline execution) shows: ./gimp-2.9 (pid:42315): [E]xit, [H]alt, show [S]tack trace or [P]roceed: S
+ Trace 232267
and gimp-2.9 --verbose ... Parsing '/Users/partha/Library/Application Support/GIMP/2.9/tool-options/gimp-text-tool' ./gimp-2.9: fatal error: Segmentation fault: 11 ./gimp-2.9 (pid:42330): [E]xit, [H]alt, show [S]tack trace or [P]roceed: See the following thread: https://mail.gnome.org/archives/gimp-developer-list/2013-July/msg00044.html Thanks, Partha
Copy from what I say on the ml: ----- I searched a little more though, and it seems on BSDs, hence on OSX, indeed setenv with a NULL value could crash the program. The setenv in GNU libc on the other hand perfectly handles the case explicitly. So obviously when I see this kind of code (note I am not 100% sure this is the code for Darwin on Mountain Lion but I can't find a reference linking the libc numbers there and the Darwin version 10.8, but I assume that should be a similar code): http://www.opensource.apple.com/source/Libc/Libc-825.26/stdlib/FreeBSD/setenv.c Before any test on the value pointer, it dereferences it (which is undefined!), and read the content of the non-existing first character of the NULL string (which I assume would crash!): if (*value == '=') /* no `=' in value */ ++value; I don't know what is the policy on BSD but I thought they were very keen on security, but this code does not look very sane to me. So yeah anyway that's a problem too in the end. I'll deal with it. ------ Note that on the ML, I also realized by reading better the glib doc that setenv/getenv may not handle well multi-threading because various calls reuse the same buffers (and gettext calls it too). So in the end, I think that's not the problem here, but I will still want to improve the code on this side too.
Hi, I made a small commit to handle directly the no setenv() of a NULL value issue: commit aa67bec9f0e8f551bec9c070f22fe8006bc15929 Author: Jehan <jehan@girinstud.io> Date: Fri Jul 19 11:39:50 2013 +0900 Bug 704510 - GIMP segfaults on OSX. setenv() does not behave well on some systems, in particular OSX (and probably some BSDs), when the set value is NULL. In this case, let's unsetenv() the environment variable instead. Mitch, I did not follow the usual procedure of code review here and pushed directly this minor change, because the user is pretty reactive on testing and I am pretty sure this should fix his immediate issue. Moreover in any case, I believe this small fix being probably a better code anyway, even on Linux. Sorry. I hope that's ok! In any case, I will also check this issue where I saw that docs says setenv/getenv are not thread safe. And for this bigger change, I will obviously pass you the usual review.
Partha said the crash was fixed with this commit on the mailing list.
Cannot confirm fixed: Current build on OS X 10.7.5 now crashes when opening the preferences dialog: $ ./gimp-2.9 --verbose <...> Writing '/Users/su_v/Library/Application Support/GIMP/2.9/pluginrc' Starting extension: 'extension-script-fu' INIT: gui_restore_after_callback Parsing '/Users/su_v/Library/Application Support/GIMP/2.9/menurc' loading menu '/Volumes/magenta/mp-trunk/src/gimp/local/share/gimp/2.0/menus/image-menu.xml' for /dummy-menubar Parsing '/Users/su_v/Library/Application Support/GIMP/2.9/devicerc' Parsing '/Users/su_v/Library/Application Support/GIMP/2.9/controllerrc' Parsing '/Users/su_v/Library/Application Support/GIMP/2.9/colorrc' Loading module '/Volumes/magenta/mp-trunk/src/gimp/local/lib/gimp/2.0/modules/libdisplay-filter-lcms.so' ./gimp-2.9: fatal error: Segmentation fault: 11 ./gimp-2.9 (pid:54992): [E]xit, [H]alt, show [S]tack trace or [P]roceed: s
+ Trace 232269
(script-fu:55164): LibGimpBase-WARNING **: script-fu: gimp_wire_read(): error $
Right. I am stupid. There was 2 pieces of code with basically the same issue! commit 237067039629c946302ad82b680cb8b21898274e Author: Jehan <jehan@girinstud.io> Date: Fri Jul 19 20:49:40 2013 +0900 Bug 704510 - GIMP segfaults on OSX. Forgot a setenv() with possible a NULL value. Please try again and tell me if it is better. :-)
Well, I didn't try to open the preference dialog either. :) Anyway, it's fixed for me now. I opened the preference dialog without a crash.
Fix for preferences dialog confirmed on OS X 10.7.5, too - thx :)
Cool. :-)