Crash [@ JSObject::compartment] or Crash [@ js::gc::detail::GetCellLocation] with evalInWorker and grayRoot

RESOLVED FIXED in Firefox -esr60

Status

()

defect
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: decoder, Assigned: sfink)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla62
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox-esr60 fixed, firefox60 wontfix, firefox61 fixed, firefox62 fixed)

Details

(Whiteboard: [jsbugmon:update][fuzzblocker], crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

a year ago
The following testcase crashes on mozilla-central revision 8c9263730393 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

evalInWorker(`
  grayRoot().x = Object.create(null);
  gc();
  grayRoot().map = wm;
`);


Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00000000005b2535 in JSObject::compartment (this=0x7ffff46110a0) at js/src/vm/JSObject.h:163
#1  js::CompartmentChecker::check (this=0x7ffff68fe150, obj=<optimized out>) at js/src/vm/JSContext-inl.h:66
#2  0x00000000005b6178 in js::CompartmentChecker::check<JS::Value> (handle=..., this=0x7ffff68fe150) at js/src/vm/JSContext-inl.h:82
#3  js::assertSameCompartmentDebugOnly<JS::MutableHandle<JS::Value> > (cx=cx@entry=0x7ffff4947000, t1=...) at js/src/vm/JSContext-inl.h:209
#4  0x00000000005b7f17 in js::CallJSNative (cx=0x7ffff4947000, native=<optimized out>, args=...) at js/src/vm/JSContext-inl.h:276
#5  0x00000000005acf1f in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff4947000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:471
#6  0x00000000005ad27d in InternalCall (cx=0x7ffff4947000, args=...) at js/src/vm/Interpreter.cpp:520
#7  0x00000000005a07bd in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:526
#8  Interpret (cx=0x7ffff4947000, state=...) at js/src/vm/Interpreter.cpp:3122
#9  0x00000000005ac9fd in js::RunScript (cx=0x7ffff4947000, state=...) at js/src/vm/Interpreter.cpp:421
#10 0x00000000005afb8d in js::ExecuteKernel (cx=<optimized out>, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7ffff68fed70) at js/src/vm/Interpreter.cpp:704
#11 0x00000000005aff31 in js::Execute (cx=<optimized out>, cx@entry=0x7ffff4947000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7ffff68fed70) at js/src/vm/Interpreter.cpp:737
#12 0x0000000000a482a9 in ExecuteScript (cx=0x7ffff4947000, scope=scope@entry=..., script=..., rval=0x7ffff68fed70) at js/src/jsapi.cpp:4752
#13 0x0000000000a4844e in JS_ExecuteScript (cx=<optimized out>, scriptArg=scriptArg@entry=..., rval=rval@entry=...) at js/src/jsapi.cpp:4778
#14 0x0000000000471b2d in WorkerMain (arg=<optimized out>) at js/src/shell/js.cpp:3631
#15 0x0000000000478c52 in js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::callMain<0ul> (this=0x7ffff5f18110) at js/src/threading/Thread.h:242
#16 js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::Start (aPack=0x7ffff5f18110) at js/src/threading/Thread.h:235
#17 0x00007ffff7bc16ba in start_thread (arg=0x7ffff68ff700) at pthread_create.c:333
#18 0x00007ffff6c383dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
rax	0xfffe4b4b4b4b4b4b	-480163195565237
rbx	0x7ffff68fe150	140737330012496
rcx	0x88	136
rdx	0x110a0	69792
rsi	0x0	0
rdi	0x7ffff46fffe8	140737294368744
rbp	0x7ffff68fe140	140737330012480
rsp	0x7ffff68fe120	140737330012448
r8	0x7ffff68fe0b0	140737330012336
r9	0x88	136
r10	0x7ffff46250d0	140737293471952
r11	0x0	0
r12	0x7ffff68fe190	140737330012560
r13	0x1	1
r14	0x7ffff68fe190	140737330012560
r15	0x7ffff5fd3090	140737320398992
rip	0x5b2535 <js::CompartmentChecker::check(JSObject*)+53>
=> 0x5b2535 <js::CompartmentChecker::check(JSObject*)+53>:	mov    0x10(%rax),%rcx
   0x5b2539 <js::CompartmentChecker::check(JSObject*)+57>:	test   %rcx,%rcx


Marking s-s because the crash has a 0x4b4b4b4b pattern.
Reporter

Comment 1

a year ago
This is an automated crash issue comment:

Summary: Crash [@ js::gc::detail::GetCellLocation]
Build version: mozilla-central revision 8c9263730393
Build flags: --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize
Runtime options: --fuzzing-safe --ion-offthread-compile=off

Testcase:

evalInWorker(`
  var wm = new WeakMap();
  grayRoot().map = wm;
  gczeal(4,10);
  evaluate(\`
  grayRoot().map = __v_1173;
  if (!class i   { constructor() { } }  ()) {
    (function __f_252( get       ,    )  {})();
  }
  \`);
`);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  js::gc::detail::GetCellLocation (cell=<optimized out>) at dist/include/js/HeapAPI.h:463
#1  js::gc::IsInsideNursery (cell=<optimized out>) at dist/include/js/HeapAPI.h:482
#2  0x00000000004d9071 in js::gc::Cell::isTenured (this=0xfffe4b4b4b4b4b4b) at js/src/gc/Cell.h:55
#3  js::gc::TenuredCell::arena (this=0xfffe4b4b4b4b4b4b) at js/src/gc/Cell.h:332
#4  0x0000000000f824c5 in js::gc::TenuredCell::zoneFromAnyThread (this=<optimized out>) at js/src/gc/Cell.h:361
#5  JSObject::zoneFromAnyThread (this=0x7ffff46110a0) at js/src/vm/JSObject.h:270
#6  js::CheckTracedThing<JSObject> (trc=trc@entry=0x7ffff496d6d0, thing=thing@entry=0x7ffff46110a0) at js/src/gc/Marking.cpp:221
#7  0x0000000000f8b6a9 in DoMarking<JSObject> (gcmarker=0x7ffff496d6d0, thing=0x7ffff46110a0) at js/src/gc/Marking.cpp:701
#8  0x00000000004ef098 in js::gc::TenuredCell::readBarrier (thing=0x7ffff46110a0) at js/src/gc/Cell.h:389
#9  0x0000000000f44099 in IncrementalReadBarrierFunctor::operator()<JSObject> (this=<synthetic pointer>, t=<optimized out>) at js/src/gc/GC.cpp:8757
#10 JS::DispatchTyped<IncrementalReadBarrierFunctor> (f=..., thing=...) at dist/include/js/HeapAPI.h:356
#11 0x0000000000f0fa59 in JS::IncrementalReadBarrier (thing=...) at js/src/gc/GC.cpp:8767
#12 0x0000000000469338 in js::gc::ExposeGCThingToActiveJS (thing=...) at dist/include/js/HeapAPI.h:618
#13 JS::ExposeObjectToActiveJS (obj=<optimized out>) at dist/include/js/HeapAPI.h:662
#14 js::BarrierMethods<JSObject*>::exposeToJS (obj=<optimized out>) at dist/include/js/RootingAPI.h:684
#15 JS::Heap<JSObject*>::exposeToActiveJS (this=<optimized out>) at dist/include/js/RootingAPI.h:306
#16 JS::Heap<JSObject*>::get (this=<optimized out>) at dist/include/js/RootingAPI.h:309
#17 JS::Heap<JSObject*>::operator JSObject* const& (this=<optimized out>) at dist/include/js/RootingAPI.h:300
#18 EnsureGrayRoot (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:6323
#19 0x00000000005b7ef1 in js::CallJSNative (cx=0x7ffff4947000, native=0x469170 <EnsureGrayRoot(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:274
#20 0x00000000005acf1f in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff4947000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:471
#21 0x00000000005ad27d in InternalCall (cx=0x7ffff4947000, args=...) at js/src/vm/Interpreter.cpp:520
#22 0x00000000005a07bd in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:526
#23 Interpret (cx=0x7ffff4947000, state=...) at js/src/vm/Interpreter.cpp:3122
#24 0x00000000005ac9fd in js::RunScript (cx=0x7ffff4947000, state=...) at js/src/vm/Interpreter.cpp:421
#25 0x00000000005afb8d in js::ExecuteKernel (cx=<optimized out>, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7ffff5fd3090) at js/src/vm/Interpreter.cpp:704
#26 0x00000000005aff31 in js::Execute (cx=<optimized out>, cx@entry=0x7ffff4947000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7ffff5fd3090) at js/src/vm/Interpreter.cpp:737
#27 0x0000000000a482a9 in ExecuteScript (cx=cx@entry=0x7ffff4947000, scope=scope@entry=..., script=script@entry=..., rval=rval@entry=0x7ffff5fd3090) at js/src/jsapi.cpp:4752
#28 0x0000000000a48873 in ExecuteScript (cx=0x7ffff4947000, envChain=..., scriptArg=..., rval=0x7ffff5fd3090) at js/src/jsapi.cpp:4771
#29 0x0000000000a4891a in JS_ExecuteScript (cx=<optimized out>, envChain=..., scriptArg=..., scriptArg@entry=..., rval=...) at js/src/jsapi.cpp:4792
#30 0x00000000004740f3 in Evaluate (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:2024
#31 0x00000000005b7ef1 in js::CallJSNative (cx=0x7ffff4947000, native=0x4735a0 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:274
[...]
#45 0x00007ffff6c383dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
rax	0x7ffff4600000	140737293320192
rbx	0xfffe4b4b4b4b4b4b	-480163195565237
rcx	0x135a054	20291668
rdx	0x1	1
rsi	0x7ffff46110a0	140737293389984
rdi	0xfffe4b4b4b4fffe8	-480163195256856
rbp	0x7ffff68fcfd0	140737330008016
rsp	0x7ffff68fcfb8	140737330007992
r8	0x7ffff68fcfe0	140737330008032
r9	0x10	16
r10	0x23050c0	36720832
r11	0x246	582
r12	0x7ffff496d6d0	140737296914128
r13	0x7ffff496c000	140737296908288
r14	0x7ffff68fd150	140737330008400
r15	0x7ffff5fd3128	140737320399144
rip	0x4799b3 <js::gc::IsInsideNursery(js::gc::Cell const*)+19>
=> 0x4799b3 <js::gc::IsInsideNursery(js::gc::Cell const*)+19>:	mov    (%rdi),%eax
   0x4799b5 <js::gc::IsInsideNursery(js::gc::Cell const*)+21>:	lea    -0x1(%rax),%edx


Very similar test and crash address, but different stack.
Reporter

Updated

a year ago
Crash Signature: [@ JSObject::compartment] → [@ JSObject::compartment][@ js::gc::detail::GetCellLocation]
Summary: Crash [@ JSObject::compartment] with evalInWorker and grayRoot → Crash [@ JSObject::compartment] or Crash [@ js::gc::detail::GetCellLocation] with evalInWorker and grayRoot
Reporter

Comment 2

a year ago
Also seeing various other failures with the combination of evalInWorker and grayRoot, waiting for this to be fixed to recheck the others.

Updated

a year ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 3

a year ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a56ac3aa583c
user:        Till Schneidereit
date:        Wed May 10 16:16:27 2017 +0200
summary:     Bug 1357958 - Move the JS shell's Promise job handling into the engine to be used as a default implementation. r=jandem

This iteration took 257.172 seconds to run.
Component: JavaScript Engine → JavaScript: GC
Reporter

Comment 4

a year ago
Marking as fuzzblocker because this is occurring frequently. Needinfo from Jonco because it involves grayRoot. Also Ccing Till based on comment 3, but I doubt that this is the real cause of the issue.
Flags: needinfo?(jcoppeard)
Whiteboard: [jsbugmon:update] → [jsbugmon:update][fuzzblocker]
Assignee

Comment 5

a year ago
WeakMap and grayRoot(); I'll take this one.
Assignee: nobody → sphink
Flags: needinfo?(jcoppeard)
Assignee

Comment 6

a year ago
We clear out the gray root tracer on the worker thread, but we never actually set it up.

This passed tests locally, but it definitely needs to survive try before I"ll attempt landing. The setup/teardown ordering has been a major source of issues in the past.
Attachment #8983607 - Flags: review?(jcoppeard)
Assignee

Comment 7

a year ago
Oops, forgot to include the test.
Attachment #8983613 - Flags: review?(jcoppeard)
Assignee

Updated

a year ago
Attachment #8983607 - Attachment is obsolete: true
Attachment #8983607 - Flags: review?(jcoppeard)
Assignee

Comment 8

a year ago
Testing only.
Group: javascript-core-security
Attachment #8983613 - Flags: review?(jcoppeard) → review+

Comment 9

a year ago
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdf9bd1db757
Set up the gray root tracer on worker thread, r=jonco

Comment 11

a year ago
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bec2206d993
Set up the gray root tracer on worker thread, r=jonco
Assignee

Updated

a year ago
Flags: needinfo?(sphink)

Comment 12

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2bec2206d993
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Please nominate this for Beta/ESR60 backport to make our fuzzers' lives less difficult :)
Flags: needinfo?(sphink)
Flags: in-testsuite+
Assignee

Comment 14

a year ago
Comment on attachment 8983613 [details] [diff] [review]
Set up the gray root tracer on worker thread

Approval Request Comment
[Feature/Bug causing the regression]: grayRoot() test function exposed.
[User impact if declined]: none; test-only. But the fuzzers won't be able to fuzz this stuff.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: test-only
[String changes made/needed]: none

[Approval Request Comment]
> If this is not a sec:{high,crit} bug, please state case for ESR consideration:

Happy fuzzers are good fuzzers.

> User impact if declined: 

None; test-only.

> Fix Landed on Version:

62

> Risk to taking this patch (and alternatives if risky): 

Test-only.

> String or UUID changes made by this patch: 

None.
Flags: needinfo?(sphink)
Attachment #8983613 - Flags: approval-mozilla-esr60?
Attachment #8983613 - Flags: approval-mozilla-beta?
Reporter

Updated

a year ago
Duplicate of this bug: 1468375
Comment on attachment 8983613 [details] [diff] [review]
Set up the gray root tracer on worker thread

Approved for 61.0b14 and ESR 60.1.
Attachment #8983613 - Flags: approval-mozilla-esr60?
Attachment #8983613 - Flags: approval-mozilla-esr60+
Attachment #8983613 - Flags: approval-mozilla-beta?
Attachment #8983613 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.