diff --git a/src/map-updater.cc b/src/map-updater.cc index b84cb12a57..c6209cd9d6 100644 --- a/src/map-updater.cc +++ b/src/map-updater.cc @@ -9,6 +9,7 @@ #include "src/isolate.h" #include "src/objects-inl.h" #include "src/objects.h" +#include "src/property-details.h" #include "src/transitions.h" namespace v8 { @@ -254,39 +255,41 @@ MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() { } 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(); - } else if (transitions.SearchSpecial(roots.sealed_symbol()) == *old_map_) { - integrity_level_ = SEALED; - integrity_level_symbol_ = isolate_->factory()->sealed_symbol(); - } else { - CHECK_EQ(transitions.SearchSpecial(roots.nonextensible_symbol()), - *old_map_); - integrity_level_ = NONE; - integrity_level_symbol_ = isolate_->factory()->nonextensible_symbol(); + Handle previous = + handle(Map::cast(old_map_->GetBackPointer()), isolate_); + Symbol integrity_level_symbol; + TransitionsAccessor last_transitions(isolate_, previous); + if (!last_transitions.HasIntegrityLevelTransitionTo( + *old_map_, &integrity_level_symbol, &integrity_level_)) { + // The last transition was not integrity level transition - just bail out. + // This can happen in the following cases: + // - there are private symbol transitions following the integrity level + // transitions (see crbug.com/v8/8854). + // - there is a getter added in addition to an existing setter (or a setter + // in addition to an existing getter). + return false; } + integrity_level_symbol_ = handle(integrity_level_symbol, isolate_); + integrity_source_map_ = previous; + + // Now walk up the back pointer chain and skip all integrity level + // transitions. If we encounter any non-integrity level transition interleaved + // with integrity level transitions, just bail out. + while (!integrity_source_map_->is_extensible()) { + previous = + handle(Map::cast(integrity_source_map_->GetBackPointer()), isolate_); + TransitionsAccessor transitions(isolate_, previous); + if (!transitions.HasIntegrityLevelTransitionTo(*integrity_source_map_)) { + return false; + } + integrity_source_map_ = previous; + } + + // Integrity-level transitions never change number of descriptors. + CHECK_EQ(old_map_->NumberOfOwnDescriptors(), + integrity_source_map_->NumberOfOwnDescriptors()); has_integrity_level_transition_ = true; old_descriptors_ = diff --git a/src/objects/map.cc b/src/objects/map.cc index 4927151704..6daf5fcfd3 100644 --- a/src/objects/map.cc +++ b/src/objects/map.cc @@ -21,7 +21,6 @@ #include "src/objects/oddball.h" #include "src/ostreams.h" #include "src/property.h" -#include "src/roots.h" #include "src/transitions-inl.h" #include "src/zone/zone-containers.h" @@ -938,40 +937,40 @@ IntegrityLevelTransitionInfo DetectIntegrityLevelTransitions( Map map, Isolate* isolate, DisallowHeapAllocation* no_allocation) { IntegrityLevelTransitionInfo info(map); + // Figure out the most restrictive integrity level transition (it should + // be the last one in the transition tree). DCHECK(!map->is_extensible()); - 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()) { + Map previous = Map::cast(map->GetBackPointer()); + TransitionsAccessor last_transitions(isolate, previous, no_allocation); + if (!last_transitions.HasIntegrityLevelTransitionTo( + map, &(info.integrity_level_symbol), &(info.integrity_level))) { + // The last transition was not integrity level transition - just bail out. + // This can happen in the following cases: + // - there are private symbol transitions following the integrity level + // transitions (see crbug.com/v8/8854). + // - there is a getter added in addition to an existing setter (or a setter + // in addition to an existing getter). 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, Map::cast(map->GetBackPointer()), - no_allocation); - if (transitions.SearchSpecial(roots.frozen_symbol()) == map) { - info.integrity_level = FROZEN; - info.integrity_level_symbol = roots.frozen_symbol(); - } else if (transitions.SearchSpecial(roots.sealed_symbol()) == map) { - info.integrity_level = SEALED; - info.integrity_level_symbol = roots.sealed_symbol(); - } else { - CHECK_EQ(transitions.SearchSpecial(roots.nonextensible_symbol()), map); - info.integrity_level = NONE; - info.integrity_level_symbol = roots.nonextensible_symbol(); + Map source_map = previous; + // Now walk up the back pointer chain and skip all integrity level + // transitions. If we encounter any non-integrity level transition interleaved + // with integrity level transitions, just bail out. + while (!source_map->is_extensible()) { + previous = Map::cast(source_map->GetBackPointer()); + TransitionsAccessor transitions(isolate, previous, no_allocation); + if (!transitions.HasIntegrityLevelTransitionTo(source_map)) { + return info; + } + source_map = previous; } + // Integrity-level transitions never change number of descriptors. + CHECK_EQ(map->NumberOfOwnDescriptors(), source_map->NumberOfOwnDescriptors()); + info.has_integrity_level_transition = true; info.integrity_level_source_map = source_map; - return info; } diff --git a/src/transitions.cc b/src/transitions.cc index 3dbc1602fa..bdf16d7eef 100644 --- a/src/transitions.cc +++ b/src/transitions.cc @@ -690,5 +690,23 @@ void TransitionArray::Sort() { DCHECK(IsSortedNoDuplicates()); } +bool TransitionsAccessor::HasIntegrityLevelTransitionTo( + Map to, Symbol* out_symbol, PropertyAttributes* out_integrity_level) { + ReadOnlyRoots roots(isolate_); + if (SearchSpecial(roots.frozen_symbol()) == to) { + if (out_integrity_level) *out_integrity_level = FROZEN; + if (out_symbol) *out_symbol = roots.frozen_symbol(); + } else if (SearchSpecial(roots.sealed_symbol()) == to) { + if (out_integrity_level) *out_integrity_level = SEALED; + if (out_symbol) *out_symbol = roots.sealed_symbol(); + } else if (SearchSpecial(roots.nonextensible_symbol()) == to) { + if (out_integrity_level) *out_integrity_level = NONE; + if (out_symbol) *out_symbol = roots.nonextensible_symbol(); + } else { + return false; + } + return true; +} + } // namespace internal } // namespace v8 diff --git a/src/transitions.h b/src/transitions.h index 58268907e3..55bc14ce5a 100644 --- a/src/transitions.h +++ b/src/transitions.h @@ -86,6 +86,10 @@ class TransitionsAccessor { static bool IsMatchingMap(Map target, Name name, PropertyKind kind, PropertyAttributes attributes); + bool HasIntegrityLevelTransitionTo( + Map to, Symbol* out_symbol = nullptr, + PropertyAttributes* out_integrity_level = nullptr); + // ===== ITERATION ===== typedef void (*TraverseCallback)(Map map, void* data); diff --git a/test/mjsunit/regress/regress-932953.js b/test/mjsunit/regress/regress-932953.js new file mode 100644 index 0000000000..5e211c79d1 --- /dev/null +++ b/test/mjsunit/regress/regress-932953.js @@ -0,0 +1,59 @@ +// 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: --allow-natives-syntax + +(function NonExtensibleBetweenSetterAndGetter() { + o = {}; + o.x = 42; + o.__defineGetter__("y", function() { }); + Object.preventExtensions(o); + o.__defineSetter__("y", function() { }); + o.x = 0.1; +})(); + +(function InterleavedIntegrityLevel() { + o = {}; + o.x = 42; + o.__defineSetter__("y", function() { }); + Object.preventExtensions(o); + o.__defineGetter__("y", function() { return 44; }); + Object.seal(o); + o.x = 0.1; + assertEquals(44, o.y); +})(); + +(function TryUpdateRepeatedIntegrityLevel() { + function C() { + this.x = 0; + this.x = 1; + Object.preventExtensions(this); + Object.seal(this); + } + + const o1 = new C(); + const o2 = new C(); + const o3 = new C(); + + function f(o) { + return o.x; + } + + // Warm up the IC. + f(o1); + f(o1); + f(o1); + + // Reconfigure to double field. + o3.x = 0.1; + + // Migrate o2 to the new shape. + f(o2); + + %OptimizeFunctionOnNextCall(f); + f(o1); + + assertTrue(%HaveSameMap(o1, o2)); + assertTrue(%HaveSameMap(o1, o3)); +})();