8000 deps: V8: backport 3e010af · nodejs/node@f2abe7b · GitHub
[go: up one dir, main page]
Skip to content

Commit f2abe7b

Browse files
BridgeARtargos
BridgeAR
authored and
targos
committed
deps: V8: backport 3e010af
Original commit message: [CloneObjectIC] clone MutableHeapNumbers only if !FLAG_unbox_double_fields Change the macros added in bf84766a2cd3e09070adcd6228a3a487c8dc4bbd to only do the hard work if FLAG_unbox_double_fields is unset (otherwise, they will attempt to dereference raw float64s, which is bad!) Also adds a write barrier in CopyPropertyArrayValues for each store if it's possible that a MutableHeapNumber is cloned. BUG=chromium:901301, chromium:902965, chromium:903070, v8:7611 R=cbruni@chromium.org, jkummerow@chromium.org, ishell@chromium.org Change-Id: I224d3c4e7b0a887684bff68985b4d97021ba4cfb Reviewed-on: https://chromium-review.googlesource.com/c/1323911 Commit-Queue: Caitlin Potter <caitp@igalia.com> Reviewed-by: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#57368} PR-URL: #25101 Refs: v8/v8@3e010af Fixes: #25089 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Yang Guo <yangguo@chromium.org>
1 parent 201cf97 commit f2abe7b

File tree

6 files changed

+77
-22
lines changed

6 files changed

+77
-22
lines changed

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030

3131
# Reset this number to 0 on major V8 upgrades.
3232
# Increment by one for each non-official patch applied to deps/v8.
33-
'v8_embedder_string': '-node.14',
33+
'v8_embedder_string': '-node.15',
3434

3535
# Enable disassembler for `--print-code` v8 options
3636
'v8_enable_disassembler': 1,

