AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[csa] Fix an old spec violation in Array.length writes

Browse Source 
We used to apply an invalid optimization which skips `length` writes
if the JSArray is 'fast' and the old value equals the new value. This
optimization is not valid if e.g. `length` is non-writable.

Fixed: chromium:1262478
Change-Id: I49ef50de293dae5c3a62c64b303ec34b9c0f6cbc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3236720
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77552}
This commit is contained in:
Jakob Gruber 2021-10-25 08:49:25 +02:00 committed by V8 LUCI CQ
parent 8678fc629d
commit a6a113c6d5
2 changed files with 10 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/codegen/code-stub-assembler.cc
View File

@ -14726,42 +14726,8 @@ TNode<JSArray> CodeStubAssembler::ArrayCreate(TNode<Context> context,
void CodeStubAssembler::SetPropertyLength(TNode<Context> context,
TNode<Object> array,
TNode<Number> length) {
Label fast(this), runtime(this), done(this);
// There's no need to set the length, if
// 1) the array is a fast JS array and
// 2) the new length is equal to the old length.
// as the set is not observable. Otherwise fall back to the run-time.
// 1) Check that the array has fast elements.
// TODO(delphick): Consider changing this since it does an an unnecessary
// check for SMIs.
// TODO(delphick): Also we could hoist this to after the array construction
// and copy the args into array in the same way as the Array constructor.
BranchIfFastJSArray(array, context, &fast, &runtime);
BIND(&fast);
{
TNode<JSArray> fast_array = CAST(array);
TNode<Smi> length_smi = CAST(length);
TNode<Smi> old_length = LoadFastJSArrayLength(fast_array);
CSA_DCHECK(this, TaggedIsPositiveSmi(old_length));
// 2) If the created array's length matches the required length, then
// there's nothing else to do. Otherwise use the runtime to set the
// property as that will insert holes into excess elements or shrink
// the backing store as appropriate.
Branch(SmiNotEqual(length_smi, old_length), &runtime, &done);
}
BIND(&runtime);
{
SetPropertyStrict(context, array, CodeStubAssembler::LengthStringConstant(),
length);
Goto(&done);
}
BIND(&done);
SetPropertyStrict(context, array, CodeStubAssembler::LengthStringConstant(),
length);
}
TNode<Smi> CodeStubAssembler::RefillMathRandom(

8
test/mjsunit/regress/regress-1262478.js Normal file
View File

@ -0,0 +1,8 @@
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
let __v_0 = [];
Object.defineProperty(__v_0, 'length', { writable: false });
function __f_5() { return __v_0; }
assertThrows(() => Array.of.call(__f_5), TypeError);