[map transitions] Fix setting writable=false for sealed objects

Also fixes existing tests which were asserting the wrong behavior (that
setting writable=false won't have an effect).

The bug was introduced by https://chromium-review.googlesource.com/c/v8/v8/+/1442640 .

Bug: chromium:1158138
Change-Id: I2d85721848eb4e7d530a980a9ecef7f8693bb9a2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2691050
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72948}

src/objects/map-updater.cc

@@ -46,11 +46,15 @@ PropertyDetails MapUpdater::GetDetails(InternalIndex descriptor) const {
   DCHECK(descriptor.is_found());
   if (descriptor == modified_descriptor_) {
     PropertyAttributes attributes = new_attributes_;
-    // If the original map was sealed or frozen, let us used the old
+    // If the original map was sealed or frozen, let's use the old
     // attributes so that we follow the same transition path as before.
     // Note that the user could not have changed the attributes because
-    // both seal and freeze make the properties non-configurable.
-    if (integrity_level_ == SEALED || integrity_level_ == FROZEN) {
+    // both seal and freeze make the properties non-configurable. An exception
+    // is transitioning from [[Writable]] = true to [[Writable]] = false (this
+    // is allowed for frozen and sealed objects). To support it, we use the new
+    // attributes if they have [[Writable]] == false.
+    if ((integrity_level_ == SEALED || integrity_level_ == FROZEN) &&
+        !(new_attributes_ & READ_ONLY)) {
       attributes = old_descriptors_->GetDetails(descriptor).attributes();
     }
     return PropertyDetails(new_kind_, attributes, new_location_, new_constness_,

test/mjsunit/object-seal.js

@@ -520,7 +520,7 @@ assertDoesNotThrow(function() {
   });
 });
 obj.propertyA = 42;
-assertEquals(obj.propertyA, 42);
+assertEquals(obj, obj.propertyA);
 assertThrows(function() {
   Object.defineProperty(obj, 'abc', {
     value: obj,
@@ -683,7 +683,7 @@ assertDoesNotThrow(function() {
   });
 });
 obj.propertyA = 42;
-assertEquals(obj.propertyA, 42);
+assertEquals(obj, obj.propertyA);
 assertThrows(function() {
   Object.defineProperty(obj, 'abc', {
     value: obj,
@@ -967,7 +967,7 @@ assertDoesNotThrow(function() {
   });
 });
 obj.propertyA = 42;
-assertEquals(obj.propertyA, 42);
+assertEquals(obj, obj.propertyA);
 assertThrows(function() {
   Object.defineProperty(obj, 'abc', {
     value: obj,
@@ -1121,7 +1121,7 @@ assertDoesNotThrow(function() {
   });
 });
 obj.propertyA = 42;
-assertEquals(obj.propertyA, 42);
+assertEquals(obj, obj.propertyA);
 assertThrows(function() {
   Object.defineProperty(obj, 'abc', {
     value: obj,

test/mjsunit/regress/regress-crbug-1158138.js

+// 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 a = { foo: 4 };
+Object.seal(a);
+assertTrue(Object.getOwnPropertyDescriptor(a, 'foo').writable);
+Object.defineProperty(a, 'foo', { writable: false });
+assertFalse(Object.getOwnPropertyDescriptor(a, 'foo').writable);