‎deps/v8/src/builtins/builtins-constructor-gen.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,7 @@ Node* ConstructorBuiltinsAssembler::EmitCreateShallowObjectLiteral(
525525
VARIABLE(offset, MachineType::PointerRepresentation(),
526526
IntPtrConstant(JSObject::kHeaderSize));
527527
// Mutable heap numbers only occur on 32-bit platforms.
528-
bool may_use_mutable_heap_numbers =
529-
FLAG_track_double_fields && !FLAG_unbox_double_fields;
528+
bool may_use_mutable_heap_numbers = !FLAG_unbox_double_fields;
530529
{
531530
Comment("Copy in-object properties fast");
532531
Label continue_fast(this, &offset);

‎deps/v8/src/code-stub-assembler.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4432,6 +4432,13 @@ void CodeStubAssembler::CopyPropertyArrayValues(Node* from_array,
44324432
Comment("[ CopyPropertyArrayValues");
44334433

44344434
bool needs_write_barrier = barrier_mode == UPDATE_WRITE_BARRIER;
4435+
4436+
if (destroy_source == DestroySource::kNo) {
4437+
// PropertyArray may contain MutableHeapNumbers, which will be cloned on the
4438+
// heap, requiring a write barrier.
4439+
needs_write_barrier = true;
4440+
}
4441+
44354442
Node* start = IntPtrOrSmiConstant(0, mode);
44364443
ElementsKind kind = PACKED_ELEMENTS;
44374444
BuildFastFixedArrayForEach(

‎deps/v8/src/ic/accessor-assembler.cc‎

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3417,7 +3417,7 @@ void AccessorAssembler::GenerateStoreInArrayLiteralIC() {
34173417

34183418
void AccessorAssembler::GenerateCloneObjectIC() {
34193419
typedef CloneObjectWithVectorDescriptor Descriptor;
3420-
Node* source = Parameter(Descriptor::kSource);
3420+
TNode<HeapObject> source = CAST(Parameter(Descriptor::kSource));
34213421
Node* flags = Parameter(Descriptor::kFlags);
34223422
Node* slot = Parameter(Descriptor::kSlot);
34233423
Node* vector = Parameter(Descriptor::kVector);
@@ -3427,8 +3427,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
34273427
Label miss(this, Label::kDeferred), try_polymorphic(this, Label::kDeferred),
34283428
try_megamorphic(this, Label::kDeferred);
34293429

3430-
CSA_SLOW_ASSERT(this, TaggedIsNotSmi(source));
3431-
Node* source_map = LoadMap(UncheckedCast<HeapObject>(source));
3430+
TNode<Map> source_map = LoadMap(UncheckedCast<HeapObject>(source));
34323431
GotoIf(IsDeprecatedMap(source_map), &miss);
34333432
TNode<MaybeObject> feedback = TryMonomorphicCase(
34343433
slot, vector, source_map, &if_handler, &var_handler, &try_polymorphic);
@@ -3449,7 +3448,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
34493448

34503449
// The IC fast case should only be taken if the result map a compatible
34513450
// elements kind with the source object.
3452-
TNode<FixedArrayBase> source_elements = LoadElements(source);
3451+
TNode<FixedArrayBase> source_elements = LoadElements(CAST(source));
34533452

34543453
auto flags = ExtractFixedArrayFlag::kAllFixedArraysDontCopyCOW;
34553454
var_elements = CAST(CloneFixedArray(source_elements, flags));
@@ -3484,22 +3483,45 @@ void AccessorAssembler::GenerateCloneObjectIC() {
34843483
// Lastly, clone any in-object properties.
34853484
// Determine the inobject property capacity of both objects, and copy the
34863485
// smaller number into the resulting object.
3487-
Node* source_start = LoadMapInobjectPropertiesStartInWords(source_map);
3488-
Node* source_size = LoadMapInstanceSizeInWords(source_map);
3489-
Node* result_start = LoadMapInobjectPropertiesStartInWords(result_map);
3490-
Node* field_offset_difference =
3486+
TNode<IntPtrT> source_start =
3487+
LoadMapInobjectPropertiesStartInWords(source_map);
3488+
TNode<IntPtrT> source_size = LoadMapInstanceSizeInWords(source_map);
3489+
TNode<IntPtrT> result_start =
3490+
LoadMapInobjectPropertiesStartInWords(result_map);
3491+
TNode<IntPtrT> field_offset_difference =
34913492
TimesPointerSize(IntPtrSub(result_start, source_start));
3492-
BuildFastLoop(source_start, source_size,
3493-
[=](Node* field_index) {
3494-
Node* field_offset = TimesPointerSize(field_index);
3495-
TNode<Object> field = LoadObjectField(source, field_offset);
3496-
field = CloneIfMutablePrimitive(field);
3497-
Node* result_offset =
3498-
IntPtrAdd(field_offset, field_offset_difference);
3499-
StoreObjectFieldNoWriteBarrier(object, result_offset,
3500-
field);
3501-
},
3502-
1, INTPTR_PARAMETERS, IndexAdvanceMode::kPost);
3493+
3494+
// If MutableHeapNumbers may be present in-object, allocations may occur
3495+
// within this loop, thus the write barrier is required.
3496+
//
3497+
// TODO(caitp): skip the write barrier until the first MutableHeapNumber
3498+
// field is found
3499+
const bool may_use_mutable_heap_numbers = !FLAG_unbox_double_fields;
3500+
3501+
BuildFastLoop(
3502+
source_start, source_size,
3503+
[=](Node* field_index) {
3504+
TNode<IntPtrT> field_offset =
3505+
TimesPointerSize(UncheckedCast<IntPtrT>(field_index));
3506+
3507+
if (may_use_mutable_heap_numbers) {
3508+
TNode<Object> field = LoadObjectField(source, field_offset);
3509+
field = CloneIfMutablePrimitive(field);
3510+
TNode<IntPtrT> result_offset =
3511+
IntPtrAdd(field_offset, field_offset_difference);
3512+
StoreObjectField(object, result_offset, field);
3513+
} else {
3514+
// Copy fields as raw data.
3515+
TNode<IntPtrT> field = UncheckedCast<IntPtrT>(
3516+
LoadObjectField(source, field_offset, MachineType::IntPtr()));
3517+
TNode<IntPtrT> result_offset =
3518+
IntPtrAdd(field_offset, field_offset_difference);
3519+
StoreObjectFieldNoWriteBarrier(
3520+
object, result_offset, field,
3521+
MachineType::IntPtr().representation());
3522+
}
3523+
},
3524+
1, INTPTR_PARAMETERS, IndexAdvanceMode::kPost);
35033525
Return(object);
35043526
}
35053527

‎deps/v8/test/mjsunit/es9/regress/regress-902965.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright 2018 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Previously, spreading in-object properties would always treat double fields
6+
// as tagged, potentially dereferencing a Float64.
7+
function inobjectDouble() {
8+
"use strict";
9+
this.x = -3.9;
10+
}
11+
const instance = new inobjectDouble();
12+
const clone = { ...instance, };

‎deps/v8/test/mjsunit/es9/regress/regress-903070.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2018 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
function clone(src) {
6+
return { ...src };
7+
}
8+
9+
function inobjectDoubles() {
10+
"use strict";
11+
this.p0 = -6400510997704731;
12+
}
13+
14+
// Check that unboxed double is not treated as tagged
15+
assertEquals({ p0: -6400510997704731 }, clone(new inobjectDoubles()));

0 commit comments

Comments
 (0)
0