After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 661709 - Crash when calling Gtk.targets_include_uri: unable to copy GdkAtom array elements
Crash when calling Gtk.targets_include_uri: unable to copy GdkAtom array elem...
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
3.0.x
Other Linux
: Normal critical
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2011-10-13 20:18 UTC by Michael Terry
Modified: 2012-04-24 04:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case (111 bytes, text/x-python)
2012-04-21 12:53 UTC, Sebastian Pölsterl
  Details
Add tests for GdkAtom handling (2.03 KB, patch)
2012-04-23 10:35 UTC, Martin Pitt
none Details | Review
Add hack for Gdk.Atom array entries from Python (4.29 KB, patch)
2012-04-23 13:01 UTC, Martin Pitt
none Details | Review

Description Michael Terry 2011-10-13 20:18:14 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...
Comment 1 Fabio Durán Verdugo 2011-10-14 00:07:20 UTC
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

Thread 1 (Thread 0xb7fdd6c0 (LWP 15036))

  • #0 __memcpy_ssse3_rep
    at ../sysdeps/i386/i686/multiarch/memcpy-ssse3-rep.S line 1412
  • #1 _pygi_marshal_from_py_array
    at /usr/include/i386-linux-gnu/bits/string3.h line 52
  • #2 _invoke_marshal_in_args
    at /build/buildd/pygobject-3.0.0/gi/pygi-invoke.c line 483
  • #3 _wrap_g_callable_info_invoke
  • #4 ext_do_call
    at ../Python/ceval.c line 4331
  • #5 PyEval_EvalFrameEx
  • #6 PyEval_EvalCodeEx
  • #7 fast_function
    at ../Python/ceval.c line 4117
  • #8 call_function
    at ../Python/ceval.c line 4042
  • #9 PyEval_EvalFrameEx
  • #10 PyEval_EvalCodeEx
  • #11 PyEval_EvalCode
  • #12 run_mod
  • #13 PyRun_InteractiveOneFlags
    at ../Python/pythonrun.c line 845
  • #14 PyRun_InteractiveLoopFlags
    at ../Python/pythonrun.c line 765
  • #15 PyRun_AnyFileExFlags
    at ../Python/pythonrun.c line 734
  • #16 Py_Main
    at ../Modules/main.c line 599
  • #17 main
    at ../Modules/python.c line 23

Comment 2 Colin Walters 2011-10-14 18:49:07 UTC
See bug 560248 for what disguised items are.  (Note I think there's a bug where we mark other things as disguised =( )
Comment 3 Dmitry Shachnev 2011-10-23 12:15:17 UTC
The same applies to ia32:

  • #0 __memcpy_ia32
    at ../sysdeps/i386/i686/multiarch/../memcpy.S line 100
  • #1 ??
    from /usr/lib/python2.7/dist-packages/gi/_gi.so
  • #2 ??
    from /usr/lib/python2.7/dist-packages/gi/_gi.so
  • #3 ext_do_call
    at ../Python/ceval.c line 4331
  • #4 PyEval_EvalFrameEx
  • #5 PyEval_EvalCodeEx
  • #6 fast_function
    at ../Python/ceval.c line 4117
  • #7 call_function
    at ../Python/ceval.c line 4042
  • #8 PyEval_EvalFrameEx
  • #9 PyEval_EvalCodeEx
  • #10 PyEval_EvalCode
  • #11 run_mod
  • #12 PyRun_InteractiveOneFlags
    at ../Python/pythonrun.c line 845
  • #13 PyRun_InteractiveLoopFlags
    at ../Python/pythonrun.c line 765
  • #14 PyRun_AnyFileExFlags
    at ../Python/pythonrun.c line 734
  • #15 Py_Main
    at ../Modules/main.c line 599
  • #16 main
    at ../Modules/python.c line 23

Comment 4 Sebastian Pölsterl 2012-04-21 12:53:18 UTC
Created attachment 212500 [details]
Test case

Still applies to latest version of pygobject:

  • #0 __memcpy_ssse3
    at ../sysdeps/x86_64/multiarch/memcpy-ssse3.S line 1816
  • #1 _pygi_marshal_from_py_array
    at pygi-marshal-from-py.c line 858
  • #2 _invoke_marshal_in_args
    at pygi-invoke.c line 483
  • #3 _wrap_g_callable_info_invoke
  • #4 ext_do_call
    at /usr/src/debug/Python-2.7.2/Python/ceval.c line 4400
  • #5 PyEval_EvalFrameEx
    at /usr/src/debug/Python-2.7.2/Python/ceval.c line 2779
  • #6 PyEval_EvalCodeEx
  • #7 fast_function
    at /usr/src/debug/Python-2.7.2/Python/ceval.c line 4186
  • #8 call_function
    at /usr/src/debug/Python-2.7.2/Python/ceval.c line 4111
  • #9 PyEval_EvalFrameEx
    at /usr/src/debug/Python-2.7.2/Python/ceval.c line 2740
  • #10 PyEval_EvalCodeEx
    at /usr/src/debug/Python-2.7.2/Python/ceval.c line 3330
  • #11 PyEval_EvalCode
    at /usr/src/debug/Python-2.7.2/Python/ceval.c line 689
  • #12 run_mod
  • #13 PyRun_FileExFlags
  • #14 PyRun_SimpleFileExFlags
    at /usr/src/debug/Python-2.7.2/Python/pythonrun.c line 944
  • #15 Py_Main
    at /usr/src/debug/Python-2.7.2/Modules/main.c line 599
  • #16 __libc_start_main
    at libc-start.c line 226
  • #17 _start

Comment 5 Martin Pitt 2012-04-23 08:53:18 UTC
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..
Comment 6 Martin Pitt 2012-04-23 09:33:01 UTC
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.
Comment 7 Martin Pitt 2012-04-23 10:35:30 UTC
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.
Comment 8 Martin Pitt 2012-04-23 12:10:11 UTC
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.
Comment 9 Martin Pitt 2012-04-23 13:01:08 UTC
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.
Comment 10 Sebastian Pölsterl 2012-04-23 18:49:27 UTC
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.
Comment 11 Martin Pitt 2012-04-24 04:47:09 UTC
(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.