8000 chore: cherry-pick ffde6ee0e4 from v8 by ppontes · Pull Request #28800 · electron/electron · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore: cherry-pick ffde6ee0e4 from v8 #28800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 26, 2021
Merged

chore: cherry-pick ffde6ee0e4 from v8 #28800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/v8/.patches
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ regexp_throw_when_length_of_text_nodes_in_alternatives_is_too.patch
cherry-pick-02f84c745fc0.patch
merged_deoptimizer_fix_bug_in_optimizedframe_summarize.patch
cherry-pick-512cd5e179f4.patch
merged_squashed_multiple_commits.patch
202 changes: 202 additions & 0 deletions patches/v8/merged_squashed_multiple_commits.patch
View file
Open in desktop
5E26
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Georg Neis <neis@chromium.org>
Date: Wed, 10 Mar 2021 09:45:36 +0100
Subject: Merged: Squashed multiple commits.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Merged: [const-tracking] Mark const field as mutable when reconfiguring
Revision: 7535b91f7cb22274de734d5da7d0324d8653d626

Merged: [const-tracking] Fix incorrect DCHECK in MapUpdater
Revision: f95db8916a731e6e5ccc0282616bc907ce06012f

BUG=chromium:1161847,chromium:1185463,v8:9233
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=​ishell@chromium.org

(cherry picked from commit 56518020bff4d0e8b82cff843c9f618c90084e42)

