Closed Bug 1182866 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox42 --- wontfix
firefox43 --- wontfix
firefox44 --- fixed
firefox45 --- fixed
firefox46 --- fixed
firefox-esr38 --- affected
firefox-esr45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: gkw, Unassigned)

References

(Blocks 2 open bugs)

Details

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

Attachments

(2 files)

// 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?
Flags: needinfo?(bhackett1024)
Blocks: 1100132
Attached file 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)
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.
Flags: needinfo?(bhackett1024) → needinfo?(shubhamjindal18)
Flags: needinfo?(shubhamjindal18)
Flags: needinfo?(shu)
No longer blocks: 1162199
If this is not related to unboxed objects, could it be a duplicate of bug 1126788?
Flags: needinfo?(bhackett1024)
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.
Flags: needinfo?(bhackett1024)
Flags: needinfo?(shu)
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?
Attachment #8696143 - Flags: review?(jdemooij) → review+
(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 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.
Attachment #8696143 - Flags: approval-mozilla-release?
Attachment #8696143 - Flags: approval-mozilla-beta?
Attachment #8696143 - Flags: approval-mozilla-aurora?
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 0babaa3edcf9).
https://hg.mozilla.org/mozilla-central/rev/63a4acec4bd3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
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.
Attachment #8696143 - Flags: approval-mozilla-beta?
Attachment #8696143 - Flags: approval-mozilla-beta+
Attachment #8696143 - Flags: approval-mozilla-aurora?
Attachment #8696143 - Flags: approval-mozilla-aurora+
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 on attachment 8696143 [details] [diff] [review]
Fix Baseline GETNAME stubs to check for uninitialized lexicals.

Too late to make it into 43.
Attachment #8696143 - Flags: approval-mozilla-release? → approval-mozilla-release-
You need to log in before you can comment on or make changes to this bug.