1311060 - Assertion failure: CurrentThreadCanAccessZone(zone), at js/src/gc/Heap.h:1264 with schedulegc and worker

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision dc89484d4b45 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

evalInWorker(`
    (new WeakMap).set(class of extends schedulegc("s1") {}.prototype, this);
`);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff01ff700 (LWP 2541)]
0x00000000004f4278 in js::gc::TenuredCell::zone (this=<optimized out>) at js/src/gc/Heap.h:1264
#0  0x00000000004f4278 in js::gc::TenuredCell::zone (this=<optimized out>) at js/src/gc/Heap.h:1264
#1  0x0000000000ca8957 in ScheduleGC (cx=cx@entry=0x7ffff032c000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:776
#2  0x0000000000b1dc31 in js::CallJSNative (cx=cx@entry=0x7ffff032c000, native=0xca87f0 <ScheduleGC(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:239
#3  0x0000000000b18eb3 in js::InternalCallOrConstruct (cx=0x7ffff032c000, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:458
#4  0x0000000000b0c82e in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:509
#5  Interpret (cx=0x7ffff032c000, state=...) at js/src/vm/Interpreter.cpp:2922
#6  0x0000000000b18c7d in js::RunScript (cx=cx@entry=0x7ffff032c000, state=...) at js/src/vm/Interpreter.cpp:404
#7  0x0000000000b1b3a8 in js::ExecuteKernel (cx=cx@entry=0x7ffff032c000, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7ffff01fed70) at js/src/vm/Interpreter.cpp:685
#8  0x0000000000b1b768 in js::Execute (cx=cx@entry=0x7ffff032c000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7ffff01fed70) at js/src/vm/Interpreter.cpp:718
#9  0x00000000008e36ca in ExecuteScript (cx=cx@entry=0x7ffff032c000, scope=scope@entry=..., script=script@entry=..., rval=rval@entry=0x7ffff01fed70) at js/src/jsapi.cpp:4307
#10 0x00000000008f108b in JS_ExecuteScript (cx=cx@entry=0x7ffff032c000, scriptArg=scriptArg@entry=..., rval=rval@entry=...) at js/src/jsapi.cpp:4333
#11 0x00000000004612fd in WorkerMain (arg=0x7ffff022fdc0) at js/src/shell/js.cpp:3332
#12 0x00000000004632b2 in js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::callMain<0ul> (this=0x7ffff69201b0) at js/src/threading/Thread.h:234
#13 js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::Start (aPack=0x7ffff69201b0) at js/src/threading/Thread.h:227
#14 0x00007ffff7bc16fa in start_thread (arg=0x7ffff01ff700) at pthread_create.c:333
#15 0x00007ffff6c38b5d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
rax	0x0	0
rbx	0x7ffff691f000	140737330147328
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7ffff01fe270	140737222009456
rsp	0x7ffff01fe260	140737222009440
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff01ff700	140737222014720
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff032c000	140737223245824
r13	0x0	0
r14	0x7ffff01fe300	140737222009600
r15	0x7ffff69b70c0	140737330770112
rip	0x4f4278 <js::gc::TenuredCell::zone() const+344>
=> 0x4f4278 <js::gc::TenuredCell::zone() const+344>:	movl   $0x0,0x0
   0x4f4283 <js::gc::TenuredCell::zone() const+355>:	ud2    



GC related, not sure if this is a shell-only problem. Hiding until investigated further.

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20151009082747" and the hash "9d24051f444950cf6ea524989df43307647ac264".
The "bad" changeset has the timestamp "20151009093836" and the hash "a67bbdf269fd2661280ab7421e9b601cab409422".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9d24051f444950cf6ea524989df43307647ac264&tochange=a67bbdf269fd2661280ab7421e9b601cab409422

Comment 2

[Daniel Veditz [:dveditz]]

Jason: you reviewed the likely regressor. We're guessing this isn't good but could you take a look and let us know.

Comment 3

[Gerry Chang [:gchang]]

Track 51+/52+ as sec-high.

Comment 4

[Jon Coppeard (:jonco)]

This is not the same as bug 1328251.  It's a shell problem because we assume we can access the zone of a string in the ScheduleGC function.  We can't always do this - in this ScheduleGC is called on a worker and the string is in the parent runtime's atom's zone.

Comment 5

[Andrew McCreight [:mccr8]]

If this is just a problem with the shell, I'm going to change this to sec-other.

Comment 6

[Jon Coppeard (:jonco)]

Attachment: bug1311060-schedule-gc

Patch to check we can access the string's zone before using it and add some error checking and reporting around schedulegc.

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8825051 [details] [diff] [review]
bug1311060-schedule-gc

Review of attachment 8825051 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, thanks.

::: js/src/builtin/TestingFunctions.cpp
@@ +786,5 @@
>          PrepareZoneForGC(zone);
>      } else if (args[0].isString()) {
>          /* This allows us to schedule the atoms zone for GC. */
> +        Zone* zone = args[0].toString()->zoneFromAnyThread();
> +        if (!CurrentThreadCanAccessZone(zone)) {

Could this also happen for an object argument (branch above)?

::: js/src/jit-test/tests/gc/bug-1311060.js
@@ +1,1 @@
> +evalInWorker(`schedulegc("s1");`);

Does this need a typeof evalInWorked !== 'undefined'?

Comment 8

[Jon Coppeard (:jonco)]

(In reply to Benjamin Bouvier [:bbouvier] from comment #7)
> Could this also happen for an object argument (branch above)?

I don't think this is possible because we don't expose objects in the self-hosting compartment to scripts.  At least, I pretty sure we don't.

> ::: js/src/jit-test/tests/gc/bug-1311060.js
> @@ +1,1 @@
> > +evalInWorker(`schedulegc("s1");`);
> 
> Does this need a typeof evalInWorked !== 'undefined'?

It seems this is always defined for jit-tests.

Thanks for the quick review!

Comment 9

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/006e9ea4166ed1849f15e042be9683d1599423d3

Comment 10

[Jon Coppeard (:jonco)]

And fix test bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/06a71c58934a2f5b2dc45f4d2eca33fce78a82fd

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/006e9ea4166e
https://hg.mozilla.org/mozilla-central/rev/06a71c58934a

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/136db7f56314
https://hg.mozilla.org/releases/mozilla-beta/rev/b4281211462e

Comment 13

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx51
JSBugMon: This bug has been automatically verified fixed on Fx52