1373356 - Assertion failure: getDenseCapacity() == 0, at js/src/vm/NativeObject-inl.h:310

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

The following testcase crashes on mozilla-central revision da66c4a05fda (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

(function () {
    (function () {
        (function () {
            x = [, 0];
            x.shift();
            x.pop();
            Object.prototype.watch.call(x, "", function (){});
            Array.prototype.push.call(arguments.callee.caller.caller.arguments, 0, 0);
            x.unshift(0);
            x.unshift(0);
        })();
    })();
})();


Backtrace:

#0  js::NativeObject::extendDenseElements (extra=1, requiredCapacity=2, cx=0x7fbd4d759000, this=0x7fbd4ed00310) at js/src/vm/NativeObject-inl.h:310
#1  js::NativeObject::ensureDenseElements (this=0x7fbd4ed00310, cx=0x7fbd4d759000, index=1, extra=1) at js/src/vm/NativeObject-inl.h:374
#2  0x00000000004f9f3b in js::array_unshift (cx=0x7fbd4d759000, argc=<optimized out>, vp=<optimized out>) at js/src/jsarray.cpp:2508
#3  0x000000000054028f in js::CallJSNative (cx=cx@entry=0x7fbd4d759000, native=0x4f9ad0 <js::array_unshift(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293
#4  0x0000000000535293 in js::InternalCallOrConstruct (cx=0x7fbd4d759000, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:470
#5  0x000000000052776c in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:521
/snip

For detailed crash information, see attachment.

Comment 1

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

Attachment: Detailed Crash Information

Comment 2

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/84ac08cff362
user:        Jan de Mooij
date:        Tue Jun 06 12:16:25 2017 +0200
summary:     Bug 1364346 part 3 - Optimize Array.prototype.unshift by taking advantage of shifted elements. r=anba

Jan, is bug 1364346 a likely regressor?

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Patch

Funny bug. |watch| sparsifies dense elements which sets the capacity to 0 to ensure we don't add new dense elements without hitting the slow path. However, unshift could bypass this if elements were shifted. The fix is just to move shifted elements in NativeObject::sparsifyDenseElements.

Comment 4

[Julien Cristau [:jcristau]]

tracking as regression in 55.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8878505 [details] [diff] [review]
Patch

anba seems to be away and I'd like to get these off my plate, so forwarding to evilpie.

Comment 6

[André Bargull [:anba]]

Comment on attachment 8878505 [details] [diff] [review]
Patch

LGTM. Is it possible to trigger this assertion without using the non-standard watch() method?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d4146242f7a
Fix NativeObject::sparsifyDenseElements to discard shifted elements. r=anba

Comment 8

[Jan de Mooij [:jandem]]

(In reply to André Bargull from comment #6)
> LGTM. Is it possible to trigger this assertion without using the
> non-standard watch() method?

For some reason I thought it wasn't possible when I wrote the patch, but Object.preventExtensions will do the trick. Changed the test.

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8878505 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1364346.
[User impact if declined]: Correctness bugs.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: Small/trivial patch, fixes the test.
[String changes made/needed]: None.

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 8878505 [details] [diff] [review]
Patch

fix regression in beta55

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/99aa29fad809

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5d4146242f7a