Skip to content

Commit

Permalink
deps: V8: backport 3e010af
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
BridgeAR authored and targos committed Jan 1, 2019
1 parent 201cf97 commit f2abe7b
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 22 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

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

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
3 changes: 1 addition & 2 deletions deps/v8/src/builtins/builtins-constructor-gen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,7 @@ Node* ConstructorBuiltinsAssembler::EmitCreateShallowObjectLiteral(
VARIABLE(offset, MachineType::PointerRepresentation(),
IntPtrConstant(JSObject::kHeaderSize));
// Mutable heap numbers only occur on 32-bit platforms.
bool may_use_mutable_heap_numbers =
FLAG_track_double_fields && !FLAG_unbox_double_fields;
bool may_use_mutable_heap_numbers = !FLAG_unbox_double_fields;
{
Comment("Copy in-object properties fast");
Label continue_fast(this, &offset);
Expand Down
7 changes: 7 additions & 0 deletions deps/v8/src/code-stub-assembler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4432,6 +4432,13 @@ void CodeStubAssembler::CopyPropertyArrayValues(Node* from_array,
Comment("[ CopyPropertyArrayValues");

bool needs_write_barrier = barrier_mode == UPDATE_WRITE_BARRIER;

if (destroy_source == DestroySource::kNo) {
// PropertyArray may contain MutableHeapNumbers, which will be cloned on the
// heap, requiring a write barrier.
needs_write_barrier = true;
}

Node* start = IntPtrOrSmiConstant(0, mode);
ElementsKind kind = PACKED_ELEMENTS;
BuildFastFixedArrayForEach(
Expand Down
60 changes: 41 additions & 19 deletions deps/v8/src/ic/accessor-assembler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3417,7 +3417,7 @@ void AccessorAssembler::GenerateStoreInArrayLiteralIC() {

void AccessorAssembler::GenerateCloneObjectIC() {
typedef CloneObjectWithVectorDescriptor Descriptor;
Node* source = Parameter(Descriptor::kSource);
TNode<HeapObject> source = CAST(Parameter(Descriptor::kSource));
Node* flags = Parameter(Descriptor::kFlags);
Node* slot = Parameter(Descriptor::kSlot);
Node* vector = Parameter(Descriptor::kVector);
Expand All @@ -3427,8 +3427,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
Label miss(this, Label::kDeferred), try_polymorphic(this, Label::kDeferred),
try_megamorphic(this, Label::kDeferred);

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

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

auto flags = ExtractFixedArrayFlag::kAllFixedArraysDontCopyCOW;
var_elements = CAST(CloneFixedArray(source_elements, flags));
Expand Down Expand Up @@ -3484,22 +3483,45 @@ void AccessorAssembler::GenerateCloneObjectIC() {
// Lastly, clone any in-object properties.
// Determine the inobject property capacity of both objects, and copy the
// smaller number into the resulting object.
Node* source_start = LoadMapInobjectPropertiesStartInWords(source_map);
Node* source_size = LoadMapInstanceSizeInWords(source_map);
Node* result_start = LoadMapInobjectPropertiesStartInWords(result_map);
Node* field_offset_difference =
TNode<IntPtrT> source_start =
LoadMapInobjectPropertiesStartInWords(source_map);
TNode<IntPtrT> source_size = LoadMapInstanceSizeInWords(source_map);
TNode<IntPtrT> result_start =
LoadMapInobjectPropertiesStartInWords(result_map);
TNode<IntPtrT> field_offset_difference =
TimesPointerSize(IntPtrSub(result_start, source_start));
BuildFastLoop(source_start, source_size,
[=](Node* field_index) {
Node* field_offset = TimesPointerSize(field_index);
TNode<Object> field = LoadObjectField(source, field_offset);
field = CloneIfMutablePrimitive(field);
Node* result_offset =
IntPtrAdd(field_offset, field_offset_difference);
StoreObjectFieldNoWriteBarrier(object, result_offset,
field);
},
1, INTPTR_PARAMETERS, IndexAdvanceMode::kPost);

// If MutableHeapNumbers may be present in-object, allocations may occur
// within this loop, thus the write barrier is required.
//
// TODO(caitp): skip the write barrier until the first MutableHeapNumber
// field is found
const bool may_use_mutable_heap_numbers = !FLAG_unbox_double_fields;

BuildFastLoop(
source_start, source_size,
[=](Node* field_index) {
TNode<IntPtrT> field_offset =
TimesPointerSize(UncheckedCast<IntPtrT>(field_index));

if (may_use_mutable_heap_numbers) {
TNode<Object> field = LoadObjectField(source, field_offset);
field = CloneIfMutablePrimitive(field);
TNode<IntPtrT> result_offset =
IntPtrAdd(field_offset, field_offset_difference);
StoreObjectField(object, result_offset, field);
} else {
// Copy fields as raw data.
TNode<IntPtrT> field = UncheckedCast<IntPtrT>(
LoadObjectField(source, field_offset, MachineType::IntPtr()));
TNode<IntPtrT> result_offset =
IntPtrAdd(field_offset, field_offset_difference);
StoreObjectFieldNoWriteBarrier(
object, result_offset, field,
MachineType::IntPtr().representation());
}
},
1, INTPTR_PARAMETERS, IndexAdvanceMode::kPost);
Return(object);
}

Expand Down
12 changes: 12 additions & 0 deletions deps/v8/test/mjsunit/es9/regress/regress-902965.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright 2018 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.

// Previously, spreading in-object properties would always treat double fields
// as tagged, potentially dereferencing a Float64.
function inobjectDouble() {
"use strict";
this.x = -3.9;
}
const instance = new inobjectDouble();
const clone = { ...instance, };
15 changes: 15 additions & 0 deletions deps/v8/test/mjsunit/es9/regress/regress-903070.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2018 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.

function clone(src) {
return { ...src };
}

function inobjectDoubles() {
"use strict";
this.p0 = -6400510997704731;
}

// Check that unboxed double is not treated as tagged
assertEquals({ p0: -6400510997704731 }, clone(new inobjectDoubles()));

0 comments on commit f2abe7b

Please sign in to comment.