Closed Bug 1122335 Opened 5 years ago Closed 5 years ago

Assertion failure: !scope_->is<ScopeObject>(), at vm/ScopeObject.h

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- affected

People

(Reporter: gkw, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

// jsfunfuzz-generated code
options('strict_mode');
// Randomly chosen test: js/src/jit-test/tests/debug/onEnterFrame-03.js
g = newGlobal()
Debugger(g).onEnterFrame = function(x) {
    x.eval("a")
}
g.eval("a")

asserts js debug shell on m-c changeset 89a190592267 with --fuzzing-safe --no-threads --no-ion at Assertion failure: !scope_->is<ScopeObject>(), at vm/ScopeObject.h.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

This was found by combining random js tests together with jsfunfuzz, the specific file(s) is/are:

http://hg.mozilla.org/mozilla-central/file/89a190592267/js/src/jit-test/tests/debug/onEnterFrame-03.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/fb00dedf441c
user:        Shu-yu Guo
date:        Wed Jan 14 22:57:35 2015 -0800
summary:     Bug 963879 - Part 1: Overhaul ScopeIter and StaticScopeIter to share iteration logic and to go through evals. (r=luke)

Shu-yu, is bug 963879 a likely regressor? The patch in bug 1122246 comment 5 did not seem to fix this issue.
Flags: needinfo?(shu)
Attached file stack
(lldb) bt 5
* thread #1: tid = 0xddb36, 0x00000001007d180e js-dbg-opt-64-dm-nsprBuild-darwin-89a190592267`GetDebugScope(JSContext*, js::ScopeIter const&) [inlined] js::ScopeIter::enclosingScope() const + 67 at ScopeObject.h:1080, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001007d180e js-dbg-opt-64-dm-nsprBuild-darwin-89a190592267`GetDebugScope(JSContext*, js::ScopeIter const&) [inlined] js::ScopeIter::enclosingScope() const + 67 at ScopeObject.h:1080
    frame #1: 0x00000001007d17cb js-dbg-opt-64-dm-nsprBuild-darwin-89a190592267`GetDebugScope(JSContext*, js::ScopeIter const&) [inlined] GetDebugScopeForNonScopeObject(js::ScopeIter const&) at ScopeObject.cpp:2587
    frame #2: 0x00000001007d17cb js-dbg-opt-64-dm-nsprBuild-darwin-89a190592267`GetDebugScope(cx=<unavailable>, si=<unavailable>) + 2651 at ScopeObject.cpp:2603
    frame #3: 0x00000001007d1d2a js-dbg-opt-64-dm-nsprBuild-darwin-89a190592267`js::GetDebugScopeForFrame(cx=0x0000000101e01cf0, pc=0x000000010411b249, frame=<unavailable>) + 202 at ScopeObject.cpp:2633
    frame #4: 0x000000010070d58b js-dbg-opt-64-dm-nsprBuild-darwin-89a190592267`DebuggerGenericEval(cx=0x0000000101e01cf0, fullMethodName=<unavailable>, code=<unavailable>, evalWithBindings=EvalWithDefaultBindings, dbg=<unavailable>, iter=0x00007fff5fbf8380, bindings=<unavailable>, options=<unavailable>, vp=<unavailable>, scope=<unavailable>) + 2155 at Debugger.cpp:6042
(lldb)
Existing bug, the scope rework surfaced it.

Strict eval always creates an extra scope. Debugger.Frame.prototype.eval underlyingly calls the same code as JS eval, and so ends up creating an extra strict-eval CallObject scope even when called via D.F. At the same time, there's an engine-wide invariant that a dynamic scope chain is a series of ScopeObjects terminated by a non-ScopeObject (e.g., a GlobalObject or a DebugScopeProxy).

In the above test case, we end up getting a dynamic scope chain that looks like:

CallObject for the g.eval
          |
   DebugScopeProxy
          |
CallObject for the x.eval

My patch unifies dynamic and static scope chain iteration. To be able to iterate across eval scopes, I introduced a StaticEvalObject. D.F.p.eval'd code do *not* introduce a StaticEvalObject, because we're injecting a non-ScopeObject in the middle of the scope chain. (Aside: this is a good thing, else we'll keep wrapping DebugScopeProxies around DebugScopeProxies.) Coming back to the present bug, since the underlying eval implementation creates a CallObject for strict mode regardless of coming from Debugger or regular JS, there's now a mismatch between the static scope chain and the dynamic scope chain. Since the Debugger eval script has no static scope chain, the dynamic scope chain must be a non-ScopeObject per the above invariant. In the non-strict case, this is true: a DebugScopeProxy isn't a ScopeObject. In the strict case, we get this incorrectly interposed CallObject.
Flags: needinfo?(shu)
Duplicate of this bug: 1122768
Comment 2 is kinda wrong, and I was overthinking it.

Turns out the correct fix was quite simple -- just create a StaticEvalObject
that terminates immediately, instead of linking up to the non-ScopeObject env.
Attachment #8552054 - Flags: review?(jimb)
Comment on attachment 8552054 [details] [diff] [review]
Fix static scope chain for Debugger.Frame.prototype.eval.

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

Looks good to me, now that I understand it.
Attachment #8552054 - Flags: review?(jimb) → review+
https://hg.mozilla.org/mozilla-central/rev/d082ba907669
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.