Closed Bug 952810 Opened 7 years ago Closed 7 years ago

Assertion failure: i < Length, at dist/include/mozilla/Array.h

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: gkw, Assigned: dougc)

References

Details

(Keywords: assertion, regression, testcase)

Attachments

(3 files)

Attached file testcase
The attached testcase asserts js debug shell on m-c changeset 599100c4ebfe with --ion-parallel-compile=off --ion-check-range-analysis --ion-eager at Assertion failure: i < Length, at dist/include/mozilla/Array.h

My configure flags are:

CC="gcc -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" AR=ar CXX="g++ -mfloat-abi=softfp -B/usr/lib/gcc/arm-linux-gnueabi/4.7" sh ./configure --target=arm-linux-gnueabi --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>
Good find, thank you.  It looks like a corner case, when the slice is full, and it appears appropriate to have isNextBranch return false in this case.  It appears to be a long standing bug that has been detected thanks to the changes in bug 948241.
Attachment #8350996 - Flags: review?(mrosenberg)
The patched function, isNextBranch, appears to have been removed in the patch of bug 760642 so this bug might be addressed by the changes in bug 760642.
Depends on: 760642
Comment on attachment 8350996 [details] [diff] [review]
Fix a corner case, out of bounds array reference, in isNextBranch.

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

::: js/src/jit/shared/IonAssemblerBufferWithConstantPools.h
@@ +220,5 @@
>          isBranch[idx >> 3] |= 1 << (idx & 0x7);
>      }
>      bool isNextBranch() {
> +        unsigned int size = this->nodeSize;
> +        if (size == InstBaseSize || size >= SliceSize)

How does this ever come up? size should always be < SliceSize.
Attachment #8350996 - Flags: review?(mrosenberg) → review+
(In reply to Marty Rosenberg [:mjrosenb] from comment #4)
> Comment on attachment 8350996 [details] [diff] [review]
> Fix a corner case, out of bounds array reference, in isNextBranch.
...
> >      bool isNextBranch() {
> > +        unsigned int size = this->nodeSize;
> > +        if (size == InstBaseSize || size >= SliceSize)
> 
> How does this ever come up? size should always be < SliceSize.

The nodeSize was growing to equal the SliceSize, and then a call to isNextBranch() was resulting in an out-of-bounds index into isBranch.  The code does appear to support the nodeSize growing to equal the SliceSize, see ensureSpace().  Can you see any other potential problems?
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a92abc01bd9

Can we land the testcase too please?
Assignee: nobody → dtc-moz
Flags: in-testsuite?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7a92abc01bd9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Keywords: verifyme
Adding keyword for verification in case the testcase doesn't land in Firefox 29 anymore. Please remove the keyword if it does.
It seems this only reproduces on ARM, so I'm removing the keyword...
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.