Bug 770244 – SEGFAULT in js::NewObjectWithClassProtoCommon when instantiating a dynamic type defined in JS

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 770244 - SEGFAULT in js::NewObjectWithClassProtoCommon when instantiating a dynamic type defined in JS


Summary:	SEGFAULT in js::NewObjectWithClassProtoCommon when instantiating a dynamic ty...


Status:	RESOLVED FIXED

Product:	gjs
Classification:	Bindings
Component:	general
Version:	unspecified
Hardware:	Other Linux

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

URL:
Whiteboard:

Depends on:	751252
Blocks:

Reported:	2016-08-22 16:34 UTC by Juan Pablo Ugarte
Modified:	2016-11-15 23:23 UTC

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

Attachments
test case (2.21 KB, text/x-csrc) 2016-08-22 16:34 UTC, Juan Pablo Ugarte		Details
Add JS defined class object instantiation test (2.09 KB, patch) 2016-08-29 18:09 UTC, Juan Pablo Ugarte	accepted-commit_now	Details \| Review
object: Enter global compartment in custom constructor (6.95 KB, patch) 2016-11-15 19:33 UTC, Philip Chimento	committed	Details \| Review

Description Juan Pablo Ugarte 2016-08-22 16:34:00 UTC

Created attachment 333927 [details]
test case

gjs segfaults if you try to call g_object_new() on a dynamic type registered in a GjsContext


Backtrace:

Thread 1 "gjs-test" received signal SIGSEGV, Segmentation fault.
0x00007ffff064b1a6 in js::NewObjectWithClassProtoCommon (cx=0x677800, clasp=0x7ffff0b9fdc0 <js::ObjectClass>, protoArg=0x0, 
    parentArg=0x0, allocKind=js::gc::FINALIZE_OBJECT0_BACKGROUND, newKind=js::GenericObject)
    at /build/mozjs24-UQFbnn/mozjs24-24.2.0/js/src/jsobj.cpp:1349
1349	/build/mozjs24-UQFbnn/mozjs24-24.2.0/js/src/jsobj.cpp: No such file or directory.
(gdb) bt

+ Trace 236571

#0 js::NewObjectWithClassProtoCommon
at /build/mozjs24-UQFbnn/mozjs24-24.2.0/js/src/jsobj.cpp line 1349
#1 js::NewObjectWithClassProtoCommon
at /build/mozjs24-UQFbnn/mozjs24-24.2.0/js/src/jsobj.cpp line 1395
#2 js::NewObjectWithClassProto
at ./jsobjinlines.h line 1493
#3 js::NewObjectWithClassProto
at ./jsobjinlines.h line 1501
#4 JS_NewObject
at /build/mozjs24-UQFbnn/mozjs24-24.2.0/js/src/jsapi.cpp line 3251
#5 gjs_object_constructor
at gi/object.cpp line 2479
#6 g_object_new_with_custom_constructor
at /home/xjuan/gnome/checkout/glib/gobject/gobject.c line 1692
#7 g_object_new_internal
at /home/xjuan/gnome/checkout/glib/gobject/gobject.c line 1772
#8 g_object_new_valist
at /home/xjuan/gnome/checkout/glib/gobject/gobject.c line 2033
#9 g_object_new
at /home/xjuan/gnome/checkout/glib/gobject/gobject.c line 1617
#10 main
at gjs-test.c line 76

Comment 1 Juan Pablo Ugarte 2016-08-29 18:09:29 UTC

Created attachment 334397 [details] [review]
Add JS defined class object instantiation test

A test case that also exposes the crash

Comment 2 Philip Chimento 2016-11-15 19:33:04 UTC

Created attachment 339959 [details] [review]
object: Enter global compartment in custom constructor

Previously you could construct JS-defined GObjects from inside JS, but
not from C. That's because when the constructor() vfunc is called from C,
the JS engine isn't inside the right compartment.

While entering the compartment using JSAutoCompartment, we also change
the constructor vfunc to use JSAutoRequest, so that it consistently uses
a more RAII style.

Test case from commit by Juan Pablo Ugarte <juanpablougarte@gmail.com>

Comment 3 Philip Chimento 2016-11-15 19:49:32 UTC

Needs mozjs31 to work. Apparently in mozjs24 the JIT doesn't like this.

Comment 4 Cosimo Cecchi 2016-11-15 22:15:58 UTC

Review of attachment 334397 [details] [review]:

OK

Comment 5 Cosimo Cecchi 2016-11-15 22:16:38 UTC

Comment on attachment 334397 [details] [review]
Add JS defined class object instantiation test

Actually this is obsolete

Comment 6 Cosimo Cecchi 2016-11-15 22:18:29 UTC

Review of attachment 339959 [details] [review]:

Looks good to me.

Comment 7 Philip Chimento 2016-11-15 23:22:29 UTC

Attachment 339959 [details] pushed as a1e1a80 - object: Enter global compartment in custom constructor

Comment 8 Philip Chimento 2016-11-15 23:23:09 UTC

I pushed it to the "mozjs31" branch — it'll be merged to master when we're ready to merge the whole branch.