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 704842 - Segfaults when handling out arrays of structs
Segfaults when handling out arrays of structs
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
1.36.x
Other Linux
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on: 705602
Blocks:
 
 
Reported: 2013-07-24 23:47 UTC by Cosimo Cecchi
Modified: 2016-02-07 04:16 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
testcase (167 bytes, text/javascript)
2013-07-24 23:48 UTC, Cosimo Cecchi 		  Details
arg: add support for arrays of flat structures (to JS) (2.74 KB, patch)
2013-07-31 09:27 UTC, Giovanni Campagna 		accepted-commit_now Details | Review
arg: add support for arrays of flat structures (to JS) (2.75 KB, patch)
2014-09-29 17:36 UTC, Bastien Nocera 		committed Details | Review

Description Cosimo Cecchi 2013-07-24 23:47:07 UTC 
gjs segfaults when marshalling an out array of structs. This is easily reproducible with e.g. gdk_keymap_get_entries_for_keyval(), where the out argument is a GdkKeymapKey **, i.e. a location to store an array of GdkKeymapKey structures.

See attached testcase.


Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7175d80 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt

+ Trace 232292

  • #0 ??
    from /lib/x86_64-linux-gnu/libc.so.6
  • #1 memcpy
    at /usr/include/x86_64-linux-gnu/bits/string3.h line 51
  • #2 gjs_boxed_from_c_struct
    at gi/boxed.c line 1312
  • #3 gjs_value_from_g_argument
    at gi/arg.c line 2561
  • #4 gjs_array_from_carray_internal
    at gi/arg.c line 2067
  • #5 gjs_value_from_explicit_array
    at gi/arg.c line 2119
  • #6 gjs_invoke_c_function
    at gi/function.c line 1074
  • #7 function_call
    at gi/function.c line 1202
  • #8 ??
    from /usr/lib/libmozjs185.so.1.0
  • #9 ??
    from /usr/lib/libmozjs185.so.1.0
  • #10 ??
    from /usr/lib/libmozjs185.so.1.0
  • #11 ??
    from /usr/lib/libmozjs185.so.1.0
  • #12 JS_EvaluateUCScriptForPrincipals
    from /usr/lib/libmozjs185.so.1.0
  • #13 JS_EvaluateScriptForPrincipals
    from /usr/lib/libmozjs185.so.1.0
  • #14 JS_EvaluateScript
    from /usr/lib/libmozjs185.so.1.0
  • #15 gjs_context_eval
    at gjs/context.c line 1013
  • #16 main
    at gjs/console.c line 112 






Comment 1


Cosimo Cecchi





          2013-07-24 23:48:19 UTC
        



Created attachment 250082 [details]
testcase






Comment 2


Giovanni Campagna





          2013-07-31 08:59:59 UTC
        



Yeah, the current code handling arrays of structures assumes that they are arrays of pointers to structures (except for "flat GValue arrays"), so it would be GdkKeymapKey*** in this case.






Comment 3


Giovanni Campagna





          2013-07-31 09:27:11 UTC
        



Created attachment 250528 [details] [review]
arg: add support for arrays of flat structures (to JS)

Add support for marshalling from C to JS arrays not of pointer
to structures but of actual structures, as for example used
by gdk_keymap_get_entries_for_keyval().






Comment 4


Cosimo Cecchi





          2013-08-01 10:02:22 UTC
        



Review of attachment 250528 [details] [review]:

Not a gjs reviewer, but the patch makes sense to me and makes the testcase work correctly.






Comment 5


Cosimo Cecchi





          2014-01-08 23:24:18 UTC
        



Ping, can we get this in?






Comment 6


Jasper St. Pierre (not reading bugmail)





          2014-01-08 23:37:55 UTC
        



Review of attachment 250528 [details] [review]:

Looks good.






Comment 7


Colin Walters





          2014-01-08 23:43:12 UTC
        



Review of attachment 250528 [details] [review]:

Would like a test case in g-i's regress.h and then consume it here.

::: gi/arg.c
@@ +2140,3 @@
+          if (info_type == GI_INFO_TYPE_ENUM ||
+              info_type == GI_INFO_TYPE_FLAGS) {
+              ITERATE(int);

Unfortunately enums/flags are not just integers always.  See https://bugzilla.gnome.org/show_bug.cgi?id=629705

If this code path isn't necessary I'd prefer to just drop it until we have a correct fix.  (Or alternatively, throw an exception).






Comment 8


Cosimo Cecchi





          2014-03-26 22:49:51 UTC
        



Giovanni, any chance we could get this in, or do we need to address Colin's comment?






Comment 9


Bastien Nocera





          2014-09-29 17:36:00 UTC
        



Created attachment 287381 [details] [review]
arg: add support for arrays of flat structures (to JS)

Add support for marshalling from C to JS arrays not of pointer
to structures but of actual structures, as for example used
by gdk_keymap_get_entries_for_keyval().






Comment 10


Bastien Nocera





          2014-09-29 17:37:11 UTC
        



It's missing a test case indeed.






Comment 11


Cosimo Cecchi





          2015-10-28 01:40:29 UTC
        



Pushed to master with a testcase.

Attachment 287381 [details] pushed as 967d696 - arg: add support for arrays of flat structures (to JS)






Comment 12


Philip Chimento





          2016-02-07 04:16:12 UTC
        



Heads up that I filed https://bugzilla.gnome.org/show_bug.cgi?id=761658 (with patch) to fix a problem with this patch.