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

RESOLVED FIXED in mozilla29

Status

()

defect
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gkw, Assigned: dougc)

Tracking

(Blocks 1 bug, {assertion, regression, testcase})

Trunk
mozilla29
ARM
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Reporter

Description

6 years ago
Posted 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>
Reporter

Comment 1

6 years ago
Posted file stack
Assignee

Comment 2

6 years ago
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)
Reporter

Updated

6 years ago
Blocks: 948241
Assignee

Comment 3

6 years ago
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+
Assignee

Comment 5

6 years ago
(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: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29

Updated

5 years ago
Keywords: verifyme

Comment 8

5 years ago
Adding keyword for verification in case the testcase doesn't land in Firefox 29 anymore. Please remove the keyword if it does.

Comment 9

5 years ago
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.