GNOME Bugzilla – Bug 456137
Pidgin will crash in fail-safe session due to a NULL pointer in gtk+
Last modified: 2007-10-16 20:09:52 UTC
This is coming from a pidgin bug http://developer.pidgin.im/ticket/842 Pidgin will crash in fail-safe session or some themes in Solaris. due to a NULL pointer in gtk+. The useful stack trace is: t@1 (l@1) signal SEGV (no mapping at the fault address) in render_icon_name_pixbuf at line 1470 in file "gtkiconfactory.c" 1470 g_warning ("Error loading theme icon for stock: %s", error->message); (dbx) where current thread: t@1 =>[1] render_icon_name_pixbuf(icon_source = 0x19be18, style = 0x19e308, direction = GTK_TEXT_DIR_LTR, state = GTK_STATE_NORMAL, size = -1, widget = 0x15d420, detail = (nil)), line 1470 in "gtkiconfactory.c" [2] find_and_render_icon_source(icon_set = 0x198a20, style = 0x19e308, direction = GTK_TEXT_DIR_LTR, state = GTK_STATE_NORMAL, size = -1, widget = 0x15d420, detail = (nil)), line 1540 in "gtkiconfactory.c" [3] gtk_icon_set_render_icon(icon_set = 0x198a20, style = 0x19e308, direction = GTK_TEXT_DIR_LTR, state = GTK_STATE_NORMAL, size = -1, widget = 0x15d420, detail = (nil)), line 1644 in "gtkiconfactory.c" [4] gtk_widget_render_icon(widget = 0x15d420, stock_id = 0xf60d8 "gtk-close", size = -1, detail = (nil)), line 5582 in "gtkwidget.c" [5] pidgin_blist_show(list = 0x2006b0), line 4285 in "gtkblist.c" [6] purple_blist_show(), line 707 in "blist.c" [7] main(argc = 1, argv = 0xffbffb8c), line 787 in "gtkmain.c" (dbx) Chris will post a patch for 2.11.x soon.
Created attachment 91915 [details] [review] The problem caused by the null pointer
Hard to see how that function can return NULL without emitting a critical warning first. Do you see any g_return_if_fail() output on the console before the crash ? Our official stance on such crashes is that all bets are off after a critical warning. Of course, it would still be good to track down the critical warning and fix that.
Hi, I was the Solaris builder who originally posted this issue on the Pidgin bugtracker a few months ago and I was just wondering what is happening with a fix... do I understand this correctly that you're pointing the finger back at the Pidgin developers again? If so, I'd like someone to confirm that so I can reopen the original ticket over there with the updated comments. Our group still has to patch the source in order to run without segfaulting in Solaris. I'd just like to get it taken care of by whoever can. Thanks.
Chris will do some investigations next week to see why the error is NULL.
Hi all, just wondered if there was any luck with this bug since it's been a couple of weeks. Thanks.
I evaluated this bug again, and find out the root cause of the bug is as following: 1. pidgin call gtk_widget_render_icon with size=-1 hard coded. 2. in function render_icon_name_pixbuf, it process the case size is -1 if gtk_icon_size_lookup_for_setting return false. 3. gtk_icon_size_lookup_for_setting calls icon_size_lookup_intern 4. in function icon_size_lookup_intern, it return FALSE only if size >= icon_sizes_used, or size == GTK_ICON_SIZE_INVALID (which is 0). so, if we pass size =-1 to gtk_icon_size_lookup_for_setting, it will never return FALSE, thus, the statement size == -1 in render_icon_name_pixbuf will never reached. and the value of pixel_value is given 0. In this case, the tmp_pixbuf is NULL with no error code. however, I think to process the null pointer is also useful I attached new patch for this fix
Created attachment 95842 [details] [review] new patch
Chris, great to hear that some work has been done on this. I'm just curious though, when I downloaded and opened up the 2.12.0 source, the second part of your patch was already applied. I first noticed it when I got a 1 of 2 hunk failure. It looks like what you're attempting was already upstreamed at some point. I just wanted to mention that for anyone using the patch. The only place I had seen this: icon_source->source.icon_name, error && error->message?error->message:"NULL"); if (error) g_error_free (error); ...was in the temp fix they posted on the pidgin site.
Created attachment 95892 [details] [review] revised patch Sorry, I used the wrong original file, please use this patch instead
Thanks for tracking this down. It looks like there are a couple of other places where we should handle (GtkIconSize)-1. Can you check if the attached patch works for you ?
Created attachment 95960 [details] [review] iconsize.patch
Matthias, I just tested your patch,it works for me, please try to commit it.
2007-10-16 Matthias Clasen <mclasen@redhat.com> * gkt/gtkiconsize.c: Be more careful when handling (GtkIconSize)-1. (#456137, Harry Lu, Chris Wang, et al)