1155455 - Assertion failure: dbgobj->zone()->isGCSweeping(), at js/src/gc/Zone.cpp:152

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision de27ac2ab94f (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-eager):

var g = newGlobal();
gczeal(10, 2)
var dbg = Debugger(g);
dbg.onDebuggerStatement = function (frame1) {
    function hit(frame2)
      hit[index] = "mutated";
    var s = frame1.script;
    var offs = s.getLineOffsets(g.line0 + 2);
    for (var i = 0; i < offs.length; i++)
      s.setBreakpoint(offs[i], {hit: hit});
};
var lfGlobal = newGlobal();
evaluate(g.eval("var line0 = Error().lineNumber;\n debugger;\nx = 1;\n"));



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x000000000079082e in JS::Zone::sweepBreakpoints (this=0x7ffff695c000, fop=fop@entry=0x7fffffff6f20) at js/src/gc/Zone.cpp:151
#0  0x000000000079082e in JS::Zone::sweepBreakpoints (this=0x7ffff695c000, fop=fop@entry=0x7fffffff6f20) at js/src/gc/Zone.cpp:151
#1  0x0000000000ab7600 in js::gc::GCRuntime::beginSweepingZoneGroup (this=this@entry=0x7ffff693c348) at js/src/jsgc.cpp:4913
#2  0x0000000000ada42a in js::gc::GCRuntime::sweepPhase (this=this@entry=0x7ffff693c348, sliceBudget=...) at js/src/jsgc.cpp:5227
#3  0x0000000000ae0ac6 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ffff693c348, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:5817
#4  0x0000000000ae1966 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff693c348, incremental=incremental@entry=true, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6000
#5  0x0000000000ae1d25 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff693c348, incremental=incremental@entry=true, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6112
#6  0x0000000000ae30ff in js::gc::GCRuntime::runDebugGC (this=this@entry=0x7ffff693c348) at js/src/jsgc.cpp:6580
#7  0x00000000005b8c4b in js::gc::GCRuntime::gcIfNeededPerAllocation (this=this@entry=0x7ffff693c348, cx=cx@entry=0x7ffff691b4e0) at js/src/gc/Allocator.cpp:28
#8  0x00000000005f0c2f in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0x7ffff693c348, cx=0x7ffff691b4e0, kind=js::gc::BASE_SHAPE) at js/src/gc/Allocator.cpp:55
#9  0x00000000005f9b80 in js::Allocate<js::BaseShape, (js::AllowGC)1> (cx=cx@entry=0x7ffff691b4e0) at js/src/gc/Allocator.cpp:203
#10 0x00000000006b6828 in js::BaseShape::getUnowned (cx=cx@entry=0x7ffff691b4e0, base=...) at js/src/vm/Shape.cpp:1219
#11 0x00000000006b7bd3 in js::EmptyShape::getInitialShape (cx=0x7ffff691b4e0, clasp=0x1994960 <js::SavedFrame::class_>, proto=..., nfixed=<optimized out>, objectFlags=<optimized out>) at js/src/vm/Shape.cpp:1441
#12 0x00000000006b85f1 in js::Shape::replaceLastProperty (cx=cx@entry=0x7ffff691b4e0, base=..., proto=..., shape=shape@entry=...) at js/src/vm/Shape.cpp:327
#13 0x00000000006b86df in js::Shape::setObjectFlags (cx=cx@entry=0x7ffff691b4e0, flags=flags@entry=js::BaseShape::DELEGATE, proto=..., last=0x7ffff5259240) at js/src/vm/Shape.cpp:1190
#14 0x00000000006b878e in JSObject::setFlags (this=0x7ffff52e2080, cx=cx@entry=0x7ffff691b4e0, flags=flags@entry=js::BaseShape::DELEGATE, generateShape=generateShape@entry=JSObject::GENERATE_SHAPE) at js/src/vm/Shape.cpp:1154
#15 0x0000000000618536 in setDelegate (cx=0x7ffff691b4e0, this=<optimized out>) at js/src/jsobj.h:216
#16 CreateBlankProto (cx=cx@entry=0x7ffff691b4e0, clasp=clasp@entry=0x1994960 <js::SavedFrame::class_>, proto=..., global=...) at js/src/vm/GlobalObject.cpp:451
#17 0x000000000061860c in js::GlobalObject::createBlankPrototypeInheriting (this=<optimized out>, cx=cx@entry=0x7ffff691b4e0, clasp=clasp@entry=0x1994960 <js::SavedFrame::class_>, proto=..., proto@entry=...) at js/src/vm/GlobalObject.cpp:472
#18 0x00000000006dbcff in js::GenericCreatePrototype (cx=0x7ffff691b4e0, key=<optimized out>) at js/src/vm/GlobalObject.h:878
#19 0x000000000063f963 in js::GlobalObject::resolveConstructor (cx=cx@entry=0x7ffff691b4e0, global=..., key=key@entry=JSProto_SavedFrame) at js/src/vm/GlobalObject.cpp:157
#20 0x000000000063fe1c in js::GlobalObject::ensureConstructor (cx=cx@entry=0x7ffff691b4e0, global=..., global@entry=..., key=key@entry=JSProto_SavedFrame) at js/src/vm/GlobalObject.cpp:99
#21 0x00000000006c4c2d in getOrCreateSavedFramePrototype (global=..., cx=0x7ffff691b4e0) at js/src/vm/GlobalObject.h:390
#22 js::SavedStacks::createFrameFromLookup (this=this@entry=0x7ffff695a8a8, cx=cx@entry=0x7ffff691b4e0, lookup=..., lookup@entry=...) at js/src/vm/SavedStacks.cpp:1057
#23 0x00000000006c4fa9 in js::SavedStacks::getOrCreateSavedFrame (this=this@entry=0x7ffff695a8a8, cx=cx@entry=0x7ffff691b4e0, lookup=...) at js/src/vm/SavedStacks.cpp:1034
#24 0x00000000006c5d49 in js::SavedStacks::insertFrames (this=this@entry=0x7ffff695a8a8, cx=cx@entry=0x7ffff691b4e0, iter=..., frame=..., frame@entry=..., maxFrameCount=<optimized out>, maxFrameCount@entry=128) at js/src/vm/SavedStacks.cpp:966
#25 0x00000000006c5f8d in js::SavedStacks::saveCurrentStack (this=0x7ffff695a8a8, cx=cx@entry=0x7ffff691b4e0, frame=frame@entry=..., maxFrameCount=128) at js/src/vm/SavedStacks.cpp:805
#26 0x0000000000a1fa26 in JS::CaptureCurrentStack (cx=cx@entry=0x7ffff691b4e0, stackp=..., stackp@entry=..., maxFrameCount=maxFrameCount@entry=128) at js/src/jsapi.cpp:5963
#27 0x0000000000a8fe54 in CaptureStack (stack=..., cx=0x7ffff691b4e0) at js/src/jsexn.cpp:282
#28 js::ErrorToException (cx=cx@entry=0x7ffff691b4e0, message=message@entry=0x7ffff517dd80 "index is not defined", reportp=reportp@entry=0x7fffffff8a90, callback=<optimized out>, userRef=<optimized out>) at js/src/jsexn.cpp:556
#29 0x0000000000a201f8 in ReportError (cx=0x7ffff691b4e0, message=0x7ffff517dd80 "index is not defined", reportp=0x7fffffff8a90, callback=<optimized out>, userRef=<optimized out>) at js/src/jscntxt.cpp:230
#30 0x0000000000a2e75a in js::ReportErrorNumberVA (cx=0x7ffff691b4e0, flags=0, callback=0xa0d1a0 <js::GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=1, argumentsType=js::ArgumentsAreASCII, ap=0x7fffffff8b58) at js/src/jscntxt.cpp:746
#31 0x0000000000a2e7ff in JS_ReportErrorNumberVA (cx=<optimized out>, errorCallback=<optimized out>, userRef=<optimized out>, errorNumber=<optimized out>, ap=ap@entry=0x7fffffff8b58) at js/src/jsapi.cpp:5091
#32 0x0000000000a2e886 in JS_ReportErrorNumber (cx=cx@entry=0x7ffff691b4e0, errorCallback=errorCallback@entry=0xa0d1a0 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1) at js/src/jsapi.cpp:5080
#33 0x0000000000a2f4dc in js::ReportIsNotDefined (cx=cx@entry=0x7ffff691b4e0, id=..., id@entry=...) at js/src/jscntxt.cpp:824
#34 0x0000000000a2f554 in js::ReportIsNotDefined (cx=cx@entry=0x7ffff691b4e0, name=..., name@entry=...) at js/src/jscntxt.cpp:832
#35 0x000000000061a13b in js::GetScopeName (cx=cx@entry=0x7ffff691b4e0, scopeChain=..., scopeChain@entry=..., name=..., name@entry=..., vp=..., vp@entry=...) at js/src/vm/Interpreter.cpp:4147
#36 0x0000000000853e6b in js::jit::DoGetNameFallback (cx=0x7ffff691b4e0, frame=<optimized out>, stub_=<optimized out>, scopeChain=..., res=...) at js/src/jit/BaselineIC.cpp:6208
#37 0x00007ffff7fef03b in ?? ()
#38 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x0	0
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffff6c10	140737488317456
rsp	0x7fffffff6b50	140737488317264
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffff6910	140737488316688
r11	0x7ffff6c27960	140737333328224
r12	0x7ffff695c000	140737330397184
r13	0x0	0
r14	0x7ffff6952800	140737330358272
r15	0x7ffff51493c0	140737305154496
rip	0x79082e <JS::Zone::sweepBreakpoints(js::FreeOp*)+766>
=> 0x79082e <JS::Zone::sweepBreakpoints(js::FreeOp*)+766>:	movl   $0x98,0x0
   0x790839 <JS::Zone::sweepBreakpoints(js::FreeOp*)+777>:	callq  0x423410 <abort@plt>