Change-Id: I7f46a701646e1dd67a049b2aa4ac32d05b6885f3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2748079
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/8.9@{#43}
Cr-Original-Branched-From: 16b9bbbd581c25391981aa03180b76aa60463a3e-refs/heads/8.9.255@{#1}
Cr-Original-Branched-From: d16a2a688498bd1c3e6a49edb25d8c4ca56232dc-refs/heads/master@{#72039}
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2794428
Reviewed-by: Victor-Gabriel Savu <vsavu@google.com>
Commit-Queue: Artem Sumaneev <asumaneev@google.com>
Cr-Commit-Position: refs/branch-heads/8.6@{#73}
Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.cc
index b4b158749381efcf780d5c8ba07c286be6ba6b30..047750ebbd454a5f3f1fce7bc06ac042085245a4 100644
--- a/src/objects/map-updater.cc
+++ b/src/objects/map-updater.cc
@@ -121,6 +121,41 @@ Handle<Map> MapUpdater::ReconfigureToDataField(InternalIndex descriptor,
PropertyDetails old_details =
old_descriptors_->GetDetails(modified_descriptor_);

+ // If the {descriptor} was "const" data field so far, we need to update the
+ // {old_map_} here, otherwise we could get the constants wrong, i.e.
+ //
+ // o.x = 1;
+ // change o.x's attributes to something else
+ // delete o.x;
+ // o.x = 2;
+ //
+ // could trick V8 into thinking that `o.x` is still 1 even after the second
+ // assignment.
+ // This situation is similar to what might happen with property deletion.
+ if (old_details.constness() == PropertyConstness::kConst &&
+ old_details.location() == kField &&
+ old_details.attributes() != new_attributes_) {
+ Handle<FieldType> field_type(
+ old_descriptors_->GetFieldType(modified_descriptor_), isolate_);
+ Map::GeneralizeField(isolate_, old_map_, descriptor,
+ PropertyConstness::kMutable,
+ old_details.representation(), field_type);
+ // The old_map_'s property must become mutable.
+ // Note, that the {old_map_} and {old_descriptors_} are not expected to be
+ // updated by the generalization if the map is already deprecated.
+ DCHECK_IMPLIES(
+ !old_map_->is_deprecated(),
+ PropertyConstness::kMutable ==
+ old_descriptors_->GetDetails(modified_descriptor_).constness());
+ // Although the property in the old map is marked as mutable we still
+ // treat it as constant when merging with the new path in transition tree.
+ // This is fine because up until this reconfiguration the field was
+ // known to be constant, so it's fair to proceed treating it as such
+ // during this reconfiguration session. The issue is that after the
+ // reconfiguration the original field might become mutable (see the delete
+ // example above).
+ }
+
// If property kind is not reconfigured merge the result with
// representation/field type from the old descriptor.
if (old_details.kind() == new_kind_) {
diff --git a/test/cctest/test-field-type-tracking.cc b/test/cctest/test-field-type-tracking.cc
index 740ae05c1eaf8104ad5c3c443b2e39429fc7fca5..99ffbe9bac6094c70ffda3f362ef0d0997d61fc1 100644
--- a/test/cctest/test-field-type-tracking.cc
+++ b/test/cctest/test-field-type-tracking.cc
@@ -1081,20 +1081,31 @@ void TestReconfigureDataFieldAttribute_GeneralizeField(
Handle<Code> code_field_type = CreateDummyOptimizedCode(isolate);
Handle<Code> code_field_repr = CreateDummyOptimizedCode(isolate);
Handle<Code> code_field_const = CreateDummyOptimizedCode(isolate);
- Handle<Map> field_owner(
- map->FindFieldOwner(isolate, InternalIndex(kSplitProp)), isolate);
- DependentCode::InstallDependency(isolate,
- MaybeObjectHandle::Weak(code_field_type),
- field_owner, DependentCode::kFieldTypeGroup);
- DependentCode::InstallDependency(
- isolate, MaybeObjectHandle::Weak(code_field_repr), field_owner,
- DependentCode::kFieldRepresentationGroup);
- DependentCode::InstallDependency(
- isolate, MaybeObjectHandle::Weak(code_field_const), field_owner,
- DependentCode::kFieldConstGroup);
+ Handle<Code> code_src_field_const = CreateDummyOptimizedCode(isolate);
+ {
+ Handle<Map> field_owner(
+ map->FindFieldOwner(isolate, InternalIndex(kSplitProp)), isolate);
+ DependentCode::InstallDependency(
+ isolate, MaybeObjectHandle::Weak(code_field_type), field_owner,
+ DependentCode::kFieldTypeGroup);
+ DependentCode::InstallDependency(
+ isolate, MaybeObjectHandle::Weak(code_field_repr), field_owner,
+ DependentCode::kFieldRepresentationGroup);
+ DependentCode::InstallDependency(
+ isolate, MaybeObjectHandle::Weak(code_field_const), field_owner,
+ DependentCode::kFieldConstGroup);
+ }
+ {
+ Handle<Map> field_owner(
+ map2->FindFieldOwner(isolate, InternalIndex(kSplitProp)), isolate);
+ DependentCode::InstallDependency(
+ isolate, MaybeObjectHandle::Weak(code_src_field_const), field_owner,
+ DependentCode::kFieldConstGroup);
+ }
CHECK(!code_field_type->marked_for_deoptimization());
CHECK(!code_field_repr->marked_for_deoptimization());
CHECK(!code_field_const->marked_for_deoptimization());
+ CHECK(!code_src_field_const->marked_for_deoptimization());

// Reconfigure attributes of property |kSplitProp| of |map2| to NONE, which
// should generalize representations in |map1|.
@@ -1102,10 +1113,21 @@ void TestReconfigureDataFieldAttribute_GeneralizeField(
Map::ReconfigureExistingProperty(isolate, map2, InternalIndex(kSplitProp),
kData, NONE, PropertyConstness::kConst);

- // |map2| should be left unchanged but marked unstable.
+ // |map2| should be mosly left unchanged but marked unstable and if the
+ // source property was constant it should also be transitioned to kMutable.
CHECK(!map2->is_stable());
CHECK(!map2->is_deprecated());
CHECK_NE(*map2, *new_map);
+ // If the "source" property was const then update constness expectations for
+ // "source" map and ensure the deoptimization dependency was triggered.
+ if (to.constness == PropertyConstness::kConst) {
+ expectations2.SetDataField(kSplitProp, READ_ONLY,
+ PropertyConstness::kMutable, to.representation,
+ to.type);
+ CHECK(code_src_field_const->marked_for_deoptimization());
+ } else {
+ CHECK(!code_src_field_const->marked_for_deoptimization());
+ }
CHECK(expectations2.Check(*map2));

for (int i = kSplitProp; i < kPropCount; i++) {
diff --git a/test/mjsunit/regress/regress-crbug-1161847-1.js b/test/mjsunit/regress/regress-crbug-1161847-1.js
new file mode 100644
index 0000000000000000000000000000000000000000..282d9b878718105db40fee0283d15227fb724a3a
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-1161847-1.js
@@ -0,0 +1,19 @@
+// 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.
+
+// Flags: --allow-natives-syntax
+
+function foo(first_run) {
+ let o = { x: 0 };
+ if (first_run) assertTrue(%HasOwnConstDataProperty(o, 'x'));
+ Object.defineProperty(o, 'x', { writable: false });
+ delete o.x;
+ o.x = 23;
+ if (first_run) assertFalse(%HasOwnConstDataProperty(o, 'x'));
+}
+%PrepareFunctionForOptimization(foo);
+foo(true);
+foo(false);
+%OptimizeFunctionOnNextCall(foo);
+foo(false);
diff --git a/test/mjsunit/regress/regress-crbug-1161847-2.js b/test/mjsunit/regress/regress-crbug-1161847-2.js
new file mode 100644
index 0000000000000000000000000000000000000000..ec61fee068acea0ea259164816142a01851f3669
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-1161847-2.js
@@ -0,0 +1,19 @@
+// 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.
+
+// Flags: --allow-natives-syntax
+
+function foo(first_run) {
+ let o = { x: 0 };
+ if (first_run) assertTrue(%HasOwnConstDataProperty(o, 'x'));
+ Object.defineProperty(o, 'x', { get() { return 1; }, configurable: true, enumerable: true });
+ delete o.x;
+ o.x = 23;
+ if (first_run) assertFalse(%HasOwnConstDataProperty(o, 'x'));
+}
+%PrepareFunctionForOptimization(foo);
+foo(true);
+foo(false);
+%OptimizeFunctionOnNextCall(foo);
+foo(false);
0