Fix map updater for non-extensible maps with private symbols.

Bailout from map update if there are private symbol transitions on
non-extensible maps.

Bug: chromium:930045
Change-Id: I02fbea0ec0afde07cded688c06122d8f2bb25921
Reviewed-on: https://chromium-review.googlesource.com/c/1460949Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59480}

src/map-updater.cc

@@ -253,12 +253,28 @@ MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() {
   return state_;  // Done.
 }
 
-void MapUpdater::SaveIntegrityLevelTransitions() {
-  integrity_source_map_ =
-      handle(Map::cast(old_map_->GetBackPointer()), isolate_);
-  ReadOnlyRoots roots(isolate_);
-  TransitionsAccessor transitions(isolate_, integrity_source_map_);
+bool MapUpdater::TrySaveIntegrityLevelTransitions() {
+  // Skip integrity level transitions.
+  integrity_source_map_ = old_map_;
+  while (!integrity_source_map_->is_extensible()) {
+    integrity_source_map_ =
+        handle(Map::cast(integrity_source_map_->GetBackPointer()), isolate_);
+  }
+
+  // If there are some non-integrity-level transitions after the first
+  // non-extensible transitions, e.g., if there were private symbols transitions
+  // after the first integrity level transition, then we just bail out and
+  // generalize all the fields.
+  if (old_map_->NumberOfOwnDescriptors() !=
+      integrity_source_map_->NumberOfOwnDescriptors()) {
+    return false;
+  }
 
+  // Figure out the most restrictive integrity level transition (it should
+  // be the last one in the transition tree).
+  ReadOnlyRoots roots(isolate_);
+  TransitionsAccessor transitions(
+      isolate_, handle(Map::cast(old_map_->GetBackPointer()), isolate_));
   if (transitions.SearchSpecial(roots.frozen_symbol()) == *old_map_) {
     integrity_level_ = FROZEN;
     integrity_level_symbol_ = isolate_->factory()->frozen_symbol();
@@ -272,16 +288,10 @@ void MapUpdater::SaveIntegrityLevelTransitions() {
     integrity_level_symbol_ = isolate_->factory()->nonextensible_symbol();
   }
 
-  // Skip all the other integrity level transitions.
-  while (!integrity_source_map_->is_extensible()) {
-    integrity_source_map_ =
-        handle(Map::cast(integrity_source_map_->GetBackPointer()), isolate_);
-  }
-
   has_integrity_level_transition_ = true;
-
   old_descriptors_ =
       handle(integrity_source_map_->instance_descriptors(), isolate_);
+  return true;
 }
 
 MapUpdater::State MapUpdater::FindRootMap() {
@@ -307,7 +317,9 @@ MapUpdater::State MapUpdater::FindRootMap() {
     DCHECK(root_map_->is_extensible());
     // We have an integrity level transition in the tree, let us make a note
     // of that transition to be able to replay it later.
-    SaveIntegrityLevelTransitions();
+    if (!TrySaveIntegrityLevelTransitions()) {
+      return CopyGeneralizeAllFields("GenAll_PrivateSymbolsOnNonExtensible");
+    }
 
     // We want to build transitions to the original element kind (before
     // the seal transitions), so change {to_kind} accordingly.

src/map-updater.h

@@ -163,7 +163,7 @@ class MapUpdater {
                        Representation new_representation,
                        Handle<FieldType> new_field_type);
 
-  void SaveIntegrityLevelTransitions();
+  bool TrySaveIntegrityLevelTransitions();
 
   Isolate* isolate_;
   Handle<Map> old_map_;

src/objects/map.cc

@@ -939,10 +939,24 @@ IntegrityLevelTransitionInfo DetectIntegrityLevelTransitions(
   IntegrityLevelTransitionInfo info(map);
 
   DCHECK(!map->is_extensible());
-  Map source_map = Map::cast(map->GetBackPointer());
+  Map source_map = map;
+  // Skip integrity level transitions.
+  while (!source_map->is_extensible()) {
+    source_map = Map::cast(source_map->GetBackPointer());
+  }
+
+  // If there are some non-integrity-level transitions after the first
+  // non-extensible transitions, e.g., if there were private symbols transitions
+  // after the first integrity level transition, then we just bail out.
+  if (map->NumberOfOwnDescriptors() != source_map->NumberOfOwnDescriptors()) {
+    return info;
+  }
 
+  // Figure out the most restrictive integrity level transition (it should
+  // be the last one in the transition tree).
   ReadOnlyRoots roots(isolate);
-  TransitionsAccessor transitions(isolate, source_map, no_allocation);
+  TransitionsAccessor transitions(isolate, Map::cast(map->GetBackPointer()),
+                                  no_allocation);
   if (transitions.SearchSpecial(roots.frozen_symbol()) == map) {
     info.integrity_level = FROZEN;
     info.integrity_level_symbol = roots.frozen_symbol();
@@ -955,11 +969,6 @@ IntegrityLevelTransitionInfo DetectIntegrityLevelTransitions(
     info.integrity_level_symbol = roots.nonextensible_symbol();
   }
 
-  // Skip all the other integrity level transitions.
-  while (!source_map->is_extensible()) {
-    source_map = Map::cast(source_map->GetBackPointer());
-  }
-
   info.has_integrity_level_transition = true;
   info.integrity_level_source_map = source_map;
 
@@ -994,6 +1003,9 @@ Map Map::TryUpdateSlow(Isolate* isolate, Map old_map) {
     DCHECK(!old_map->is_extensible());
     DCHECK(root_map->is_extensible());
     info = DetectIntegrityLevelTransitions(old_map, isolate, &no_allocation);
+    // Bail out if there were some private symbol transitions mixed up
+    // with the integrity level transitions.
+    if (!info.has_integrity_level_transition) return Map();
     // Make sure replay the original elements kind transitions, before
     // the integrity level transition sets the elements to dictionary mode.
     DCHECK(to_kind == DICTIONARY_ELEMENTS ||

test/mjsunit/regress-930045.js

+// Copyright 2019 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: --harmony-private-fields
+
+(function CaptureStackTracePrivateSymbol() {
+  var o = {};
+  Object.preventExtensions(o);
+
+  try { Error.captureStackTrace(o); } catch (e) {}
+  try { Error.captureStackTrace(o); } catch (e) {}
+})();
+
+(function PrivateFieldAfterPreventExtensions() {
+  class C {
+    constructor() {
+      this.x = 1;
+      Object.preventExtensions(this);
+    }
+  }
+
+  class D extends C {
+    #i = 42;
+
+    set(i) { this.#i = i; }
+    get(i) { return this.#i; }
+  }
+
+  let d = new D();
+  d.x = 0.1;
+  assertEquals(42, d.get());
+  d.set(43);
+  assertEquals(43, d.get());
+})();