1001547 - Assertion failure: index < tarray.length(), at vm/TypedArrayObject.cpp

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

x = ArrayBuffer(64)
valueOf = function() {
    neuter(x)
};
Uint32Array(x)[4] = this

asserts js debug shell on m-c changeset 5ecd532a167e without any CLI arguments at Assertion failure: index < tarray.length(), at vm/TypedArrayObject.cpp

My configure flags are:

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-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/58ccbf57d72a
user:        Jeff Walden
date:        Thu Mar 20 16:38:12 2014 -0700
summary:     Bug 985733 - Make assignments into typed arrays use ToNumber-style semantics.  r=sfink, r=jandem

Setting s-s by default due to this involving TypedArrays, but pending further analysis I won't be setting a rating just yet.

Waldo, is bug 985733 a likely regressor?

Comment 1

[Jeff Walden [:Waldo]]

That's...odd.  I remember being specifically aware of this potential when fixing this, and I thought sufficiently careful, yet looking at the code now, I don't actually see a place that enforces in-boundsness here.  Blah.

Fix after I get to the office, and maybe after lunch, depending.

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Test for element-setting being careful to bounds-check. NOT REVIEWED YET

Comment 3

[Jeff Walden [:Waldo]]

Attachment: Bounds-checking for dummies. NOT REVIEWED YET

Comment 4

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8412843 [details] [diff] [review]
Bounds-checking for dummies.  NOT REVIEWED YET

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

::: js/src/vm/TypedArrayObject.cpp
@@ +1978,4 @@
>  void
>  TypedArrayObject::setElement(TypedArrayObject &obj, uint32_t index, double d)
>  {
> +    if (index >= obj.length())

Please comment why this can't set an error, as you did in the test. Or rather, why it's (more or less) ok that it doesn't.

Comment 5

[Jeff Walden [:Waldo]]

sec-critical, but bug 985733 landed this cycle, so let's get 'er done now with minimal fuss.

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

I held off on the test for the moment (and would like this bug to remain hidden for now) because the neuter() usage has me somewhat paranoid that it might give malicious readers too many ideas, while we are still in a state of neutering-unsafety (bug 991981, bug 999651, many others still in flight).  :-\  Once that's all wrapped up I'll go back and land the test (probably on any branches this bugfix needs to make its way into, as well -- only aurora if I can move fast, and I dearly hope I can), and we can open up this bug.  But please -- no sooner.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 8

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a11304b51d5

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/4a11304b51d5

Comment 10

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

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