1103027 - Crash [@ js::jit::RecompileOnStackBaselineScriptsForDebugMode] or Assertion failure: result, at jit/BaselineJIT.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

// Random chosen test: js/src/jit-test/tests/debug/onExceptionUnwind-06.js
g = newGlobal()
g.parent = this
g.eval("Debugger(parent).onExceptionUnwind = function(){}")
// Random chosen test: js/src/jit-test/tests/ion/bug925067-2.js
o = {
    next: function(y) {
        if (y > 0) {
            throw 4
        }
    }
}
o.next(0)
o.next(1)

asserts js debug shell on m-c changeset 7d17b594834f with --ion-eager --no-threads at Assertion failure: result, at jit/BaselineJIT.cpp.

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 jit-tests together with jsfunfuzz, the specific file(s) is/are:

http://hg.mozilla.org/mozilla-central/file/7d17b594834f/js/src/jit-test/tests/debug/onExceptionUnwind-06.js
http://hg.mozilla.org/mozilla-central/file/7d17b594834f/js/src/jit-test/tests/ion/bug925067-2.js

Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b160657339f8
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:39 2014 -0800
summary:     Bug 1032869 - Part 2: Move debuggee-ness to frames and selectively deoptimize when Debugger needs to observe execution. (r=jimb)

changeset:   https://hg.mozilla.org/mozilla-central/rev/bb2f13ba7b1c
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1062629 - Off-thread compartment debug mode should match main thread compartment debug mode. (r=jimb)

changeset:   https://hg.mozilla.org/mozilla-central/rev/1176cc3c3b34
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1063328 - Fix on-stack live iterator handling when bailing out in-place due to debug mode OSR. (r=jandem)

changeset:   https://hg.mozilla.org/mozilla-central/rev/f8e316fa65bb
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1063330 - Remove the JS shell's evalInFrame. (r=jimb)

changeset:   https://hg.mozilla.org/mozilla-central/rev/96a2f59f6ce4
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:40 2014 -0800
summary:     Bug 1032869 - Part 3: Don't consider onExceptionUnwind an all-execution-observing hook. (r=jandem)

changeset:   https://hg.mozilla.org/mozilla-central/rev/06d07689a043
user:        Shu-yu Guo
date:        Thu Nov 13 14:39:41 2014 -0800
summary:     Bug 1032869 - Part 4: Add an auto-updated DebugModeOSRVolatileJitFrameIterator. (r=jandem)

Shu-yu, are any of these bugs likely regressors?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x279465, 0x000000010023b501 js-dbg-opt-64-dm-nsprBuild-darwin-7d17b594834f`js::jit::BaselineScript::icEntryFromReturnAddress(this=<unavailable>, returnAddr=<unavailable>) + 193 at BaselineJIT.cpp:577, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010023b501 js-dbg-opt-64-dm-nsprBuild-darwin-7d17b594834f`js::jit::BaselineScript::icEntryFromReturnAddress(this=<unavailable>, returnAddr=<unavailable>) + 193 at BaselineJIT.cpp:577
    frame #1: 0x00000001001faccd js-dbg-opt-64-dm-nsprBuild-darwin-7d17b594834f`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] CollectJitStackScripts(cx=<unavailable>, obs=<unavailable>, activation=0x00007fff5fbfdeb0, entries=0x0000000101b14ed0) + 532 at BaselineDebugModeOSR.cpp:200
    frame #2: 0x00000001001faab9 js-dbg-opt-64-dm-nsprBuild-darwin-7d17b594834f`js::jit::RecompileOnStackBaselineScriptsForDebugMode(cx=0x0000000101b14ed0, obs=0x00007fff5fbfcfb8, observing=Observing) + 521 at BaselineDebugModeOSR.cpp:758
    frame #3: 0x0000000100626ed9 js-dbg-opt-64-dm-nsprBuild-darwin-7d17b594834f`js::Debugger::updateExecutionObservabilityOfFrames(cx=0x0000000101b14ed0, obs=0x00007fff5fbfcfb8, observing=Observing) + 89 at Debugger.cpp:1823
    frame #4: 0x000000010061cc1a js-dbg-opt-64-dm-nsprBuild-darwin-7d17b594834f`js::Debugger::ensureExecutionObservabilityOfFrame(cx=0x0000000101b14ed0, frame=(ptr_ = 140734799797954)) + 330 at Debugger.cpp:1988
