Re: Glib 1.102 (stable)

[muppet <scott asofyet org>]

On Jan 13, 2006, at 6:21 PM, Sean Dague wrote:

> The following program was provided by one of our users that  
> demonstrates the
> glib crash we're seeing.  Hopefully this will help figure out if  
> there is a
> bug in glib, or a bug in our brains, and we should be doing something
> different.

Many thanks, i wouldn't have found this without your code.

Wow, how embarrassing.  This is an overlooked problem with the undead  
objects patch from last fall.

The tipoff was that i could keep the crash from happening by  
providing a SET_PROPERTY() in your class.  This avoids all of the  
fallback-to-hash-key machinery.

1. The cell renderer is created and added to the column in the view.
2. The surrounding block goes out of scope, and the cell renderer's  
wrapper hash is marked "undead".
3. From $window->show_all, the treeview starts to paint itself, and  
needs to render columns.
4. TreeViewColumn calls g_object_set_property() on the  
CrashTest::CellRenderer.
5. We wind up in gperl_type_set_property(), as this is a perl-derived  
class.
6. gperl_type_set_property() sees that  
CrashTest::CellRenderer::SET_PROPERTY does not exist, so it goes into  
the code that will use the key in the wrapper hash.
7. It calls _gperl_fetch_wrapper_key() to get the wrapper key from  
the object.
8. _gperl_fetch_wrapper_key() calls g_object_get_qdata() to fetch the  
wrapper HV.
9. _gperl_fetch_wrapper_key() passes the wrapper HV to hv_fetch().
10. BOOM!

It goes boom because in step 2, the wrapper hash was marked undead.   
This happens by using an evil C trick, which is manipulating the low  
bits in the pointer value, which are unused due to word alignment.   
However, _gperl_fetch_wrapper_key() doesn't account for the fact that  
the pointer may have been adjusted to mark the wrapper as undead, and  
in this case, that's exactly what happened.  We wind up passing an  
invalid pointer to hv_fetch(), and the world comes crashing down  
around us.

Note that this didn't happen when you removed the braces because you  
didn't allow $renderer to go out of scope and become undead!

The fix is a one-liner, applied to both stable-1-10 and HEAD.

Index: GObject.xs
===================================================================
RCS file: /cvsroot/gtk2-perl/gtk2-perl-xs/Glib/GObject.xs,v
retrieving revision 1.48.2.2
diff -u -r1.48.2.2 GObject.xs
--- GObject.xs  13 Nov 2005 18:07:10 -0000      1.48.2.2
+++ GObject.xs  14 Jan 2006 02:43:35 -0000
@@ -768,6 +768,11 @@
        SV * svname;
        HV * wrapper_hash;
        wrapper_hash = g_object_get_qdata (object, wrapper_quark);
+
+       /* we don't care whether the wrapper is alive or undead.  forcibly
+        * remove the undead bit, or the pointer will be unusable. */
+       wrapper_hash = REVIVE_UNDEAD (wrapper_hash);
+
        svname = newSVpv (name, strlen (name));
        svp = hv_fetch (wrapper_hash, SvPV_nolen (svname), SvLEN (svname)-1,
                        FALSE); /* never create on the first try; prefer


I do apologize, as i should've caught that in patch review.


--
"Ghostbusters" is the best movie of this decade.
  -- Neal, circa 1996, referring to a movie released in 1984.