1086842 - Assertion failure: [infer failure] Missing type in object [0x10512ecf0] value: [0x10512e858], at js/src/jsinfer.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

x = []
for (v of x) {}
Object.defineProperty(x, 1, {})
Set(Object.create([0, Number, []]))

asserts js debug shell on m-c changeset 33c0181c4a25 with --ion-eager --no-threads at Assertion failure: [infer failure] Missing type in object [0x10512ecf0] value: [0x10512e858], at js/src/jsinfer.cpp.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar 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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/2048240a81d2
user:        Jan de Mooij
date:        Tue Apr 29 08:54:46 2014 +0200
summary:     Bug 1000942 - Eliminate some unnecessary object type barriers. r=bhackett

Setting s-s and assuming sec-critical because this is a type inference failure.

Jan, is bug 1000942 a possible regressor?

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x8a14d, 0x00000001004fefaf js-dbg-opt-64-dm-nsprBuild-darwin-33c0181c4a25`js::types::TypeFailure(JSContext*, char const*, ...) [inlined] js::ExclusiveContext::compartment() const + 52 at jscntxt.h:367, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001004fefaf js-dbg-opt-64-dm-nsprBuild-darwin-33c0181c4a25`js::types::TypeFailure(JSContext*, char const*, ...) [inlined] js::ExclusiveContext::compartment() const + 52 at jscntxt.h:367
    frame #1: 0x00000001004fef7b js-dbg-opt-64-dm-nsprBuild-darwin-33c0181c4a25`js::types::TypeFailure(cx=<unavailable>, fmt=<unavailable>) + 411 at jsinfer.cpp:287
    frame #2: 0x00000001004feddf js-dbg-opt-64-dm-nsprBuild-darwin-33c0181c4a25`js::types::TypeHasProperty(cx=<unavailable>, obj=<unavailable>, value=<unavailable>, id=<unavailable>) + 847 at jsinfer.cpp:264
    frame #3: 0x000000010064246b js-dbg-opt-64-dm-nsprBuild-darwin-33c0181c4a25`bool NativeGetInline<(cx=0x0000000101b0eb00, obj=<unavailable>, receiver=<unavailable>, pobj=<unavailable>, shape=<unavailable>, vp=<unavailable>)1>(JSContext*, js::MaybeRooted<JSObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::Shape*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) + 379 at NativeObject.cpp:1596
    frame #4: 0x000000010064280b js-dbg-opt-64-dm-nsprBuild-darwin-33c0181c4a25`bool GetPropertyHelperInline<(cx=0x0000000101b0eb00, obj=<unavailable>, receiver=<unavailable>, id=<unavailable>, vp=<unavailable>)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<jsid, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) + 267 at NativeObject.cpp:1874
(lldb)

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Fix

Problem is that we have a MGetElementCache with a BarrierKind::TypeTagOnly barrier, and we set the "monitoredResult = true" flag on the cache. This means the cache doesn't call TypeScript::Monitor, but this is wrong: with TypeTagOnly we don't check the object types.

Fix is to only set monitoredResult = true for BarrierKind::TypeSet. Also fixes the same issue with MGetPropertyCache.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Add debug checks

Some debug-only code to verify that if we have a TypeTagOnly barrier, the object type is in the TypeSet, to catch bugs sooner.

This patch can land after the first patch is on all branches.

Comment 4

[Jan de Mooij [:jandem]]

Very nice catch btw, Gary! This is bad.

Part 2 should make it much easier for the fuzzers to find those bugs.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8511153 [details] [diff] [review]
Part 1 - Fix

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
Not trivial but with some work it's doable.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No.

> Which older supported branches are affected by this flaw?
32+

> If not all supported branches, which bug introduced the flaw?
Bug 1000942.

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Should be easy.

> How likely is this patch to cause regressions; how much testing does it need?
Unlikely to cause regressions.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 8511153 [details] [diff] [review]
Part 1 - Fix

sec-approval+ and a=dveditz for aurora and beta

Please land soon so we can start testing for regressions.

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1128da4be7b3

Comment 8

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/393498c26e3c
https://hg.mozilla.org/releases/mozilla-beta/rev/b58f505f18df

I'm adding leave-open as well since I assume that Part 2 still needs to land.

Comment 9

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/mozilla-central/rev/1128da4be7b3

Comment 10

[Christian Holler (:decoder)]

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

Comment 11

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/b58f505f18df
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2da2e7714b52

Comment 12

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/2da2e7714b52

Comment 13

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

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #8)
> I'm adding leave-open as well since I assume that Part 2 still needs to land.

Setting needinfo? from Jan about Part 2 in comment 3. Part 1 has landed on all branches.

Comment 14

[Jan de Mooij [:jandem]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #13)
> Setting needinfo? from Jan about Part 2 in comment 3. Part 1 has landed on
> all branches.

Thanks for the reminder!

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

Comment 15

[Carsten Book [:Tomcat]]

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