1224710 - Assertion failure: aIndex < mLength, at dist/include/mozilla/Vector.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

// Adapted from randomly chosen test: js/src/jit-test/tests/gc/oomInParseAsmJS.js
oomTest(function() {
    eval("\
        function g() {\
            \"use asm\";\
            function f(d) {\
                d = +d;\
                print(.0 + d);\
            }\
        }\
    ")
})

asserts js debug shell on m-c changeset faf815a0fa9b with --fuzzing-safe --no-threads --no-ion --no-baseline --ion-check-range-analysis at Assertion failure: aIndex < mLength, at dist/include/mozilla/Vector.h

Configure options:

LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -msse2 -mfpmath=sse -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -msse2 -mfpmath=sse" AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 HOST_CXX="clang++ -Qunused-arguments -msse2 -mfpmath=sse" sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-debug --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic --32" -r faf815a0fa9b

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/9c365490d4ce
user:        Jon Coppeard
date:        Tue Oct 13 13:37:07 2015 +0100
summary:     Bug 1212469 - Make oomTest() into a shell function r=nbp

Jon, is bug 1212469 a likely regressor? (I tried removing oomTest but it did not seem to reproduce)

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0xc235a, 0x0051f67b js-dbg-32-dm-darwin-faf815a0fa9b`mozilla::VectorBase<js::jit::MacroAssemblerX86::Double, 0ul, js::SystemAllocPolicy, js::Vector<js::jit::MacroAssemblerX86::Double, 0ul, js::SystemAllocPolicy> >::operator[](this=<unavailable>, aIndex=<unavailable>) + 187 at Vector.h:438, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0051f67b js-dbg-32-dm-darwin-faf815a0fa9b`mozilla::VectorBase<js::jit::MacroAssemblerX86::Double, 0ul, js::SystemAllocPolicy, js::Vector<js::jit::MacroAssemblerX86::Double, 0ul, js::SystemAllocPolicy> >::operator[](this=<unavailable>, aIndex=<unavailable>) + 187 at Vector.h:438
    frame #1: 0x00502375 js-dbg-32-dm-darwin-faf815a0fa9b`js::jit::MacroAssemblerX86::getDouble(this=<unavailable>, d=<unavailable>) + 261 at MacroAssembler-x86.cpp:112
    frame #2: 0x00502419 js-dbg-32-dm-darwin-faf815a0fa9b`js::jit::MacroAssemblerX86::loadConstantDouble(this=0x02ca7000, d=<unavailable>, dest=(reg_ = xmm0, type_ = Double, isInvalid_ = false)) + 57 at MacroAssembler-x86.cpp:122
    frame #3: 0x001f4de5 js-dbg-32-dm-darwin-faf815a0fa9b`js::jit::CodeGenerator::emitAssertRangeD(this=<unavailable>, r=<unavailable>, input=(reg_ = xmm1, type_ = Double, isInvalid_ = false), temp=(reg_ = xmm0, type_ = Double, isInvalid_ = false)) + 885 at CodeGenerator.cpp:9899
    frame #4: 0x001f5855 js-dbg-32-dm-darwin-faf815a0fa9b`js::jit::CodeGenerator::visitAssertRangeD(this=<unavailable>, ins=<unavailable>) + 165 at CodeGenerator.cpp:9981
(lldb)

Comment 2

[Jon Coppeard (:jonco)]

Attachment: bug-1224710-get-double-oom

MacroAssemblerX86::getDouble() can fail to append the double to the vector but successfully add the index to the map.  Fixed for that method and the same issue for float/simd data.  This already got fixed for x64.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8687969 [details] [diff] [review]
bug-1224710-get-double-oom

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

Stealing the review, as it's subsumed by bug 1223355 I think. r=me for landing the test though! Thanks.

Comment 4

[Jon Coppeard (:jonco)]

(In reply to Benjamin Bouvier [:bbouvier] from comment #3)
Great, nice to see the common code factored out too.

Comment 5

[Pulsebot]

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

Comment 6

[Jon Coppeard (:jonco)]

Backed out for SM(arm) failures on Linux.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ab0e8525e0c

Comment 7

[Jon Coppeard (:jonco)]

Attachment: bug1224710-fix-oom-handling

Adding this test found some more unrelated OOM handling issues.  Here's a fix for these.  It looks like we already have the equivalent changes for X86.

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8690081 [details] [diff] [review]
bug1224710-fix-oom-handling

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

Indeed, thank you for the patch.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b3a2844b7569
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fdfc96fd7bf

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/b3a2844b7569
https://hg.mozilla.org/mozilla-central/rev/1fdfc96fd7bf