Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 201cf97

Browse filesBrowse files
BridgeARtargos
BridgeAR
authored and
targos
committed
deps: V8: backport bf84766
Original commit message: [CloneObjectIC] clone MutableHeapNumbers instead of referencing them Adds a helper macro "CloneIfMutablePrimitive", which tests if the operand is a MutableHeapNumber, and if so, clones it, otherwise returning the original value. Also modifies the signature of "CopyPropertyArrayValues" to take a "DestroySource" enum, indicating whether or not the resulting object is supplanting the source object or not, and removes all default parameters from that macro (which were not used anyways). This corrects the issue reported in chromium:901301, where StaNamedOwnProperty was replacing the value of a MutableHeapNumber referenced by both the cloned object and the source object. BUG=chromium:901301, v8:7611 R=cbruni@chromium.org, jkummerow@chromium.org Change-Id: I43df1ddc84dfa4840e680b6affeba452ce0b6629 Reviewed-on: https://chromium-review.googlesource.com/c/1318096 Commit-Queue: Caitlin Potter <caitp@igalia.com> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/master@{#57304} PR-URL: #25101 Refs: v8/v8@bf84766 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 f62e35f commit 201cf97
Copy full SHA for 201cf97

File tree

Expand file treeCollapse file tree

5 files changed

+83
-10
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+83
-10
lines changed
Open diff view settings
Collapse file

‎common.gypi‎

Copy file name to clipboardExpand all lines: common.gypi
+1-1Lines 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.13',
33+
'v8_embedder_string': '-node.14',
3434

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

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

Copy file name to clipboardExpand all lines: deps/v8/src/code-stub-assembler.cc
+38-2Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2984,6 +2984,24 @@ TNode<MutableHeapNumber> CodeStubAssembler::AllocateMutableHeapNumber() {
29842984
return UncheckedCast<MutableHeapNumber>(result);
29852985
}
29862986

