Crash [@ js::Debugger::removeDebuggeeGlobal]

RESOLVED FIXED in Firefox 50

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gkw, Assigned: efaust)

Tracking

(Blocks: 2 bugs, {crash, testcase})

Trunk
mozilla50
x86_64
Mac OS X
crash, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 wontfix, firefox48 wontfix, firefox49 affected, firefox50 fixed)

Details

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

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
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.
(Reporter)

Comment 1

2 years ago
Created attachment 8746568 [details]
Detailed Crash Information
(Reporter)

Comment 2

2 years ago
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?
Blocks: 1251921
status-firefox47: --- → affected
status-firefox48: --- → affected
Flags: needinfo?(efaustbmo)
(Assignee)

Comment 3

2 years ago
Created attachment 8748416 [details] [diff] [review]
Fix
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
Flags: needinfo?(efaustbmo)
Attachment #8748416 - Flags: review?(shu)
(Assignee)

Comment 5

2 years ago
Created attachment 8748972 [details] [diff] [review]
Fix

Depends on patch in bug 1270331.

Now with a lot less special looking case.
Attachment #8748416 - Attachment is obsolete: true
Attachment #8748416 - Flags: review?(shu)
Attachment #8748972 - Flags: review?(shu)
(Assignee)

Comment 6

2 years ago
This crash will only reproduce when the jits are disabled. There's no user exposure here. No need to uplift. WONTFIX 47,48.
status-firefox47: affected → wontfix
status-firefox48: affected → wontfix

Comment 7

2 years ago
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 ;
Attachment #8748972 - Flags: review?(shu) → review+
(Reporter)

Comment 8

2 years ago
Eric, is this ready for landing?
Flags: needinfo?(efaustbmo)
(Reporter)

Comment 9

2 years ago
I have a testcase asserting at:

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

but it seems to be fixed by this patch.
(Reporter)

Comment 10

2 years ago
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

2 years ago
Pushed by efaustbmo@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc090fbcc8cb
Don't observe half-initalized frames from the debugger. (r=shu)

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bc090fbcc8cb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(Assignee)

Comment 13

2 years ago
Landed.
Flags: needinfo?(efaustbmo)
You need to log in before you can comment on or make changes to this bug.