1182866 - Assertion failure: data.s.payload.why == why, at dist/include/js/Value.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

// Randomly chosen test: js/src/jit-test/tests/ion/lexical-check-2.js
with(7) {
    function f() {
        if (i == 15) {
            g();
        }
        const x = 42;
        function g() {
            return x;
        }
        return g;
    }
}
for (var i = 0; i < 99; i++) {
    assertEq(f()(), 42);
}

asserts js debug shell on m-c changeset 7ec3e4b2a45f with --fuzzing-safe --no-threads --no-ion at Assertion failure: data.s.payload.why == why, at dist/include/js/Value.h.

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-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build" -r 7ec3e4b2a45f

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/322487136b28
user:        Brian Hackett
date:        Sun May 17 20:12:14 2015 -0600
summary:     Bug 1162199 - Use unboxed objects by default, r=jandem.

Brian, is bug 1162199 a likely regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x17cb35, 0x0000000100486039 js-dbg-64-dm-nsprBuild-darwin-7ec3e4b2a45f`js::jit::DoTypeMonitorFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICTypeMonitor_Fallback*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) + 52 at Value.h:1174, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000100486039 js-dbg-64-dm-nsprBuild-darwin-7ec3e4b2a45f`js::jit::DoTypeMonitorFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICTypeMonitor_Fallback*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) + 52 at Value.h:1174
    frame #1: 0x0000000100486005 js-dbg-64-dm-nsprBuild-darwin-7ec3e4b2a45f`js::jit::DoTypeMonitorFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICTypeMonitor_Fallback*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) [inlined] js::ValueOperations<JS::Handle<JS::Value> >::isMagic(why=JS_OPTIMIZED_OUT) const at Value.h:1680
    frame #2: 0x0000000100486005 js-dbg-64-dm-nsprBuild-darwin-7ec3e4b2a45f`js::jit::DoTypeMonitorFallback(cx=<unavailable>, frame=<unavailable>, stub=<unavailable>, value=<unavailable>, res=<unavailable>) + 1045 at BaselineIC.cpp:506
    frame #3: 0x0000000101fb9657
    frame #4: 0x0000000101fb1dc4
(lldb)

Comment 2

[Brian Hackett [Laid off!]]

I don't think this is related to unboxed objects.  If I disable generation of unboxed objects then the assertion still reproduces.  The bytecode for g() here starts with a GETNAME x, which fetches the uninitialized lexical value and then starts to pass it around without any check for whether it is initialized or not.

Comment 3

[Christian Holler (:decoder)]

If this is not related to unboxed objects, could it be a duplicate of bug 1126788?

Comment 4

[Brian Hackett [Laid off!]]

I don't think this is a dupe of that bug.  In that bug the correct opcode was emitted (jsop_initaliasedlexical) but it is ion compiled in a way that doesn't account for this magic uninitialized lexical value.  In this bug the opcode that is emitted (jsop_getname) doesn't account for the uninitialized lexical value, even if ion is not enabled.

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Fix Baseline GETNAME stubs to check for uninitialized lexicals.

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8696143 [details] [diff] [review]
Fix Baseline GETNAME stubs to check for uninitialized lexicals.

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

Do you know which branches are affected?

Comment 7

[Shu-yu Guo [:shu]]

(In reply to Jan de Mooij [:jandem] from comment #6)
> Comment on attachment 8696143 [details] [diff] [review]
> Fix Baseline GETNAME stubs to check for uninitialized lexicals.
> 
> Review of attachment 8696143 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Do you know which branches are affected?

I'm afraid every branch is affected.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63a4acec4bd3

Comment 9

[Shu-yu Guo [:shu]]

Comment on attachment 8696143 [details] [diff] [review]
Fix Baseline GETNAME stubs to check for uninitialized lexicals.

Approval Request Comment
[Feature/regressing bug #]: TDZ support
[User impact if declined]: Possible incorrect behavior. Probably rare.
[Describe test coverage new/current, TreeHerder]: on TH
[Risks and why]: Low, bug fix only.
[String/UUID change made/needed]: None.

Comment 10

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 0babaa3edcf9).

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/63a4acec4bd3

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8696143 [details] [diff] [review]
Fix Baseline GETNAME stubs to check for uninitialized lexicals.

Fixes a test assertion failure. Verified, taking it in Beta44 and Aurora45.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/87ca4a7f2c2a

Comment 14

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Too late for 43, as I would like to minimize any risk from changes in this dot release. Since it has been a regression since 40, I hope that won't have too much new impact in 43.

Comment 15

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8696143 [details] [diff] [review]
Fix Baseline GETNAME stubs to check for uninitialized lexicals.

Too late to make it into 43.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/87ca4a7f2c2a