Comment 1

[Christian Holler (:decoder)]

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/b336dc0af92c
user:        Nick Fitzgerald
date:        Fri Mar 27 13:08:46 2015 -0700
summary:     Bug 1038238 - Part 4: Rename JS::StringifySavedFrameStack to JS::BuildStackString; r=jorendorff

This iteration took 542.063 seconds to run.

Comment 2

[Christian Holler (:decoder)]

Needinfo from fitzgen based on comment 1.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Huh. It isn't immediately clear what is going on here.

We GC under creating the SavedFrame prototype.

Which is under capturing a SavedFrame stack.

Which is under js::ErrorToException for me locally and ReportError in the stack in comment 0.

Somehow this messes up some invariant in the code for sweeping breakpoints. This is where I get lost -- I'm not really sure what the invariant this assertion is trying to maintain is, or why it is so.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

:nbp, you added the assertion that is failing. Can you help explain why it is there and perhaps what might be going wrong for it to fail?

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Nick Fitzgerald [:fitzgen] from comment #4)
> :nbp, you added the assertion that is failing. Can you help explain why it
> is there and perhaps what might be going wrong for it to fail?

Forwarding ni? to the original author of the assertion.

Comment 6

[Jon Coppeard (:jonco)]

> MOZ_ASSERT_IF(isGCSweeping() && dbgobj->zone()->isCollecting(),
>               dbgobj->zone()->isGCSweeping());

