Closed Bug 1461448 Opened 2 years ago Closed 2 years ago

Assertion failure: phase != Phase::NONE (Requested child phase not found under current phase), at js/src/gc/Statistics.cpp:176 with Debugger and wasm


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- fixed
firefox60 --- wontfix
firefox61 --- fixed
firefox62 --- fixed


(Reporter: decoder, Assigned: jonco)


(4 keywords, Whiteboard: [jsbugmon:update,ignore])


(1 file)

The following testcase crashes on mozilla-central revision a7461494a7a0 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --cpu-count=2 --disable-oom-functions):

let lfPreamble = `
  var lfOffThreadGlobal = newGlobal();
  for (lfLocal in this)
    try {} catch(lfVare5) {}
  var g = newGlobal();
  var dbg = new Debugger;
  var gw = dbg.addDebuggee(g);
  for (lfLocal in this)
    if (!(lfLocal in lfOffThreadGlobal))
      try {
        lfOffThreadGlobal[lfLocal] = this[lfLocal];
      } catch(lfVare5) {}
  var g = newGlobal();
  var gw = dbg.addDebuggee(g);
  gcparam("markStackLimit", 1);
  grayRoot()[0] = "foo";
  var lfOffThreadGlobal = newGlobal();
  try { evaluate(\`
    gczeal(18, 1);
    grayRoot()[0] = "foo";
    let inst = new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(
       (memory (export "memory") 1 1)
\`); } catch(exc) {}


Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x0000000000f7f560 in js::gcstats::Statistics::lookupChildPhase (this=0x7ffb3f619780, phaseKind=<optimized out>) at js/src/gc/Statistics.cpp:175
#1  0x0000000000f82b9e in js::gcstats::Statistics::beginPhase (this=this@entry=0x7ffb3f619780, phaseKind=phaseKind@entry=js::gcstats::PhaseKind::UNMARK_GRAY) at js/src/gc/Statistics.cpp:1201
#2  0x0000000000f4d5c6 in js::gcstats::AutoPhase::AutoPhase (phaseKind=js::gcstats::PhaseKind::UNMARK_GRAY, stats=..., this=<synthetic pointer>) at js/src/gc/Statistics.h:418
#3  UnmarkGrayGCThing (rt=0x7ffb3f619000, thing=...) at js/src/gc/Marking.cpp:3642
#4  0x0000000000f4d7ba in ShouldTraceCrossCompartment (trc=0x7ffb3f61a690, src=0x7ffb3ef8b190, cell=0x7ffb3efd0060) at js/src/gc/Marking.cpp:324
#5  0x0000000000f762dc in ShouldTraceCrossCompartment (cell=<optimized out>, src=<optimized out>, trc=0x7ffb3f61a690) at js/src/gc/Marking.cpp:595
#6  js::TraceManuallyBarrieredCrossCompartmentEdge<JSObject*> (trc=0x7ffb3f61a690, src=<optimized out>, dst=0x7ffe14dbaea0, name=0x12e17a2 "Debugger.Object referent") at js/src/gc/Marking.cpp:594
#7  0x0000000000acd30d in DebuggerObject_trace (trc=0x7ffb3f61a690, obj=0x7ffb3ef8b190) at js/src/vm/Debugger.cpp:8664
#8  0x0000000000b99f0c in js::Class::doTrace (this=<optimized out>, obj=0x7ffb3ef8b190, trc=0x7ffb3f61a690) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/instrumentation/none/type/debug/dist/include/js/Class.h:869
#9  JSObject::traceChildren (this=0x7ffb3ef8b190, trc=0x7ffb3f61a690) at js/src/vm/JSObject.cpp:4005
#10 0x0000000000f8036a in js::TraceChildren (trc=<optimized out>, trc@entry=0x7ffb3f61a690, thing=<optimized out>, thing@entry=0x7ffb3ef8b190, kind=kind@entry=JS::TraceKind::Object) at js/src/gc/Tracer.cpp:137
#11 0x0000000000f4d995 in js::GCMarker::markDelayedChildren (this=this@entry=0x7ffb3f61a690, arena=arena@entry=0x7ffb3ef8b000) at js/src/gc/Marking.cpp:2598
#12 0x0000000000f4dbc4 in js::GCMarker::markDelayedChildren (this=this@entry=0x7ffb3f61a690, budget=...) at js/src/gc/Marking.cpp:2623
#13 0x0000000000f5bfef in js::GCMarker::drainMarkStack (this=this@entry=0x7ffb3f61a690, budget=...) at js/src/gc/Marking.cpp:1666
#14 0x0000000000edf61b in js::gc::GCRuntime::drainMarkStack (this=this@entry=0x7ffb3f619700, sliceBudget=..., phase=phase@entry=js::gcstats::PhaseKind::MARK) at js/src/gc/GC.cpp:5901
#15 0x0000000000f0fb86 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ffb3f619700, budget=..., reason=reason@entry=JS::gcreason::TOO_MUCH_MALLOC, session=...) at js/src/gc/GC.cpp:7114
#16 0x0000000000f10e60 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffb3f619700, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::TOO_MUCH_MALLOC) at js/src/gc/GC.cpp:7476
#17 0x0000000000f114f5 in js::gc::GCRuntime::collect (this=this@entry=0x7ffb3f619700, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::TOO_MUCH_MALLOC) at js/src/gc/GC.cpp:7619
#18 0x0000000000f119a2 in js::gc::GCRuntime::startGC (this=0x7ffb3f619700, gckind=GC_NORMAL, reason=JS::gcreason::TOO_MUCH_MALLOC, millis=0) at js/src/gc/GC.cpp:7701
#19 0x0000000000f11bda in js::gc::GCRuntime::gcIfRequested (this=this@entry=0x7ffb3f619700) at js/src/gc/GC.cpp:7881
#20 0x0000000000f13308 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0x7ffb3f619700, cx=cx@entry=0x7ffb3f617000) at js/src/gc/Allocator.cpp:318
#21 0x0000000000f41168 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=<optimized out>, cx=0x7ffb3f617000, kind=<optimized out>) at js/src/gc/Allocator.cpp:273
#22 0x0000000000f41fe5 in js::Allocate<js::BaseShape, (js::AllowGC)1> (cx=cx@entry=0x7ffb3f617000) at js/src/gc/Allocator.cpp:222
#23 0x0000000000c6b589 in js::BaseShape::getUnowned (cx=cx@entry=0x7ffb3f617000, base=...) at js/src/vm/Shape.cpp:1436
#24 0x0000000000c6bc72 in js::EmptyShape::getInitialShape (cx=0x7ffb3f617000, clasp=clasp@entry=0x22cb560 <js::WasmMemoryObject::class_>, proto=..., nfixed=<optimized out>, objectFlags=<optimized out>) at js/src/vm/Shape.cpp:2112
#25 0x0000000000ba907e in NewObject (cx=<optimized out>, group=..., kind=<optimized out>, newKind=js::GenericObject, initialShapeFlags=0) at js/src/vm/JSObject.cpp:736
#26 0x0000000000ba9542 in js::NewObjectWithGivenTaggedProto (cx=<optimized out>, cx@entry=0x7ffb3f617000, clasp=clasp@entry=0x22cb560 <js::WasmMemoryObject::class_>, proto=..., allocKind=js::gc::AllocKind::OBJECT2, newKind=newKind@entry=js::GenericObject, initialShapeFlags=initialShapeFlags@entry=0) at js/src/vm/JSObject.cpp:808
#27 0x0000000000e5127b in js::NewObjectWithGivenTaggedProto (initialShapeFlags=0, newKind=js::GenericObject, proto=..., clasp=0x22cb560 <js::WasmMemoryObject::class_>, cx=0x7ffb3f617000) at js/src/vm/JSObject-inl.h:609
#28 js::NewObjectWithGivenTaggedProto<js::WasmMemoryObject> (initialShapeFlags=0, newKind=js::GenericObject, proto=..., cx=0x7ffb3f617000) at js/src/vm/JSObject-inl.h:619
#29 js::NewObjectWithGivenProto<js::WasmMemoryObject> (newKind=js::GenericObject, proto=..., cx=0x7ffb3f617000) at js/src/vm/JSObject-inl.h:662
#30 js::WasmMemoryObject::create (cx=0x7ffb3f617000, buffer=buffer@entry=..., proto=proto@entry=...) at js/src/wasm/WasmJS.cpp:1483
#31 0x0000000000e51bfe in js::wasm::Module::instantiateMemory (this=this@entry=0x7ffb3eb47ff0, cx=<optimized out>, cx@entry=0x7ffb3f617000, memory=memory@entry=...) at js/src/wasm/WasmModule.cpp:947
#32 0x0000000000e53a0b in js::wasm::Module::instantiate (this=this@entry=0x7ffb3eb47ff0, cx=0x7ffb3f617000, funcImports=funcImports@entry=..., tableImport=tableImport@entry=..., memoryImport=..., memoryImport@entry=..., globalImportValues=..., globalObjs=..., instanceProto=..., instance=...) at js/src/wasm/WasmModule.cpp:1227
#33 0x0000000000e55af0 in Instantiate (cx=<optimized out>, cx@entry=0x7ffb3f617000, module=..., importObj=..., importObj@entry=..., instanceObj=..., instanceObj@entry=...) at js/src/wasm/WasmJS.cpp:1162
#34 0x0000000000e55dc0 in js::WasmInstanceObject::construct (cx=0x7ffb3f617000, argc=<optimized out>, vp=<optimized out>) at js/src/wasm/WasmJS.cpp:1187
#35 0x00000000005b6efe in js::CallJSNative (cx=0x7ffb3f617000, native=native@entry=0xe55c00 <js::WasmInstanceObject::construct(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:280
#65 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9294
rax	0x0	0
rbx	0x68	104
rcx	0x7ffb402ff2ad	140717090402989
rdx	0x0	0
rsi	0x7ffb405ce770	140717093349232
rdi	0x7ffb405cd540	140717093344576
rbp	0x7ffe14dbad70	140729248361840
rsp	0x7ffe14dbad30	140729248361776
r8	0x7ffb405ce770	140717093349232
r9	0x7ffb416bf780	140717111113600
r10	0x58	88
r11	0x7ffb402757a0	140717089839008
r12	0x69	105
r13	0x7ffb3f619780	140717076879232
r14	0x67	103
r15	0x22f0080	36634752
rip	0xf7f560 <js::gcstats::Statistics::lookupChildPhase(js::gcstats::PhaseKind) const+304>
=> 0xf7f560 <js::gcstats::Statistics::lookupChildPhase(js::gcstats::PhaseKind) const+304>:	movl   $0x0,0x0
   0xf7f56b <js::gcstats::Statistics::lookupChildPhase(js::gcstats::PhaseKind) const+315>:	ud2
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision c96b4323d7b8).
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
user:        Jon Coppeard
date:        Fri Nov 03 10:25:25 2017 +0000
summary:     Bug 1413914 - Add zeal mode to check gray marking invariants after every GC r=sfink

This iteration took 269.826 seconds to run.
Flags: needinfo?(sdetar)
jonco, could you look at this bug?  Or help me find the right person that should.
Flags: needinfo?(sdetar) → needinfo?(jcoppeard)
Flags: needinfo?(jcoppeard)
We just need to allow the gray unmarking stats phase to occur under the delayed marking phase.
Assignee: nobody → jcoppeard
Attachment #8976484 - Flags: review?(sphink)
Comment on attachment 8976484 [details] [diff] [review]

Review of attachment 8976484 [details] [diff] [review]:

Huh. I guess we don't hit this much? Maybe we need more zeal.
Attachment #8976484 - Flags: review?(sphink) → review+
Pushed by
Add gray marking phase to delayed marking phase r=sfink
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
(function() {
    q = function() {};
    wm = new WeakMap;
    global = newGlobal();
    var wrapper2 = global.eval("Object.create(null)");
    wm.set(wrapper2, global.l);
    grayRoot().r = wrapper2;
gcparam('markStackLimit', 9);
function f() {
for (let j = 0; j < 9999; j++) {
    try {
    } catch (e) {}

I have another testcase here (run with "--fuzzing-safe --no-threads --no-baseline --no-ion") that is fixed by the patch.

I don't think Debugger stuff is involved - :jonco do you mind seeing if this warrants a relook at landing on mozilla-beta?
Flags: needinfo?(jcoppeard)
As I read it, this is marked wontfix, not unaffected. I think it was just such a rare failure that we weren't bothering to backport. But if it's going to interfere with fuzzing, we can.
Flags: needinfo?(jcoppeard)
Comment on attachment 8976484 [details] [diff] [review]

Ok, I've changed my mind. It may be rare, but it's a MOZ_RELEASE_ASSERT, so it would crash the browser. Seems like we ought to backport it.

Approval Request Comment
[Feature/Bug causing the regression]: delayed marking + GC statistics tracking, both of which have been there for a long time.
[User impact if declined]: very rare crashes
[Is this code covered by automated tests?]: only by the regression test added in this bug.
[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?]: it's just weakening an incorrect assertion.
[String changes made/needed]: none
Attachment #8976484 - Flags: approval-mozilla-beta?
Thanks! (Just wondering, what about ESR 60?)
Comment on attachment 8976484 [details] [diff] [review]

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is release assert that can cause  rare browser crashes.
User impact if declined: Possible crash.
Fix Landed on Version: 62
Risk to taking this patch (and alternatives if risky): Low
String or UUID changes made by this patch: None
Attachment #8976484 - Flags: approval-mozilla-esr60?
Comment on attachment 8976484 [details] [diff] [review]

Makes life easier for the fuzzers. Approved for 61.0b12 and ESR 60.1.
Attachment #8976484 - Flags: approval-mozilla-esr60?
Attachment #8976484 - Flags: approval-mozilla-esr60+
Attachment #8976484 - Flags: approval-mozilla-beta?
Attachment #8976484 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.