GNOME Bugzilla – Bug 661709
Crash when calling Gtk.targets_include_uri: unable to copy GdkAtom array elements
Last modified: 2012-04-24 04:49:51 UTC
Run the following to crash python: from gi.repository import Gtk, Gdk Gtk.targets_include_uri([Gdk.Atom.intern('text/plain', False)]) This is apparently because Gdk.Atoms look to PyGI like a struct pointer, but shouldn't be dereferenced. That's why they are declared with disguised="1" in the GIR file. I've tracked this down to _pygi_marshal_from_py_array() needing to check an item's disguised value before calling memcpy on it's item.v_pointer member. But I'm not sure how to check the disguised value...
I can confirm this bug >>> from gi.repository import Gtk, Gdk >>> Gtk.targets_include_uri([Gdk.Atom.intern('text/plain', False)]) Program received signal SIGSEGV, Segmentation fault. __memcpy_ssse3_rep () at ../sysdeps/i386/i686/multiarch/memcpy-ssse3-rep.S:1412 1412 ../sysdeps/i386/i686/multiarch/memcpy-ssse3-rep.S: No such file or directory. in ../sysdeps/i386/i686/multiarch/memcpy-ssse3-rep.S (gdb) thread apply all bt
+ Trace 228791
Thread 1 (Thread 0xb7fdd6c0 (LWP 15036))
See bug 560248 for what disguised items are. (Note I think there's a bug where we mark other things as disguised =( )
The same applies to ia32:
+ Trace 228895
Created attachment 212500 [details] Test case Still applies to latest version of pygobject:
+ Trace 230115
Some notekeeping: Functions which take a single GdkAtom work: >>> atom = Gdk.Atom.intern('default', False) >>> Gtk.Clipboard.get(atom) In the array case, we crash in _pygi_marshal_from_py_array(). GdkAtom is not a boxed struct, so we hit this code path: } else if (!is_boxed) { memcpy (array_->data + (i * item_size), item.v_pointer, item_size); However, item is not a pointer: (gdb) p item $24 = {v_boolean = 120, v_int8 = 120 'x', v_uint8 = 120 'x', v_int16 = 120, v_uint16 = 120, v_int32 = 120, v_uint32 = 120, v_int64 = 120, v_uint64 = 120, v_float = 1.68155816e-43, v_double = 5.9287877500949585e-322, v_short = 120, v_ushort = 120, v_int = 120, v_uint = 120, v_long = 120, v_ulong = 120, v_ssize = 120, v_size = 120, v_string = 0x78 <Address 0x78 out of bounds>, v_pointer = 0x78} So it seems _pygi_marshal_from_py_interface_struct() did not return a valid GdkAtom GIArgument pointer. Stepping through now..
I now read how Gdk handles GdkAtoms: This is really just an int pretending to be a pointer, so you must never actually dereference its value. I don't want to hardcode the GdkAtom case into pygobject, so I think it would be better to box that type.
Created attachment 212595 [details] [review] Add tests for GdkAtom handling This converts the test case into a proper unittest format for pygobject. While I was at it, I also added tests for "single Atom return", "single Atom argument in", and Atom GList return", which work fine.
When I add _gdk_atom_{copy,free}() methods and try to box GdkAtom, I get gdkdisplaymanager.c: In function 'gdk_atom_get_type': gdkdisplaymanager.c:476:784: error: incompatible type for argument 2 of '_g_register_boxed' gdkdisplaymanager.c:476:784: note: expected 'union <anonymous>' but argument is of type 'struct _GdkAtom * (*)(struct _GdkAtom *)' gdkdisplaymanager.c:476:784: error: incompatible type for argument 3 of '_g_register_boxed' gdkdisplaymanager.c:476:784: note: expected 'union <anonymous>' but argument is of type 'void (*)(struct _GdkAtom *)' I. e. it can't be boxed either, at least not the usual way. At this point my wisdom about boxed types ends, I'm afraid.
Created attachment 212600 [details] [review] Add hack for Gdk.Atom array entries from Python This is a really evil hack, as it hardcodes the Gdk.Atom special case. If anyone has an idea how to generalize that, I'm all ears.
Review of attachment 212600 [details] [review]: For hack this is quite small and you can still follow the logic behind it. Due to lack of a better solution I'm for it. ::: gi/pygi-marshal-from-py.c @@ +861,3 @@ + if (g_strcmp0 (item_iface_cache->type_name, "Gdk.Atom") == 0) { + g_assert (item_size == sizeof (item.v_pointer)); + memcpy (array_->data + (i * item_size), &item.v_pointer, item_size); You are not calling "from_py_cleanup" in the Gdk.Atom case. Not sure if that's an issue, just something I noticed.
(In reply to comment #10) > For hack this is quite small and you can still follow the logic behind it. Due > to lack of a better solution I'm for it. OK, thanks; I'll add a comment to it saying that this would better be generalized. > ::: gi/pygi-marshal-from-py.c > @@ +861,3 @@ > + if (g_strcmp0 (item_iface_cache->type_name, > "Gdk.Atom") == 0) { > + g_assert (item_size == sizeof (item.v_pointer)); > + memcpy (array_->data + (i * item_size), > &item.v_pointer, item_size); > > You are not calling "from_py_cleanup" in the Gdk.Atom case. Not sure if that's > an issue, just something I noticed. I actually did that on purpose. For a Gdk.Atom item from_py_cleanup is NULL anyway (and better be), but as again the v_pointer must neither be referenced nor freed, it's safer to not even try calling a cleanup function. In the end it's not much difference either way, though.
Pushed: http://git.gnome.org/browse/pygobject/commit/?id=8ee21619b3cfc179cf114813478470d9aa3f6fb8