This assertion is saying that breakpoints must always be swept in the same zone group as their debugger.

However, I don't think this takes account of breakpoints that are added during incremental sweeping, after we have computed the zone groups.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: bug1155455-breakpoint-sweep-assertion

Update the assertion and add a comment explaining it.

Remove an assertion that can never fail.

Comment 8

[Terrence Cole [:terrence]]

Comment on attachment 8594825 [details] [diff] [review]
bug1155455-breakpoint-sweep-assertion

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

::: js/src/gc/Zone.cpp
@@ +147,5 @@
>              for (Breakpoint* bp = site->firstBreakpoint(); bp; bp = nextbp) {
>                  nextbp = bp->nextInSite();
>                  HeapPtrNativeObject& dbgobj = bp->debugger->toJSObjectRef();
> +
> +                // If we are sweeping then we expect the script and the debugger

I think this needs a comma after the introductory prepositional phrase ending in sweeping. Unless I'm reading it wrong.

@@ +150,5 @@
> +
> +                // If we are sweeping then we expect the script and the debugger
> +                // object to be swept in the same zone group, except if the
> +                // breakpoint was added after we computed the zone groups in
> +                // which case both script and debugger object must be live.

I'd split this into a second sentence for readability. Something like "zone groups. In this case, both script and ...."

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dbe8f15eefa1

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/dbe8f15eefa1