1122886 - Assertion failure: Baseline OSR lastProfilingFrame mismatch., at jit/MacroAssembler.cpp

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

for (var j = 0; j < 1; ++j) {
    (function() {
        enableSPSProfiling()
    })()
}

asserts js debug shell on m-c changeset 369a8f14ccf8 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: Baseline OSR lastProfilingFrame mismatch., at jit/MacroAssembler.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 "20150114125936" and the hash "9c75fef0e790".
The "bad" changeset has the timestamp "20150114132037" and the hash "99213cacd671".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9c75fef0e790&tochange=99213cacd671

Setting s-s to be safe because this seems to concern the SPS profiler.

Kannan, is bug 1057082 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt
* thread #1: tid = 0xc033, 0x0000000101bfc9c3, queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0)
  * frame #0: 0x0000000101bfc9c3
(lldb)

Comment 2

[Kannan Vijayan [:djvj]]

Yes, this was most likely introduced by bug 1057082.  Taking.

Comment 3

[Kannan Vijayan [:djvj]]

Attachment: fix-bug-1122886.patch

The issue here is an over-conservative assertion check.

We check, when OSR-ing with profiling turned on, that the current frame is the one registered as the 'lastProfilingFrame'.  However, the 'lastProfilingFrame' can be null in certain circumstances, namely:

1. We enter a baseline frame for a function.
2. The function enters a loop.
3. The function calls a callee within a loop (establishing a new JitActivation) which turns on profiling.
4. This sets the lastProfilingFrame on the new JitActivation.
5. With profiling turned on, the callee returns to the main function, popping the new JitActivation and returning to the old JitActivation with a null lastProfilingFrame.
6. The function then OSRs
7. The debug check fails because the JitActivation has a null lastProfilingFrame, and profiling is turned on.

Fix is simple: allow null for lastProfilingFrame when OSR-ing.

This is not a secure bug.  Not even sec-low.  It's just overprotective debug logic.

Comment 4

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

(In reply to Kannan Vijayan [:djvj] from comment #3)
> This is not a secure bug.  Not even sec-low.  It's just overprotective debug
> logic.

Opening up.

Comment 5

[Hannes Verschore [:h4writer]]

Comment on attachment 8552587 [details] [diff] [review]
fix-bug-1122886.patch

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

::: js/src/jit/BaselineIC.cpp
@@ +1015,5 @@
>          masm.branch32(Assembler::Equal, addressOfEnabled, Imm32(0), &checkOk);
>          masm.loadPtr(AbsoluteAddress((void*)&cx->mainThread().jitActivation), scratchReg);
>          masm.loadPtr(Address(scratchReg, JitActivation::offsetOfLastProfilingFrame()), scratchReg);
> +
> +        // It may be the case that we entered the baseline frame without profiling turned

"with profiling turned off,"

Comment 6

[Kannan Vijayan [:djvj]]

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

Comment 7

[Phil Ringnalda (:philor)]

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

Comment 8

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.