1121083 - Assertion failure: needsRecompileInfo() && recompInfo, at jit/BaselineDebugModeOSR.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

// Randomly chosen test: js/src/tests/js1_5/Regress/regress-253150.js
options('werror');
// Randomly chosen test: js/src/jit-test/tests/debug/onExceptionUnwind-12.js
g = newGlobal();
g.parent = this;
g.eval("Debugger(parent).onExceptionUnwind = function () {};");
// Randomly chosen test: js/src/jit-test/tests/basic/timeout-check.js
function f(x) {
    if (x === 0) {
        return;
    }
    f(x - 1);
    f(x - 1);
}
timeout(0.01);
f(100);

asserts js debug shell on m-c changeset 67257a3edeb5 with --fuzzing-safe --no-threads --no-ion at Assertion failure: needsRecompileInfo() && recompInfo, at jit/BaselineDebugModeOSR.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

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150110105930" and the hash "a55f093defe1".
The "bad" changeset has the timestamp "20150110111030" and the hash "c213d9d53886".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a55f093defe1&tochange=c213d9d53886

Jan, is bug 1118826 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x10f2d, 0x000000010025842f js-dbg-opt-64-dm-nsprBuild-darwin-67257a3edeb5`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] DebugModeOSREntry::takeRecompInfo(this=<unavailable>) + 67 at BaselineDebugModeOSR.cpp:112, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010025842f js-dbg-opt-64-dm-nsprBuild-darwin-67257a3edeb5`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] DebugModeOSREntry::takeRecompInfo(this=<unavailable>) + 67 at BaselineDebugModeOSR.cpp:112
    frame #1: 0x00000001002583ec js-dbg-opt-64-dm-nsprBuild-darwin-67257a3edeb5`js::jit::RecompileOnStackBaselineScriptsForDebugMode(JSContext*, js::Debugger::ExecutionObservableSet const&, js::Debugger::IsObserving) [inlined] PatchBaselineFramesForDebugMode(cx=<unavailable>, obs=<unavailable>, activation=0x00007fff5fbf9870, entries=0x0000000101d02630, start=<unavailable>) + 913 at BaselineDebugModeOSR.cpp:470
    frame #2: 0x000000010025805b js-dbg-opt-64-dm-nsprBuild-darwin-67257a3edeb5`js::jit::RecompileOnStackBaselineScriptsForDebugMode(cx=<unavailable>, obs=<unavailable>, observing=<unavailable>) + 8827 at BaselineDebugModeOSR.cpp:865
    frame #3: 0x00000001006f819b js-dbg-opt-64-dm-nsprBuild-darwin-67257a3edeb5`js::Debugger::updateExecutionObservabilityOfFrames(cx=0x0000000101d02630, obs=0x00007fff5fbf8958, observing=Observing) + 91 at Debugger.cpp:1844
    frame #4: 0x00000001006ee3ca js-dbg-opt-64-dm-nsprBuild-darwin-67257a3edeb5`js::Debugger::ensureExecutionObservabilityOfFrame(cx=0x0000000101d02630, frame=(ptr_ = 140734799780002)) + 330 at Debugger.cpp:2008
(lldb)

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

The fix, suggested by Shu, is to always set an override pc in AutoDebuggerHandlingException. This also allows us to simplify a condition in BaselineDebugModeOSR.cpp.

Very nice testcase btw.

Comment 3

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

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

http://hg.mozilla.org/mozilla-central/file/67257a3edeb5/js/src/tests/js1_5/Regress/regress-253150.js
http://hg.mozilla.org/mozilla-central/file/67257a3edeb5/js/src/jit-test/tests/debug/onExceptionUnwind-12.js
http://hg.mozilla.org/mozilla-central/file/67257a3edeb5/js/src/jit-test/tests/basic/timeout-check.js

Comment 4

[Jan de Mooij [:jandem]]

[Tracking Requested - why for this release]:

Comment 5

[Shu-yu Guo [:shu]]

Comment on attachment 8548406 [details] [diff] [review]
Patch

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

Great, thanks for the fix!

::: js/src/jit/BaselineDebugModeOSR.cpp
@@ +214,1 @@
>                  // allocation).

Could you delete the 2nd sentence in this comment block? It's not clear what that's explaining anymore with the new override pc scheme.

Comment 6

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/34f416671316

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 8548406 [details] [diff] [review]
Patch

This is a small and safe fix, we should uplift this.

Approval Request Comment
[Feature/regressing bug #]: Bug 1118826.
[User impact if declined]: Crashes when using the debugger in some corner cases.
[Describe test coverage new/current, TBPL]: Passes tests, fixes testcase.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/34f416671316

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Jan de Mooij [:jandem] from comment #7)
> [Feature/regressing bug #]: Bug 1118826.

This bug only landed a day ago on 38. Is this the right bug ref?

Comment 10

[Jan de Mooij [:jandem]]

(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #9)
> This bug only landed a day ago on 38. Is this the right bug ref?

There were two patches and the first patch is the main one and landed in 37, right before the merge. Sorry for the confusion.

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8548406 [details] [diff] [review]
Patch

Gotcha. Missed that. Aurora+

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

This bug doesn't need to be tracked but I'm glad to have it fixed.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c64b0256416e