1120063 - Assertion failure: idx < getDenseInitializedLength(), at vm/NativeObject.h

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Object.defineProperty(wrapWithProto(new SharedInt16Array(1), {}), 0, {})

asserts js debug shell on m-c changeset bb8d6034f5f2 with --fuzzing-safe --no-threads --ion-eager at Assertion failure: idx < getDenseInitializedLength(), at vm/NativeObject.h.

Debug 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-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

Not sure if shared memory is anything scary yet, so setting s-s first.

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x3187a, 0x000000010077dc04 js-dbg-opt-64-dm-nsprBuild-darwin-bb8d6034f5f2`js::NativeObject::sparsifyDenseElement(js::ExclusiveContext*, JS::Handle<js::NativeObject*>, unsigned int) [inlined] JS::Handle<js::NativeObject*>::operator->(this=0x0000000000000000, idx=<unavailable>) const + 67 at NativeObject.h:385, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010077dc04 js-dbg-opt-64-dm-nsprBuild-darwin-bb8d6034f5f2`js::NativeObject::sparsifyDenseElement(js::ExclusiveContext*, JS::Handle<js::NativeObject*>, unsigned int) [inlined] JS::Handle<js::NativeObject*>::operator->(this=0x0000000000000000, idx=<unavailable>) const + 67 at NativeObject.h:385
    frame #1: 0x000000010077dbc1 js-dbg-opt-64-dm-nsprBuild-darwin-bb8d6034f5f2`js::NativeObject::sparsifyDenseElement(cx=<unavailable>, index=<unavailable>, obj=<unavailable>) + 657 at NativeObject.cpp:508
    frame #2: 0x000000010077a50f js-dbg-opt-64-dm-nsprBuild-darwin-bb8d6034f5f2`js::DefineNativeProperty(cx=0x0000000101e01cf0, attrs=61446, obj=<unavailable>, id=<unavailable>, value=<unavailable>)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>), bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>), unsigned int) + 1247 at NativeObject.cpp:1440
    frame #3: 0x0000000100620e15 js-dbg-opt-64-dm-nsprBuild-darwin-bb8d6034f5f2`JSObject::defineGeneric(cx=<unavailable>, getter=<unavailable>, setter=<unavailable>, attrs=<unavailable>, obj=<unavailable>, id=<unavailable>, value=<unavailable>)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>), bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool, JS::MutableHandle<JS::Value>), unsigned int) + 181 at jsobj.cpp:2902
    frame #4: 0x00000001006cab4b js-dbg-opt-64-dm-nsprBuild-darwin-bb8d6034f5f2`js::DirectProxyHandler::defineProperty(this=<unavailable>, cx=0x0000000101e01cf0, proxy=<unavailable>, id=<unavailable>, desc=<unavailable>) const + 267 at DirectProxyHandler.cpp:44
(lldb)

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/677c24618e33
user:        Lars T Hansen
date:        Fri Oct 31 13:37:11 2014 +0100
summary:     Bug 1068684 - remove asm.js length restriction on SharedArrayBuffer. r=luke

This iteration took 319.801 seconds to run.

Comment 3

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

Lars, is bug 1068684 a likely regressor?

Comment 4

[Lars T Hansen [:lth]]

The problem here is an incorrect type test and it is easily fixed.  I don't think the type test was introduced by bug 1068684, but it exposed the problem because it allowed shared arrays of length 1 to be allocated, as in this test, thus the test no longer fails with a "bad length" error.  If you were to run the bisect with an array length of 4096 you would probably find an earlier changeset that introduced the problem.

Unfortunately that part of the code has seen some large changes since the SharedTypedArray work first landed (files have been renamed and merged), so it's hard to know exactly where the incorrect type test came from.

Comment 5

[Lars T Hansen [:lth]]

Attachment: Correct the type test by checking for SharedTypedArray too

Comment 6

[Lars T Hansen [:lth]]

Incorrect type test introduced in jsobj.cpp here (this has no bearing on the fix):

changeset:   202533:721ba9525397
user:        Eric Faust <[e-mail]>
date:        Fri Aug 29 14:59:51 2014 -0700
summary:     Bug 1058869 - Don't forget about Arrays for attribute-only Object.defineProperty calls. (r=jorendorff)

Later it was merged over to vm/NativeObject.cpp where it is now.

Comment 7

[Lars T Hansen [:lth]]

Comment on attachment 8547175 [details] [diff] [review]
Correct the type test by checking for SharedTypedArray too

Redirecting r? in light of the origin of the bug.

Comment 8

[Andrew McCreight [:mccr8]]

Sounds kind of bad.  It sounds like this affects more than shared arrays?  Is that right?

Comment 9

[Eric Faust [:efaust]]

Comment on attachment 8547175 [details] [diff] [review]
Correct the type test by checking for SharedTypedArray too

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

Yep. Probably there was a push race, and I lost. Thanks for looking into this.

Comment 10

[Lars T Hansen [:lth]]

(In reply to Andrew McCreight [:mccr8] from comment #8)
> Sounds kind of bad.  It sounds like this affects more than shared arrays? 
> Is that right?

I don't think so.  The test in question checks for typed array and if it finds it it returns early.  It should also check for shared typed array and in that case also return early.

Shared typed array is disabled except on Nightly, so this bug should not have been exposed anywhere else, even if the buggy code is now probably in a release build.  (Developer edition?  I need to check if that is compiled as Nightly or with a more narrow feature set; my guess is the latter.)

Comment 11

[Lars T Hansen [:lth]]

Shared typed array is not exposed in Developer edition.  Is there any reason I can't just land this patch (with a proper commit message) on mozilla-inbound?

Comment 12

[Lars T Hansen [:lth]]

Pushed to m-i since Nightly-only:
https://hg.mozilla.org/integration/mozilla-inbound/rev/30830a78e0a6

Comment 13

[Andrew McCreight [:mccr8]]

(In reply to Lars T Hansen [:lth] from comment #11)
> Is there any reason I can't just land this patch (with a proper commit message) on
> mozilla-inbound?

Yes, if something is Nightly-only (or is not rated sec-high or sec-critical) than it does not need sec-approval before landing.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/30830a78e0a6

Comment 15

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.