Closed
Bug 1190272
Opened 6 years ago
Closed 6 years ago
Assertion failure: isInt32(), at dist/include/js/Value.h or Assertion failure: isString(), at dist/include/js/Value.h or Assertion failure: isObjectOrNull(), at dist/include/js/Value.h or Assertion failure: isNumber(), at dist/include/js/Value.h
Categories
(Core :: JavaScript Engine: JIT, defect)
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox40 | --- | unaffected |
firefox41 | --- | fixed |
firefox42 | --- | fixed |
firefox-esr31 | --- | unaffected |
firefox-esr38 | --- | unaffected |
b2g-v2.0 | --- | unaffected |
b2g-v2.0M | --- | unaffected |
b2g-v2.1 | --- | unaffected |
b2g-v2.1S | --- | unaffected |
b2g-v2.2 | --- | unaffected |
b2g-v2.2r | --- | unaffected |
b2g-master | --- | fixed |
People
(Reporter: gkw, Assigned: bhackett1024)
References
Details
(4 keywords, Whiteboard: [fuzzblocker][jsbugmon:update][post-critsmash-triage][b2g-adv-main2.5-])
Attachments
(2 files, 1 obsolete file)
2.73 KB,
text/plain
|
Details | |
6.43 KB,
patch
|
jandem
:
review+
abillings
:
approval-mozilla-aurora+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
function t(f, inputs) { var x = []; for (var j = 0; j < inputs.length; ++j) { for (var k = 0; k < 33; ++k) { x.push(f()); } } } function g() {} t(g, [0]); relazifyFunctions(); function h() { return 0; } t(h, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32]); t(g, [0]); asserts js debug shell on m-c changeset afa67b6957bb with --fuzzing-safe --no-threads --baseline-eager --unboxed-arrays at Assertion failure: isInt32(), 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 afa67b6957bb autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/57dce88fc620 user: Brian Hackett date: Tue May 26 16:29:19 2015 -0600 summary: Bug 1165392, Bug 1165463 - Various unboxed array fixes and optimizations, r=jandem. This iteration took 216.055 seconds to run. Brian, is bug 1165392 or bug 1165463 a likely regressor?
Flags: needinfo?(bhackett1024)
![]() |
Reporter | |
Comment 1•6 years ago
|
||
(lldb) bt 5 * thread #1: tid = 0x205411, 0x000000010039aa8c js-dbg-64-dm-nsprBuild-darwin-afa67b6957bb`js::SetUnboxedValueNoTypeChange(JSObject*, unsigned char*, JSValueType, JS::Value const&, bool) + 52 at Value.h:1212, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x000000010039aa8c js-dbg-64-dm-nsprBuild-darwin-afa67b6957bb`js::SetUnboxedValueNoTypeChange(JSObject*, unsigned char*, JSValueType, JS::Value const&, bool) + 52 at Value.h:1212 frame #1: 0x000000010039aa58 js-dbg-64-dm-nsprBuild-darwin-afa67b6957bb`js::SetUnboxedValueNoTypeChange(unboxedObject=<unavailable>, p=<unavailable>, type=<unavailable>, v=<unavailable>, preBarrier=<unavailable>) + 696 at UnboxedObject-inl.h:60 frame #2: 0x00000001003c2594 js-dbg-64-dm-nsprBuild-darwin-afa67b6957bb`js::DenseElementResult SetOrExtendBoxedOrUnboxedDenseElementsFunctor::operator()<(JSValueType)1>() + 406 at UnboxedObject-inl.h:502 frame #3: 0x00000001003c23fe js-dbg-64-dm-nsprBuild-darwin-afa67b6957bb`js::DenseElementResult SetOrExtendBoxedOrUnboxedDenseElementsFunctor::operator(this=<unavailable>)<(JSValueType)1>() + 110 at UnboxedObject.cpp:2034 frame #4: 0x00000001003be599 js-dbg-64-dm-nsprBuild-darwin-afa67b6957bb`js::DenseElementResult js::CallBoxedOrUnboxedSpecialization<SetOrExtendBoxedOrUnboxedDenseElementsFunctor>(f=SetOrExtendBoxedOrUnboxedDenseElementsFunctor at 0x00007fff5fbfdc80, obj=<unavailable>) + 185 at UnboxedObject-inl.h:604 (lldb)
![]() |
Reporter | |
Comment 2•6 years ago
|
||
function t(f, inputs) { var x = []; for (var j = 0; j < inputs.length; ++j) { for (var k = 0; k < 33; ++k) { x.push(f()); } } } function g() {} t(g, [0]); relazifyFunctions(); function h() { return ''; } t(h, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32]); t(g, [0]); Assertion failure: isString(), at dist/include/js/Value.h function t(f, inputs) { var x = []; for (var j = 0; j < inputs.length; ++j) { for (var k = 0; k < 33; ++k) { x.push(f()); } } } function g() {} t(g, [0]); relazifyFunctions(); function h() { return []; } t(h, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32]); t(g, [0]); Assertion failure: isObjectOrNull(), at dist/include/js/Value.h I used to have a variant that asserts at Assertion failure: isNumber(), at dist/include/js/Value.h Setting [fuzzblocker] because of the many variations these testcases can produce.
![]() |
Reporter | |
Updated•6 years ago
|
Summary: Assertion failure: isInt32(), at dist/include/js/Value.h → Assertion failure: isInt32(), at dist/include/js/Value.h or Assertion failure: isString(), at dist/include/js/Value.h or Assertion failure: isObjectOrNull(), at dist/include/js/Value.h or Assertion failure: isNumber(), at dist/include/js/Value.h
![]() |
Reporter | |
Updated•6 years ago
|
Whiteboard: [jsbugmon:update] → [fuzzblocker][jsbugmon:update]
Assignee | ||
Updated•6 years ago
|
Group: javascript-core-security
Assignee | ||
Comment 4•6 years ago
|
||
The basic problem here is that we can't rely on type information for an unboxed object's group to only describe values which can be stored to the object --- if some preliminary objects are created and then collected by the GC, they will not be considered when we construct an unboxed layout for the group, but the types of their properties will still be reflected in type information. This could be fixed by checking or constraining the type information when making the unboxed layout, but this would be making an already bad design even worse. It would be best if Ion did not assume that checking the type information for an unboxed object is a substitute for making sure the value being stored can actually fit in the unboxed property. IonBuilder::storeUnboxedValue already takes care of this via type policies on the instructions it emits, but places where code generation calls MacroAssembler::storeUnboxedProperty directly and with a null failure label need to be fixed. For unboxed plain objects this happens for SetProp ICs and SetPropertyPolymorphic. Crashes in these ops are triggered (without --unboxed-arrays) with variations of the following testcase: with({}){} function Thing(a) { this.a = a; } function Other() { } new Thing(0); var arr = []; arr.push(new Other()); arr[0].a = 0; arr[0].a = ""; arr.push(new Thing("")); gc(); for (var i = 0; i < 30; i++) arr.push(new Thing("")); for (var i = 0; i < 1005; i++) { var index = i % arr.length; var v = index ? "" : index; f(index, v); } f(1, 0); print(arr[1].a); function f(i, v) { arr[i].a = v; }
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8643913 -
Flags: review?(jdemooij)
![]() |
Reporter | |
Comment 5•6 years ago
|
||
For the testcase in comment 4: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/c3714f520752 user: Brian Hackett date: Mon Feb 02 09:27:59 2015 -0700 summary: Bug 1127987 - Fix transposed parent/metadata arguments in EmptyShape::getInitialShape, r=jandem.
Updated•6 years ago
|
Attachment #8643913 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 6•6 years ago
|
||
Actually, I'm sorry but I attached an older version of the patch that addressed the Array.push issue but not the unboxed plain objects bugs.
Attachment #8643913 -
Attachment is obsolete: true
Attachment #8644415 -
Flags: review?(jdemooij)
Updated•6 years ago
|
Attachment #8644415 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 7•6 years ago
|
||
Comment on attachment 8644415 [details] [diff] [review] patch [Security approval request comment] How easily could an exploit be constructed based on the patch? not easily. 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? aurora If not all supported branches, which bug introduced the flaw? bug 1162199 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? simple How likely is this patch to cause regressions; how much testing does it need? not likely Approval Request Comment [Feature/regressing bug #]: bug 1162199 [User impact if declined]: exploitable crashes [Describe test coverage new/current, TreeHerder]: none [Risks and why]: low
Attachment #8644415 -
Flags: sec-approval?
Attachment #8644415 -
Flags: approval-mozilla-aurora?
Comment 8•6 years ago
|
||
Comment on attachment 8644415 [details] [diff] [review] patch Approvals given.
Attachment #8644415 -
Flags: sec-approval?
Attachment #8644415 -
Flags: sec-approval+
Attachment #8644415 -
Flags: approval-mozilla-aurora?
Attachment #8644415 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 9•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f95d4a32526
Comment 10•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3f95d4a32526
Status: NEW → RESOLVED
Closed: 6 years ago
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.0M:
--- → unaffected
status-b2g-v2.1:
--- → unaffected
status-b2g-v2.1S:
--- → unaffected
status-b2g-v2.2:
--- → unaffected
status-b2g-v2.2r:
--- → unaffected
status-b2g-master:
--- → fixed
status-firefox40:
--- → unaffected
status-firefox41:
--- → affected
status-firefox-esr31:
--- → unaffected
status-firefox-esr38:
--- → unaffected
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Updated•6 years ago
|
Group: javascript-core-security
Updated•6 years ago
|
Group: core-security → core-security-release
Updated•6 years ago
|
Whiteboard: [fuzzblocker][jsbugmon:update] → [fuzzblocker][jsbugmon:update][post-critsmash-triage]
Updated•5 years ago
|
Group: core-security-release
Updated•5 years ago
|
Whiteboard: [fuzzblocker][jsbugmon:update][post-critsmash-triage] → [fuzzblocker][jsbugmon:update][post-critsmash-triage][b2g-adv-main2.5-]
You need to log in
before you can comment on or make changes to this bug.
Description
•