(lldb)

Comment 2

[Shu-yu Guo [:shu]]

Attachment: Handle bailed-out-in-place Baseline frames in debug mode OSR.

Jan, as we discussed on IRC, the bug is that if we are bailing out in place to
service an onExceptionUnwind, we could settle the bailed out frame on a pc
without an ICEntry, which debug mode OSR doesn't know how to handle.

Comment 3

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

This also crashes 64-bit opt shell on m-c rev 8c02f3280d0c:

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

Comment 4

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

Attachment: stack for opt crash

(lldb) bt 5
* thread #1: tid = 0x3ef61d, 0x0000000100158b80 js-dbgDisabled-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] js::jit::ICEntry::pcOffset(this=0x0000000000000000) const at BaselineIC.h:274, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xc)
  * frame #0: 0x0000000100158b80 js-dbgDisabled-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] js::jit::ICEntry::pcOffset(this=0x0000000000000000) const at BaselineIC.h:274
    frame #1: 0x0000000100158b80 js-dbgDisabled-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] DebugModeOSREntry::DebugModeOSREntry(script=<unavailable>, icEntry=0x0000000000000000) at BaselineDebugModeOSR.cpp:50
    frame #2: 0x0000000100158b80 js-dbgDisabled-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] DebugModeOSREntry::DebugModeOSREntry(script=<unavailable>, icEntry=0x0000000000000000) at BaselineDebugModeOSR.cpp:57
    frame #3: 0x0000000100158b80 js-dbgDisabled-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] CollectJitStackScripts(cx=<unavailable>, obs=<unavailable>, activation=0x00007fff5fbfe530, entries=0x0000000103001f80) + 714 at BaselineDebugModeOSR.cpp:201
    frame #4: 0x00000001001588b6 js-dbgDisabled-opt-64-dm-nsprBuild-darwin-8c02f3280d0c`js::jit::RecompileOnStackBaselineScriptsForDebugMode(cx=<unavailable>, obs=0x00007fff5fbfd7d0, observing=Observing) + 518 at BaselineDebugModeOSR.cpp:758
(lldb)

Comment 5

[Jan de Mooij [:jandem]]

Shu, we can set the flag on the frame and then re-enter JS for a catch/finally block, with the flag still set, right? It'd be nice if we didn't set the flag in this case...

Comment 6

[Shu-yu Guo [:shu]]

(In reply to Jan de Mooij [:jandem] from comment #5)
> Shu, we can set the flag on the frame and then re-enter JS for a
> catch/finally block, with the flag still set, right? It'd be nice if we
> didn't set the flag in this case...

I was thinking of actually moving it to a DEBUG-only thing, because it has no uses other than asserts.

Comment 7

[Shu-yu Guo [:shu]]

Attachment: Handle bailed-out-in-place Baseline frames in debug mode OSR.

Only set the flag during calling out to Debugger from inside HandleExceptionBaseline.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8527970 [details] [diff] [review]
Handle bailed-out-in-place Baseline frames in debug mode OSR.

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

::: js/src/jit/IonFrames.cpp
@@ +550,5 @@
>  
> +struct AutoDebuggerHandlingException
> +{
> +    BaselineFrame *frame;
> +    AutoDebuggerHandlingException(BaselineFrame *frame)

Nit: make the constructor |explicit|

@@ +553,5 @@
> +    BaselineFrame *frame;
> +    AutoDebuggerHandlingException(BaselineFrame *frame)
> +      : frame(frame)
> +    {
> +        frame->setIsDebuggerHandlingException();

Maybe wrap these calls + the frame flag/methods in #ifdef DEBUG to make it clear this is a debug-only thing?

Comment 9

[Shu-yu Guo [:shu]]

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

Comment 10

[Carsten Book [:Tomcat]]

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