2987+
TNode<Object> CodeStubAssembler::CloneIfMutablePrimitive(TNode<Object> object) {
2988+
TVARIABLE(Object, result, object);
2989+
Label done(this);
2990+
2991+
GotoIf(TaggedIsSmi(object), &done);
2992+
GotoIfNot(IsMutableHeapNumber(UncheckedCast<HeapObject>(object)), &done);
2993+
{
2994+
// Mutable heap number found --- allocate a clone.
2995+
TNode<Float64T> value =
2996+
LoadHeapNumberValue(UncheckedCast<HeapNumber>(object));
2997+
result = AllocateMutableHeapNumberWithValue(value);
2998+
Goto(&done);
2999+
}
3000+
3001+
BIND(&done);
3002+
return result.value();
3003+
}
3004+
29873005
TNode<MutableHeapNumber> CodeStubAssembler::AllocateMutableHeapNumberWithValue(
29883006
SloppyTNode<Float64T> value) {
29893007
TNode<MutableHeapNumber> result = AllocateMutableHeapNumber();
@@ -4405,7 +4423,8 @@ void CodeStubAssembler::CopyPropertyArrayValues(Node* from_array,
44054423
Node* to_array,
44064424
Node* property_count,
44074425
WriteBarrierMode barrier_mode,
4408-
ParameterMode mode) {
4426+
ParameterMode mode,
4427+
DestroySource destroy_source) {
44094428
CSA_SLOW_ASSERT(this, MatchesParameterMode(property_count, mode));
44104429
CSA_SLOW_ASSERT(this, Word32Or(IsPropertyArray(from_array),
44114430
IsEmptyFixedArray(from_array)));
@@ -4417,9 +4436,14 @@ void CodeStubAssembler::CopyPropertyArrayValues(Node* from_array,
44174436
ElementsKind kind = PACKED_ELEMENTS;
44184437
BuildFastFixedArrayForEach(
44194438
from_array, kind, start, property_count,
4420-
[this, to_array, needs_write_barrier](Node* array, Node* offset) {
4439+
[this, to_array, needs_write_barrier, destroy_source](Node* array,
4440+
Node* offset) {
44214441
Node* value = Load(MachineType::AnyTagged(), array, offset);
44224442

4443+
if (destroy_source == DestroySource::kNo) {
4444+
value = CloneIfMutablePrimitive(CAST(value));
4445+
}
4446+
44234447
if (needs_write_barrier) {
44244448
Store(to_array, offset, value);
44254449
} else {
@@ -4428,6 +4452,18 @@ void CodeStubAssembler::CopyPropertyArrayValues(Node* from_array,
44284452
}
44294453
},
44304454
mode);
4455+
4456+
#ifdef DEBUG
4457+
// Zap {from_array} if the copying above has made it invalid.
4458+
if (destroy_source == DestroySource::kYes) {
4459+
Label did_zap(this);
4460+
GotoIf(IsEmptyFixedArray(from_array), &did_zap);
4461+
FillPropertyArrayWithUndefined(from_array, start, property_count, mode);
4462+
4463+
Goto(&did_zap);
4464+
BIND(&did_zap);
4465+
}
4466+
#endif
44314467
Comment("] CopyPropertyArrayValues");
44324468
}
44334469

Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/code-stub-assembler.h
+17-4Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,10 +1454,19 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
14541454
Node* to_index,
14551455
ParameterMode mode = INTPTR_PARAMETERS);
14561456

1457-
void CopyPropertyArrayValues(
1458-
Node* from_array, Node* to_array, Node* length,
1459-
WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER,
1460-
ParameterMode mode = INTPTR_PARAMETERS);
1457+
enum class DestroySource { kNo, kYes };
1458+
1459+
// Specify DestroySource::kYes if {from_array} is being supplanted by
1460+
// {to_array}. This offers a slight performance benefit by simply copying the
1461+
// array word by word. The source may be destroyed at the end of this macro.
1462+
//
1463+
// Otherwise, specify DestroySource::kNo for operations where an Object is
1464+
// being cloned, to ensure that MutableHeapNumbers are unique between the
1465+
// source and cloned object.
1466+
void CopyPropertyArrayValues(Node* from_array, Node* to_array, Node* length,
1467+
WriteBarrierMode barrier_mode,
1468+
ParameterMode mode,
1469+
DestroySource destroy_source);
14611470

14621471
// Copies all elements from |from_array| of |length| size to
14631472
// |to_array| of the same size respecting the elements kind.
@@ -2864,6 +2873,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
28642873
void InitializeFunctionContext(Node* native_context, Node* context,
28652874
int slots);
28662875

2876+
// Allocate a clone of a mutable primitive, if {object} is a
2877+
// MutableHeapNumber.
2878+
TNode<Object> CloneIfMutablePrimitive(TNode<Object> object);
2879+
28672880
private:
28682881
friend class CodeStubArguments;
28692882

Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/ic/accessor-assembler.cc
+5-3Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,7 +1667,8 @@ Node* AccessorAssembler::ExtendPropertiesBackingStore(Node* object,
16671667
// |new_properties| is guaranteed to be in new space, so we can skip
16681668
// the write barrier.
16691669
CopyPropertyArrayValues(var_properties.value(), new_properties,
1670-
var_length.value(), SKIP_WRITE_BARRIER, mode);
1670+
var_length.value(), SKIP_WRITE_BARRIER, mode,
1671+
DestroySource::kYes);
16711672

16721673
// TODO(gsathya): Clean up the type conversions by creating smarter
16731674
// helpers that do the correct op based on the mode.
@@ -3471,7 +3472,7 @@ void AccessorAssembler::GenerateCloneObjectIC() {
34713472
auto mode = INTPTR_PARAMETERS;
34723473
var_properties = CAST(AllocatePropertyArray(length, mode));
34733474
CopyPropertyArrayValues(source_properties, var_properties.value(), length,
3474-
SKIP_WRITE_BARRIER, mode);
3475+
SKIP_WRITE_BARRIER, mode, DestroySource::kNo);
34753476
}
34763477

34773478
Goto(&allocate_object);
@@ -3491,7 +3492,8 @@ void AccessorAssembler::GenerateCloneObjectIC() {
34913492
BuildFastLoop(source_start, source_size,
34923493
[=](Node* field_index) {
34933494
Node* field_offset = TimesPointerSize(field_index);
3494-
Node* field = LoadObjectField(source, field_offset);
3495+
TNode<Object> field = LoadObjectField(source, field_offset);
3496+
field = CloneIfMutablePrimitive(field);
34953497
Node* result_offset =
34963498
IntPtrAdd(field_offset, field_offset_difference);
34973499
StoreObjectFieldNoWriteBarrier(object, result_offset,
Collapse file

‎deps/v8/test/mjsunit/es9/object-spread-ic.js‎

Copy file name to clipboardExpand all lines: deps/v8/test/mjsunit/es9/object-spread-ic.js
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,25 @@
9999
// Megamorphic
100100
assertEquals({ boop: 1 }, f({ boop: 1 }));
101101
})();
102+
103+
// There are 2 paths in CloneObjectIC's handler which need to handle double
104+
// fields specially --- in object properties, and copying the property array.
105+
function testMutableInlineProperties() {
106+
function inobject() { "use strict"; this.x = 1.1; }
107+
const src = new inobject();
108+
const x0 = src.x;
109+
const clone = { ...src, x: x0 + 1 };
110+
assertEquals(x0, src.x);
111+
assertEquals({ x: 2.1 }, clone);
112+
}
113+
testMutableInlineProperties()
114+
115+
function testMutableOutOfLineProperties() {
116+
const src = { a: 1, b: 2, c: 3 };
117+
src.x = 2.3;
118+
const x0 = src.x;
119+
const clone = { ...src, x: x0 + 1 };
120+
assertEquals(x0, src.x);
121+
assertEquals({ a: 1, b: 2, c: 3, x: 3.3 }, clone);
122+
}
123+
testMutableOutOfLineProperties();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.