Closed Bug 1121083 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox36 --- unaffected
firefox37 - fixed
firefox38 - fixed

People

(Reporter: gkw, Assigned: jandem)

References

(Blocks 2 open bugs)

Details

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

Attachments

(2 files)

// 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?
Flags: needinfo?(jdemooij)
Attached file 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)
Attached patch PatchSplinter Review
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.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8548406 - Flags: review?(shu)
[Tracking Requested - why for this release]:
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.
Attachment #8548406 - Flags: review?(shu) → review+
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.
Attachment #8548406 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/34f416671316
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(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?
Flags: needinfo?(jdemooij)
(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.
Flags: needinfo?(jdemooij)
Comment on attachment 8548406 [details] [diff] [review]
Patch

Gotcha. Missed that. Aurora+
Attachment #8548406 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This bug doesn't need to be tracked but I'm glad to have it fixed.
You need to log in before you can comment on or make changes to this bug.