If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 55

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: gkw, Assigned: jandem)

Tracking

(Blocks: 1 bug, {assertion, jsbugmon, testcase})

Trunk
mozilla56
x86_64
Linux
assertion, jsbugmon, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox-esr52 unaffected, firefox54 unaffected, firefox55+ fixed, firefox56+ fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

3 months ago
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.
(Reporter)

Comment 1

3 months ago
Created attachment 8878136 [details]
Detailed Crash Information
(Reporter)

Comment 2

3 months ago
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?
Blocks: 1364346
Flags: needinfo?(jdemooij)
(Assignee)

Comment 3

3 months ago
Created attachment 8878505 [details] [diff] [review]
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.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8878505 - Flags: review?(andrebargull)
(Assignee)

Updated

3 months ago
status-firefox54: --- → unaffected
status-firefox55: --- → affected
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → unaffected
tracking-firefox55: --- → ?
tracking-firefox56: --- → ?
tracking as regression in 55.
tracking-firefox55: ? → +
tracking-firefox56: ? → +
(Assignee)

Comment 5

3 months ago
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.
Attachment #8878505 - Flags: review?(andrebargull) → review?(evilpies)

Comment 6

3 months ago
Comment on attachment 8878505 [details] [diff] [review]
Patch

LGTM. Is it possible to trigger this assertion without using the non-standard watch() method?
Attachment #8878505 - Flags: review?(evilpies) → review+

Comment 7

3 months ago
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d4146242f7a
Fix NativeObject::sparsifyDenseElements to discard shifted elements. r=anba
(Assignee)

Comment 8

3 months ago
(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.
(Assignee)

Comment 9

3 months ago
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.
Attachment #8878505 - Flags: approval-mozilla-beta?
Comment on attachment 8878505 [details] [diff] [review]
Patch

fix regression in beta55
Attachment #8878505 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 11

3 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/99aa29fad809
status-firefox55: affected → fixed
Flags: in-testsuite+

Comment 12

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5d4146242f7a
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.