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

[runtime] Fix object cloning with spreads

Browse Source 
When cloning objects using spread and update properties (e.g.
obj = {...o, x: 0}), we wrongly used the setter for the update argument
if one was set.
This CL changes the behaviour such that all arguments following the
spread are treated as dynamic arguments.

Bug: chromium:1251366
Change-Id: I76a6d02606dca0faa0a256f465834d85d3df4f6f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3178969
Commit-Queue: Patrick Thier <pthier@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77079}
This commit is contained in:
Patrick Thier 2021-09-27 06:48:25 +00:00 committed by V8 LUCI CQ
parent ecfd167dd6
commit 732d09a63b
2 changed files with 100 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

166
src/interpreter/bytecode-generator.cc
View File

@ -3005,96 +3005,104 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
BuildCreateObjectLiteral(literal, flags, entry);
}
// Store computed values into the literal.
AccessorTable<ObjectLiteral::Property> accessor_table(zone());
for (; property_index < expr->properties()->length(); property_index++) {
ObjectLiteral::Property* property = expr->properties()->at(property_index);
if (property->is_computed_name()) break;
if (!clone_object_spread && property->IsCompileTimeValue()) continue;
// If we used CloneObject for the first element is spread case, we already
// copied accessors. Therefore skip the static initialization and treat all
// properties after the spread as dynamic.
// TOOD(v8:9888): Use new Define ICs instead of Set ICs in the clone object
// spread case.
if (!clone_object_spread) {
// Store computed values into the literal.
AccessorTable<ObjectLiteral::Property> accessor_table(zone());
for (; property_index < expr->properties()->length(); property_index++) {
ObjectLiteral::Property* property =
expr->properties()->at(property_index);
if (property->is_computed_name()) break;
if (property->IsCompileTimeValue()) continue;
RegisterAllocationScope inner_register_scope(this);
Literal* key = property->key()->AsLiteral();
switch (property->kind()) {
case ObjectLiteral::Property::SPREAD:
UNREACHABLE();
case ObjectLiteral::Property::CONSTANT:
case ObjectLiteral::Property::MATERIALIZED_LITERAL:
DCHECK(clone_object_spread || !property->value()->IsCompileTimeValue());
V8_FALLTHROUGH;
case ObjectLiteral::Property::COMPUTED: {
// It is safe to use [[Put]] here because the boilerplate already
// contains computed properties with an uninitialized value.
if (key->IsStringLiteral()) {
DCHECK(key->IsPropertyName());
object_literal_context_scope.SetEnteredIf(
property->value()->IsConciseMethodDefinition());
if (property->emit_store()) {
builder()->SetExpressionPosition(property->value());
VisitForAccumulatorValue(property->value());
FeedbackSlot slot = feedback_spec()->AddStoreOwnICSlot();
builder()->StoreNamedOwnProperty(literal, key->AsRawPropertyName(),
feedback_index(slot));
RegisterAllocationScope inner_register_scope(this);
Literal* key = property->key()->AsLiteral();
switch (property->kind()) {
case ObjectLiteral::Property::SPREAD:
case ObjectLiteral::Property::CONSTANT:
UNREACHABLE();
case ObjectLiteral::Property::MATERIALIZED_LITERAL:
DCHECK(!property->value()->IsCompileTimeValue());
V8_FALLTHROUGH;
case ObjectLiteral::Property::COMPUTED: {
// It is safe to use [[Put]] here because the boilerplate already
// contains computed properties with an uninitialized value.
if (key->IsStringLiteral()) {
DCHECK(key->IsPropertyName());
object_literal_context_scope.SetEnteredIf(
property->value()->IsConciseMethodDefinition());
if (property->emit_store()) {
builder()->SetExpressionPosition(property->value());
VisitForAccumulatorValue(property->value());
FeedbackSlot slot = feedback_spec()->AddStoreOwnICSlot();
builder()->StoreNamedOwnProperty(
literal, key->AsRawPropertyName(), feedback_index(slot));
} else {
builder()->SetExpressionPosition(property->value());
VisitForEffect(property->value());
}
} else {
RegisterList args = register_allocator()->NewRegisterList(3);
builder()->MoveRegister(literal, args[0]);
builder()->SetExpressionPosition(property->key());
VisitForRegisterValue(property->key(), args[1]);
object_literal_context_scope.SetEnteredIf(
property->value()->IsConciseMethodDefinition());
builder()->SetExpressionPosition(property->value());
VisitForEffect(property->value());
VisitForRegisterValue(property->value(), args[2]);
if (property->emit_store()) {
builder()->CallRuntime(Runtime::kSetKeyedProperty, args);
}
}
} else {
RegisterList args = register_allocator()->NewRegisterList(3);
break;
}
case ObjectLiteral::Property::PROTOTYPE: {
// __proto__:null is handled by CreateObjectLiteral.
if (property->IsNullPrototype()) break;
DCHECK(property->emit_store());
DCHECK(!property->NeedsSetFunctionName());
RegisterList args = register_allocator()->NewRegisterList(2);
builder()->MoveRegister(literal, args[0]);
builder()->SetExpressionPosition(property->key());
VisitForRegisterValue(property->key(), args[1]);
object_literal_context_scope.SetEnteredIf(
property->value()->IsConciseMethodDefinition());
object_literal_context_scope.SetEnteredIf(false);
builder()->SetExpressionPosition(property->value());
VisitForRegisterValue(property->value(), args[2]);
VisitForRegisterValue(property->value(), args[1]);
builder()->CallRuntime(Runtime::kInternalSetPrototype, args);
break;
}
case ObjectLiteral::Property::GETTER:
if (property->emit_store()) {
builder()->CallRuntime(Runtime::kSetKeyedProperty, args);
accessor_table.LookupOrInsert(key)->getter = property;
}
}
break;
break;
case ObjectLiteral::Property::SETTER:
if (property->emit_store()) {
accessor_table.LookupOrInsert(key)->setter = property;
}
break;
}
case ObjectLiteral::Property::PROTOTYPE: {
// __proto__:null is handled by CreateObjectLiteral.
if (property->IsNullPrototype()) break;
DCHECK(property->emit_store());
DCHECK(!property->NeedsSetFunctionName());
RegisterList args = register_allocator()->NewRegisterList(2);
builder()->MoveRegister(literal, args[0]);
object_literal_context_scope.SetEnteredIf(false);
builder()->SetExpressionPosition(property->value());
VisitForRegisterValue(property->value(), args[1]);
builder()->CallRuntime(Runtime::kInternalSetPrototype, args);
break;
}
case ObjectLiteral::Property::GETTER:
if (property->emit_store()) {
accessor_table.LookupOrInsert(key)->getter = property;
}
break;
case ObjectLiteral::Property::SETTER:
if (property->emit_store()) {
accessor_table.LookupOrInsert(key)->setter = property;
}
break;
}
}
// Define accessors, using only a single call to the runtime for each pair of
// corresponding getters and setters.
object_literal_context_scope.SetEnteredIf(true);
for (auto accessors : accessor_table.ordered_accessors()) {
RegisterAllocationScope inner_register_scope(this);
RegisterList args = register_allocator()->NewRegisterList(5);
builder()->MoveRegister(literal, args[0]);
VisitForRegisterValue(accessors.first, args[1]);
VisitLiteralAccessor(accessors.second->getter, args[2]);
VisitLiteralAccessor(accessors.second->setter, args[3]);
builder()
->LoadLiteral(Smi::FromInt(NONE))
.StoreAccumulatorInRegister(args[4])
.CallRuntime(Runtime::kDefineAccessorPropertyUnchecked, args);
// Define accessors, using only a single call to the runtime for each pair
// of corresponding getters and setters.
object_literal_context_scope.SetEnteredIf(true);
for (auto accessors : accessor_table.ordered_accessors()) {
RegisterAllocationScope inner_register_scope(this);
RegisterList args = register_allocator()->NewRegisterList(5);
builder()->MoveRegister(literal, args[0]);
VisitForRegisterValue(accessors.first, args[1]);
VisitLiteralAccessor(accessors.second->getter, args[2]);
VisitLiteralAccessor(accessors.second->setter, args[3]);
builder()
->LoadLiteral(Smi::FromInt(NONE))
.StoreAccumulatorInRegister(args[4])
.CallRuntime(Runtime::kDefineAccessorPropertyUnchecked, args);
}
}
// Object literals have two parts. The "static" part on the left contains no

13
test/mjsunit/regress/regress-crbug-1251366.js Normal file
View File

@ -0,0 +1,13 @@
// 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.
Object.defineProperty(Object.prototype, "x", {
set: function (val) {}
});
let o = {...{}, x: 3};
assertEquals(o.x, 3);
let o2 = {...{x: 1}, x: 3};
assertEquals(o2.x, 3);