Bug 696933 – Crashes running test program

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 696933 - Crashes running test program


Summary:	Crashes running test program


Status:	RESOLVED FIXED

Product:	gjs
Classification:	Bindings
Component:	general
Version:	1.36.x
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	---
Assigned To:	gjs-maint
QA Contact:	gjs-maint

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2013-03-30 22:48 UTC by Bastien Nocera
Modified:	2013-04-02 16:33 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
test.js (4.87 KB, application/x-javascript) 2013-03-30 22:48 UTC, Bastien Nocera		Details
test.js (186 bytes, text/plain) 2013-03-30 22:53 UTC, Bastien Nocera		Details
Fix crash when marshalling a GType array containing non objects (2.41 KB, patch) 2013-04-02 16:05 UTC, Giovanni Campagna	reviewed	Details \| Review
Fix crash when marshalling a GType array containing non objects (2.75 KB, patch) 2013-04-02 16:25 UTC, Giovanni Campagna	committed	Details \| Review

Description Bastien Nocera 2013-03-30 22:48:24 UTC

Created attachment 240180 [details]
test.js

js-1.8.5-13.fc19.x86_64
gjs-1.36.0-1.fc19.x86_64

Running the attached test program, gjs crashes with the following bt.

+ Trace 231700

#0 JS_GetPropertyById
at jsapi.cpp line 3806
#1 JS_GetProperty
at jsapi.cpp line 3825
#2 gjs_gtype_get_actual_gtype
at gi/gtype.c line 160
#3 gjs_gtypearray_to_array
at gi/arg.c line 630
#4 gjs_array_to_array
at gi/arg.c line 918
#5 gjs_array_to_explicit_array_internal
at gi/arg.c line 1090
#6 gjs_value_to_explicit_array
at gi/arg.c line 1894
#7 gjs_invoke_c_function
at gi/function.c line 803
#8 function_call
at gi/function.c line 1202
#9 CallJSNative
at jscntxtinlines.h line 701
#10 js::Invoke
at jsinterp.cpp line 696
#11 js::Interpret
at jsinterp.cpp line 4810
#12 js::RunScript
at jsinterp.cpp line 653
#13 js::Invoke
at jsinterp.cpp line 740
#14 js_fun_apply
at jsfun.cpp line 2205
#15 CallJSNative
at jscntxtinlines.h line 701
#16 js::Interpret
at jsinterp.cpp line 4799
#17 js::RunScript
at jsinterp.cpp line 653
#18 js::Invoke
at jsinterp.cpp line 740
#19 js_fun_apply
at jsfun.cpp line 2205
#20 CallJSNative
at jscntxtinlines.h line 701
#21 js::Interpret
at jsinterp.cpp line 4799
#22 js::RunScript
at jsinterp.cpp line 653
#23 js::Invoke
at jsinterp.cpp line 740
#24 js::CallOrConstructBoundFunction
at jsfun.cpp line 2319
#25 CallJSNative
at jscntxtinlines.h line 701
#26 js::Invoke
at jsinterp.cpp line 703
#27 js::ExternalInvoke
at jsinterp.cpp line 863
#28 JS_CallFunctionValue
at jsapi.cpp line 5145
#29 gjs_call_function_value
at gjs/jsapi-util.c line 708
#30 gjs_closure_invoke
at gi/closure.c line 276
#31 closure_marshal
at gi/value.c line 127
#32 g_closure_invoke
at gclosure.c line 777
#33 signal_emit_unlocked_R
at gsignal.c line 3566
#34 g_signal_emit_valist
at gsignal.c line 3314
#35 g_signal_emit
at gsignal.c line 3370
#36 g_application_register
at gapplication.c line 1318
#37 g_application_real_local_command_line
at gapplication.c line 474
#38 g_application_real_local_command_line
at gapplication.c line 462
#39 g_application_run
at gapplication.c line 1570
#40 ffi_call_unix64
from /lib64/libffi.so.6
#41 ffi_call
from /lib64/libffi.so.6
#42 gjs_invoke_c_function
at gi/function.c line 893
#43 function_call
at gi/function.c line 1202
#44 CallJSNative
at jscntxtinlines.h line 701
#45 js::Invoke
at jsinterp.cpp line 696
#46 js::Interpret
at jsinterp.cpp line 4810
#47 js::RunScript
at jsinterp.cpp line 653
#48 js::Execute
at jsinterp.cpp line 1028
#49 EvaluateUCScriptForPrincipalsCommon
at jsapi.cpp line 5031
#50 JS_EvaluateUCScriptForPrincipals
at jsapi.cpp line 5058
#51 JS_EvaluateScriptForPrincipals
at jsapi.cpp line 5081
#52 JS_EvaluateScript
at jsapi.cpp line 5101
#53 gjs_context_eval
at gjs/context.c line 987
#54 main
at gjs/console.c line 112

Comment 1 Bastien Nocera 2013-03-30 22:53:19 UTC

Created attachment 240181 [details]
test.js

Smaller reproducer

Comment 2 Giovanni Campagna 2013-04-02 16:05:05 UTC

The issue is that Gdkpixbuf.GdkPixbuf is undefined (use Gdkpixbuf.Pixbuf instead), but we definitely shouldn't blow up here.

Comment 3 Giovanni Campagna 2013-04-02 16:05:29 UTC

Created attachment 240401 [details] [review]
Fix crash when marshalling a GType array containing non objects

It would cast a jsval of the wrong type to object and pass NULL
to gjs_gtype_get_actual_gtype()

Tests included.

Comment 4 Jasper St. Pierre (not reading bugmail) 2013-04-02 16:10:13 UTC

Review of attachment 240401 [details] [review]:

::: gi/arg.c
@@ +629,3 @@
 
+        if (!JSVAL_IS_OBJECT(elem) ||
+            (gtype = gjs_gtype_get_actual_gtype(context, JSVAL_TO_OBJECT(elem)))

I don't quite like the style here. I'd prefer:

if (!JSVAL_IS_OBJECT(elem))
    goto err;

gtype = gjs_gtype_get_actual_gtype(context, JSVAL_TO_OBJECT(elem));

if (gtype == G_TYPE_INVALID)
    goto err;

Comment 5 Giovanni Campagna 2013-04-02 16:25:44 UTC

Created attachment 240406 [details] [review]
Fix crash when marshalling a GType array containing non objects

It would cast a jsval of the wrong type to object and pass NULL
to gjs_gtype_get_actual_gtype()

Tests included.

Comment 6 Jasper St. Pierre (not reading bugmail) 2013-04-02 16:30:31 UTC

Review of attachment 240406 [details] [review]:

OK.

Comment 7 Giovanni Campagna 2013-04-02 16:33:23 UTC

Attachment 240406 [details] pushed as f4a5b2c - Fix crash when marshalling a GType array containing non objects