Closed Bug 1086842 Opened 6 years ago Closed 6 years ago

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


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox33 --- wontfix
firefox34 + fixed
firefox35 + fixed
firefox36 + fixed
firefox-esr31 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed


(Reporter: gkw, Assigned: jandem)



(4 keywords, Whiteboard: [jsbugmon:update][adv-main34+][b2g-adv-main2.2-])


(3 files)

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:
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?
Flags: needinfo?(jdemooij)
Attached file 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 = '', 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
Attached patch Part 1 - FixSplinter Review
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.
Assignee: nobody → jdemooij
Flags: needinfo?(jdemooij)
Attachment #8511153 - Flags: review?(bhackett1024)
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.
Attachment #8511162 - Flags: review?(bhackett1024)
Very nice catch btw, Gary! This is bad.

Part 2 should make it much easier for the fuzzers to find those bugs.
Attachment #8511153 - Flags: review?(bhackett1024) → review+
Attachment #8511162 - Flags: review?(bhackett1024) → review+
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?

> Which older supported branches are affected by this flaw?

> 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.
Attachment #8511153 - Flags: sec-approval?
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.
Attachment #8511153 - Flags: sec-approval?
Attachment #8511153 - Flags: sec-approval+
Attachment #8511153 - Flags: approval-mozilla-beta+
Attachment #8511153 - Flags: approval-mozilla-aurora+
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 80e18ff7c7b2).
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,ignore][adv-main34+]
(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.
Flags: needinfo?(jdemooij)
(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!
Flags: needinfo?(jdemooij)
Keywords: leave-open
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [jsbugmon:update,ignore][adv-main34+] → [jsbugmon:update][adv-main34+]
Group: javascript-core-security
Whiteboard: [jsbugmon:update][adv-main34+] → [jsbugmon:update][adv-main34+][b2g-adv-main2.2-]
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.