New issue
Advanced search Search tips

Issue 734314 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

array restores its value when using arr.push/arr.unshift and modifying arr.length

Reported by yggdfol...@gmail.com, Jun 17 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3128.0 Safari/537.36

Steps to reproduce the problem:
var arr1 = [1, 2, 3];
arr1.push(4);
arr1.length = 3;
arr1.length = 4;
console.log(arr1);
var arr2 = [2, 3, 4];
arr2.unshift(1);
arr2.length = 3;
arr2.length = 4;
console.log(arr2);

What is the expected behavior?
[1, 2, 3, undefined × 1]
[1, 2, 3, undefined × 1]

What went wrong?
[1, 2, 3, 4]
[1, 2, 3, 4]

Did this work before? Yes maybe Chrome 59.0.3071.86, V8 5.9.211.31 or later

Chrome version: 61.0.3128.0  Channel: dev
OS Version: 10.0
Flash Version: Shockwave Flash 26.0 r0

This also bugs on Chrome 60.0.3107.4 Dev, V8 6.0.286
 

Comment 1 by woxxom@gmail.com, Jun 17 2017

Bisect: 472090 (good) - 472095 (bad), 60.0.3102.0
https://chromium.googlesource.com/chromium/src/+log/6a9a1ec0..961d7a5a?pretty=fuller

Suspecting r472091 "Update V8 to version 6.0.228" 
https://chromium.googlesource.com/v8/v8/+log/9b91ebdf..649a1343?pretty=fuller

In the V8 update above suspecting https://chromium-review.googlesource.com/502911 
"[runtime] avoid trim/grow loop when adding and removing one element"
Cc: cbruni@chromium.org jkummerow@chromium.org
Components: -Blink Blink>JavaScript
Labels: M-60
Owner: tebbi@chromium.org
Status: Assigned (was: Unconfirmed)
First guess: in the following code in elements.cc:

      if (2 * length + JSObject::kMinAddedElementsCapacity <= capacity) {
        // If more than half the elements won't be used, trim the array.
        // Do not trim from short arrays to prevent frequent trimming on
        // repeated pop operations.
        // Leave some space to allow for subsequent push operations.
        int elements_to_trim = length + 1 == old_length
                                   ? (capacity - length) / 2
                                   : capacity - length;
        isolate->heap()->RightTrimFixedArray(*backing_store, elements_to_trim);
      } else {
        // Otherwise, fill the unused tail with holes.
        BackingStore::cast(*backing_store)->FillWithHoles(length, old_length);
      }

when we decide *not* to trim everything, then the remaining elements must be filled with holes.

Comment 3 by tebbi@chromium.org, Jun 22 2017

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f030838700a83cde6992cb8ebcb3facc6a8fc1f1

commit f030838700a83cde6992cb8ebcb3facc6a8fc1f1
Author: Tobias Tebbi <tebbi@chromium.org>
Date: Mon Jun 26 12:02:27 2017

[runtime] clear array elements when right trimming while leaving free space

Bug:  chromium:734314 
Change-Id: I4e1bd1264c2c4088ce9fdcdbe3b9e233faa516df
Reviewed-on: https://chromium-review.googlesource.com/544990
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46211}
[modify] https://crrev.com/f030838700a83cde6992cb8ebcb3facc6a8fc1f1/src/elements.cc
[modify] https://crrev.com/f030838700a83cde6992cb8ebcb3facc6a8fc1f1/test/mjsunit/array-length.js

Comment 5 by tebbi@chromium.org, Jul 14 2017

Status: Fixed (was: Started)

Comment 6 by adamk@chromium.org, Aug 15 2017

 Issue 755538  has been merged into this issue.
 Issue 755719  has been merged into this issue.

Sign in to add a comment