1268526 - Crash [@ js::Debugger::removeDebuggeeGlobal]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The following testcase crashes on mozilla-central revision 4292da9df16b (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager --no-baseline):

// Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1263899.js
g = newGlobal();
g.parent = this;
g.eval("(" + function() {
    Debugger(parent).onExceptionUnwind = function(frame) {
        frame.older;
    }
} + ")()");
// Adapted from randomly chosen test: js/src/jit-test/tests/debug/prologueFailure-01.js
var handler = {
    get() {
        r;
    }
};
new(new Proxy(function() {}, handler));

Backtrace:

0   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001006eee30 js::Debugger::removeDebuggeeGlobal(js::FreeOp*, js::GlobalObject*, js::detail::HashTable<js::ReadBarriered<js::GlobalObject*> const, js::HashSet<js::ReadBarriered<js::GlobalObject*>, js::MovableCellHasher<js::ReadBarriered<js::GlobalObject*> >, js::RuntimeAllocPolicy>::SetOps, js::RuntimeAllocPolicy>::Enum*) + 528 (jsscript.h:1274)
1   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001006ef701 js::Debugger::detachAllDebuggersFromGlobal(js::FreeOp*, js::GlobalObject*) + 97 (Debugger.cpp:2873)
2   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001005a89cb JSCompartment::sweepGlobalObject(js::FreeOp*) + 59 (Barrier.h:631)
3   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001005f9413 js::gc::GCRuntime::beginSweepingZoneGroup() + 2563 (Zone.h:619)
4   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001005fac33 js::gc::GCRuntime::beginSweepPhase(bool) + 1491 (Statistics.h:408)
5   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001005feb30 js::gc::GCRuntime::incrementalCollectSlice(js::SliceBudget&, JS::gcreason::Reason) + 1104 (SliceBudget.h:77)
6   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001005ff90c js::gc::GCRuntime::gcCycle(bool, js::SliceBudget&, JS::gcreason::Reason) + 460 (jsgc.cpp:6410)
7   js-dbg-64-dm-clang-darwin-4292da9df16b	0x0000000100600275 js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::gcreason::Reason) + 741 (jsgc.cpp:6512)
8   js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001005ef8a6 js::gc::GCRuntime::gc(JSGCInvocationKind, JS::gcreason::Reason) + 86 (jsgc.cpp:6573)
9   js-dbg-64-dm-clang-darwin-4292da9df16b	0x000000010058f24a js::DestroyContext(JSContext*, js::DestroyContextMode) + 458 (Utility.h:384)
10  js-dbg-64-dm-clang-darwin-4292da9df16b	0x00000001000064a5 main + 12405 (js.cpp:7487)
11  js-dbg-64-dm-clang-darwin-4292da9df16b	0x0000000100001474 start + 52

For detailed crash information, see attachment.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9d99253b3e00
user:        Eric Faust
date:        Mon Mar 14 14:29:12 2016 -0700
summary:     Bug 1251921 - Do not call debugger hooks with half-initialized frame if InterpeterFrame::prologue fails. (r=jorendorff)

Eric, is bug 1251921 a likely regressor?

Comment 3

[Eric Faust [:efaust]]

Attachment: Fix

Comment 4

[Jason Orendorff [:jorendorff]]

Wow.

Comment 5

[Eric Faust [:efaust]]

Attachment: Fix

Depends on patch in bug 1270331.

Now with a lot less special looking case.

Comment 6

[Eric Faust [:efaust]]

This crash will only reproduce when the jits are disabled. There's no user exposure here. No need to uplift. WONTFIX 47,48.

Comment 7

[Shu-yu Guo [:shu]]

Comment on attachment 8748972 [details] [diff] [review]
Fix

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

Looks good, thanks!

::: js/src/vm/Debugger.cpp
@@ +5861,2 @@
>  Debugger::observesFrame(AbstractFramePtr frame) const
>  {

In the AFP overload, could you assert that if frame.isInterpreterFrame() && frame.isFunctionFrame(), then !thisv.isMagic() || thisv.whyMagic() != JS_IS_CONSTRUCTING?

@@ +5869,5 @@
> +    // Skip frames not yet fully initialized during their prologue.
> +    if (iter.isInterp() && iter.isFunctionFrame()) {
> +        const Value& thisVal = iter.interpFrame()->thisArgument();
> +        if (thisVal.isMagic() && thisVal.whyMagic() == JS_IS_CONSTRUCTING)
> +            return false;;

Nit: extra ;

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Eric, is this ready for landing?

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I have a testcase asserting at:

Assertion failure: !inFrameMaps(frame), at vm/Debugger-inl.h

but it seems to be fixed by this patch.

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Backtrace for assertion failure in comment 9:

#0  0x00000000006d3a88 in js::Debugger::onLeaveFrame (cx=0x7f64f0a1a400, frame=..., pc=0x7f64f0a3781e "\005\231\220\r\226\210\006\233\200", ok=true) at /home/fuzz3lin/trees/mozilla-central/js/src/vm/Debugger-inl.h:25
#1  0x0000000000a7dadc in Interpret (cx=0x7f64f0a1a400, state=...) at /home/fuzz3lin/trees/mozilla-central/js/src/vm/Interpreter.cpp:1972
#2  0x0000000000a8a2ee in js::RunScript (cx=cx@entry=0x7f64f0a1a400, state=...) at /home/fuzz3lin/trees/mozilla-central/js/src/vm/Interpreter.cpp:426
#3  0x0000000000a8c53b in js::ExecuteKernel (cx=cx@entry=0x7f64f0a1a400, script=..., script@entry=..., scopeChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x0) at /home/fuzz3lin/trees/mozilla-central/js/src/vm/Interpreter.cpp:704
#4  0x0000000000a8cb18 in js::Execute (cx=cx@entry=0x7f64f0a1a400, script=..., script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at /home/fuzz3lin/trees/mozilla-central/js/src/vm/Interpreter.cpp:737
#5  0x00000000008b73fc in ExecuteScript (cx=cx@entry=0x7f64f0a1a400, scope=scope@entry=..., script=script@entry=..., rval=rval@entry=0x0) at /home/fuzz3lin/trees/mozilla-central/js/src/jsapi.cpp:4425
#6  0x00000000008b76ff in JS_ExecuteScript (cx=cx@entry=0x7f64f0a1a400, scriptArg=scriptArg@entry=...) at /home/fuzz3lin/trees/mozilla-central/js/src/jsapi.cpp:4458

/snip

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc090fbcc8cb
Don't observe half-initalized frames from the debugger. (r=shu)

Comment 12

[Carsten Book [:Tomcat]]

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

Comment 13

[Eric Faust [:efaust]]

Landed.