Merged: [const-tracking] Ensure map is updated before generalizing constness

isheludko · isheludko · commit 5a0dd788cdae · 2021-04-16T11:04:56.000Z
Revision: db2acd7 BUG=chromium:1195331 NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=leszeks@chromium.org Change-Id: I7ce1b36b8860a49838d208bc7857021e03f83916 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2831474 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/branch-heads/9.0@{v8#37} Cr-Branched-From: bd0108b-refs/heads/9.0.257@{#1} Cr-Branched-From: 349bcc6-refs/heads/master@{#73001}
diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.cc
@@ -139,11 +139,20 @@ Handle<Map> MapUpdater::ReconfigureToDataField(InternalIndex descriptor,
   if (old_details.constness() == PropertyConstness::kConst &&
       old_details.location() == kField &&
       old_details.attributes() != new_attributes_) {
+    // Ensure we'll be updating constness of the up-to-date version of old_map_.
+    Handle<Map> old_map = Map::Update(isolate_, old_map_);
+    PropertyDetails details =
+        old_map->instance_descriptors(kRelaxedLoad).GetDetails(descriptor);
     Handle<FieldType> field_type(
-        old_descriptors_->GetFieldType(modified_descriptor_), isolate_);
-    Map::GeneralizeField(isolate_, old_map_, descriptor,
-                         PropertyConstness::kMutable,
-                         old_details.representation(), field_type);
+        old_map->instance_descriptors(kRelaxedLoad).GetFieldType(descriptor),
+        isolate_);
+    Map::GeneralizeField(isolate_, old_map, descriptor,
+                         PropertyConstness::kMutable, details.representation(),
+                         field_type);
+    DCHECK_EQ(PropertyConstness::kMutable,
+              old_map->instance_descriptors(kRelaxedLoad)
+                  .GetDetails(descriptor)
+                  .constness());
     // 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.
diff --git a/test/mjsunit/regress/regress-crbug-1195331.js b/test/mjsunit/regress/regress-crbug-1195331.js
@@ -0,0 +1,36 @@
+// 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
+
+let o1 = { a: 1, b: 0 };
+let o2 = { a: 2, b: 0 };
+assertTrue(%HaveSameMap(o1, o2));
+assertTrue(%HasOwnConstDataProperty(o1, "a"));
+assertTrue(%HasOwnConstDataProperty(o1, "b"));
+
+Object.defineProperty(o1, "b", {
+  value: 4.2, enumerable: true, configurable: true, writable: true,
+});
+assertFalse(%HaveSameMap(o1, o2));
+assertTrue(%HasOwnConstDataProperty(o1, "a"));
+assertFalse(%HasOwnConstDataProperty(o1, "b"));
+assertTrue(%HasOwnConstDataProperty(o2, "a"));
+assertTrue(%HasOwnConstDataProperty(o2, "b"));
+
+let o3 = { a: "foo", b: 0 };
+assertFalse(%HaveSameMap(o2, o3));
+assertTrue(%HasOwnConstDataProperty(o3, "a"));
+assertFalse(%HasOwnConstDataProperty(o3, "b"));
+
+Object.defineProperty(o2, "a", {
+  value:2, enumerable: false, configurable: true, writable: true,
+});
+assertFalse(%HasOwnConstDataProperty(o1, "a"));
+assertFalse(%HasOwnConstDataProperty(o1, "b"));
+assertFalse(%HasOwnConstDataProperty(o3, "a"));
+assertFalse(%HasOwnConstDataProperty(o3, "b"));
+
+assertFalse(%HasOwnConstDataProperty(o2, "a"));
+assertTrue(%HasOwnConstDataProperty(o2, "b"));
