1132265 - Assertion failure: entry.isIon() || entry.isBaseline() || entry.isIonCache(), at jit/JitFrames.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

// Randomly chosen test: js/src/jit-test/tests/basic/bug908915.js
(function() {
    function callFromJIT(f) {
        f();
    }

    const fs = [
        function() {
            /* warm up the caller */
        },
        function() {
            enableSPSProfiling();
            enableSingleStepProfiling();
        },
    ];

    for each (let f in fs) {
        callFromJIT(f);
    }
})();

asserts js debug shell on m-c changeset ee093ca70666 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: entry.isIon() || entry.isBaseline() || entry.isIonCache(), at jit/JitFrames.cpp.

Debug configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-arm-simulator --enable-debug --enable-profiling --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/fuzzing/js/compileShell.py -b "--32 --enable-debug --enable-profiling --enable-nspr-build --enable-more-deterministic --enable-arm-simulator -R ~/trees/mozilla-central" -r ee093ca70666

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/70a8168c7d24
user:        Kannan Vijayan
date:        Thu Jan 15 20:11:21 2015 -0500
summary:     Bug 1057082 - 3/7 - Modify jits to use lastProfilingFrame and lastProfilingCallSite fields. r=jandem

Setting s-s by default because this involves the profiler, see bug 1124036 comment 4. Big thanks to Jesse for helping reduce this testcase.

Kannan, is bug 1057082 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x302164, 0x00503761 js-dbg-32-prof-dm-nsprBuild-armSim-darwin-ee093ca70666`js::jit::JitProfilingFrameIterator::tryInitWithTable(this=<unavailable>, table=<unavailable>, pc=<unavailable>, rt=<unavailable>, forLastCallSite=<unavailable>) + 401 at JitFrames.cpp:2892, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00503761 js-dbg-32-prof-dm-nsprBuild-armSim-darwin-ee093ca70666`js::jit::JitProfilingFrameIterator::tryInitWithTable(this=<unavailable>, table=<unavailable>, pc=<unavailable>, rt=<unavailable>, forLastCallSite=<unavailable>) + 401 at JitFrames.cpp:2892
    frame #1: 0x0050329c js-dbg-32-prof-dm-nsprBuild-armSim-darwin-ee093ca70666`js::jit::JitProfilingFrameIterator::JitProfilingFrameIterator(this=0xbfffe244, rt=0x02041400, state=<unavailable>) + 252 at JitFrames.cpp:2796
    frame #2: 0x00291f6e js-dbg-32-prof-dm-nsprBuild-armSim-darwin-ee093ca70666`JS::ProfilingFrameIterator::iteratorConstruct(this=0xbfffe244, state=<unavailable>) + 78 at Stack.cpp:1796
    frame #3: 0x00291e37 js-dbg-32-prof-dm-nsprBuild-armSim-darwin-ee093ca70666`JS::ProfilingFrameIterator::ProfilingFrameIterator(this=0xbfffe238, rt=0x02041400, state=<unavailable>) + 103 at Stack.cpp:1737
    frame #4: 0x00016e06 js-dbg-32-prof-dm-nsprBuild-armSim-darwin-ee093ca70666`SingleStepCallback(arg=0xbfffe238, sim=0x0204de00, pc=<unavailable>) + 166 at js.cpp:4174
(lldb)

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 3

[Kannan Vijayan [:djvj]]

Attachment: fix-bug-1132265.patch

We need to handle dummy frames in tryInitWithTable in ProfilingFrameIterator.

Comment 4

[Shu-yu Guo [:shu]]

Comment on attachment 8576237 [details] [diff] [review]
fix-bug-1132265.patch

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

Seems okay to me. But why does this happen? Turning on profiling with on-stack frames?

Comment 5

[Kannan Vijayan [:djvj]]

Yeah, specifically triggered by the single-stepper.  Profiling is turned on for a frame.  Typically with Ion, this will cause invalidation of the frame (so we'll bail immediately after we return to the ion frame, and there isn't a high chance of the sampler catching it within that).  With the single stepper turned on, we do profiling at every instruction boundary, so we definitely hit the frame before it gets bailed out, and we see the frameptr with a dummy mapping.

Comment 6

[Kannan Vijayan [:djvj]]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Do we need this on esr38?

Comment 9

[Kannan Vijayan [:djvj]]

It's a security issue that's definitely present, but it's also extremely hard to trigger and only presents when the profiler is enabled.

On the other hand, it's a very low-risk patch that adds a few lines to a single file.  I'd recommend including it in esr38.  No need to expose ourselves when it's easy enough to close off a vector.  However, if we are otherwise pressed for time and need to leave it out, it shouldn't be too big of a deal.

What are the criteria for inclusion in esr38 is what i'm asking, I guess..

Comment 10

[Ryan VanderMeulen [:RyanVM]]

sec-moderates aren't slam-dunk approvals for ESR releases, but I've seen them taken before if the patch author feels strongly enough about it. Personally, I'd nominate it and state your case. Can't hurt to ask.