Commit 1a3a2bc3 authored by Jaroslav Sevcik's avatar Jaroslav Sevcik Committed by Commit Bot

Fix accessor update of non-extensible maps.

When installing getter/setter of non-extensible map with existing
setter/getter of the same name, we introduce a new transition
(so we have two transitions with the same name!). This triggers
an assertion in map updater.

This fix carefully checks that on the back-pointer path from
non-extensible map to the extensible map there are only
integrity level transitions. Otherwise, we just bail out.

Bug: chromium:932953
Change-Id: I02e91c3b652428a84a9f5c58b6691ea9b1fc44d6
Reviewed-on: https://chromium-review.googlesource.com/c/1477067Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59667}
parent 4d9381ba
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "src/isolate.h" #include "src/isolate.h"
#include "src/objects-inl.h" #include "src/objects-inl.h"
#include "src/objects.h" #include "src/objects.h"
#include "src/property-details.h"
#include "src/transitions.h" #include "src/transitions.h"
namespace v8 { namespace v8 {
...@@ -254,40 +255,42 @@ MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() { ...@@ -254,40 +255,42 @@ MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() {
} }
bool MapUpdater::TrySaveIntegrityLevelTransitions() { bool MapUpdater::TrySaveIntegrityLevelTransitions() {
// Skip integrity level transitions. // Figure out the most restrictive integrity level transition (it should
integrity_source_map_ = old_map_; // be the last one in the transition tree).
while (!integrity_source_map_->is_extensible()) { Handle<Map> previous =
integrity_source_map_ = handle(Map::cast(old_map_->GetBackPointer()), isolate_);
handle(Map::cast(integrity_source_map_->GetBackPointer()), isolate_); Symbol integrity_level_symbol;
} TransitionsAccessor last_transitions(isolate_, previous);
if (!last_transitions.HasIntegrityLevelTransitionTo(
// If there are some non-integrity-level transitions after the first *old_map_, &integrity_level_symbol, &integrity_level_)) {
// non-extensible transitions, e.g., if there were private symbols transitions // The last transition was not integrity level transition - just bail out.
// after the first integrity level transition, then we just bail out and // This can happen in the following cases:
// generalize all the fields. // - there are private symbol transitions following the integrity level
if (old_map_->NumberOfOwnDescriptors() != // transitions (see crbug.com/v8/8854).
integrity_source_map_->NumberOfOwnDescriptors()) { // - there is a getter added in addition to an existing setter (or a setter
// in addition to an existing getter).
return false; return false;
} }
integrity_level_symbol_ = handle(integrity_level_symbol, isolate_);
integrity_source_map_ = previous;
// Figure out the most restrictive integrity level transition (it should // Now walk up the back pointer chain and skip all integrity level
// be the last one in the transition tree). // transitions. If we encounter any non-integrity level transition interleaved
ReadOnlyRoots roots(isolate_); // with integrity level transitions, just bail out.
TransitionsAccessor transitions( while (!integrity_source_map_->is_extensible()) {
isolate_, handle(Map::cast(old_map_->GetBackPointer()), isolate_)); previous =
if (transitions.SearchSpecial(roots.frozen_symbol()) == *old_map_) { handle(Map::cast(integrity_source_map_->GetBackPointer()), isolate_);
integrity_level_ = FROZEN; TransitionsAccessor transitions(isolate_, previous);
integrity_level_symbol_ = isolate_->factory()->frozen_symbol(); if (!transitions.HasIntegrityLevelTransitionTo(*integrity_source_map_)) {
} else if (transitions.SearchSpecial(roots.sealed_symbol()) == *old_map_) { return false;
integrity_level_ = SEALED; }
integrity_level_symbol_ = isolate_->factory()->sealed_symbol(); integrity_source_map_ = previous;
} else {
CHECK_EQ(transitions.SearchSpecial(roots.nonextensible_symbol()),
*old_map_);
integrity_level_ = NONE;
integrity_level_symbol_ = isolate_->factory()->nonextensible_symbol();
} }
// Integrity-level transitions never change number of descriptors.
CHECK_EQ(old_map_->NumberOfOwnDescriptors(),
integrity_source_map_->NumberOfOwnDescriptors());
has_integrity_level_transition_ = true; has_integrity_level_transition_ = true;
old_descriptors_ = old_descriptors_ =
handle(integrity_source_map_->instance_descriptors(), isolate_); handle(integrity_source_map_->instance_descriptors(), isolate_);
......
...@@ -21,7 +21,6 @@ ...@@ -21,7 +21,6 @@
#include "src/objects/oddball.h" #include "src/objects/oddball.h"
#include "src/ostreams.h" #include "src/ostreams.h"
#include "src/property.h" #include "src/property.h"
#include "src/roots.h"
#include "src/transitions-inl.h" #include "src/transitions-inl.h"
#include "src/zone/zone-containers.h" #include "src/zone/zone-containers.h"
...@@ -938,40 +937,39 @@ IntegrityLevelTransitionInfo DetectIntegrityLevelTransitions( ...@@ -938,40 +937,39 @@ IntegrityLevelTransitionInfo DetectIntegrityLevelTransitions(
Map map, Isolate* isolate, DisallowHeapAllocation* no_allocation) { Map map, Isolate* isolate, DisallowHeapAllocation* no_allocation) {
IntegrityLevelTransitionInfo info(map); 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()); DCHECK(!map->is_extensible());
Map source_map = map; Map previous = Map::cast(map->GetBackPointer());
// Skip integrity level transitions. TransitionsAccessor last_transitions(isolate, previous, no_allocation);
while (!source_map->is_extensible()) { if (!last_transitions.HasIntegrityLevelTransitionTo(
source_map = Map::cast(source_map->GetBackPointer()); 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:
// If there are some non-integrity-level transitions after the first // - there are private symbol transitions following the integrity level
// non-extensible transitions, e.g., if there were private symbols transitions // transitions (see crbug.com/v8/8854).
// after the first integrity level transition, then we just bail out. // - there is a getter added in addition to an existing setter (or a setter
if (map->NumberOfOwnDescriptors() != source_map->NumberOfOwnDescriptors()) { // in addition to an existing getter).
return info; return info;
} }
// Figure out the most restrictive integrity level transition (it should Map source_map = previous;
// be the last one in the transition tree). // Now walk up the back pointer chain and skip all integrity level
ReadOnlyRoots roots(isolate); // transitions. If we encounter any non-integrity level transition interleaved
TransitionsAccessor transitions(isolate, Map::cast(map->GetBackPointer()), // with integrity level transitions, just bail out.
no_allocation); while (!source_map->is_extensible()) {
if (transitions.SearchSpecial(roots.frozen_symbol()) == map) { previous = Map::cast(source_map->GetBackPointer());
info.integrity_level = FROZEN; TransitionsAccessor transitions(isolate, previous, no_allocation);
info.integrity_level_symbol = roots.frozen_symbol(); if (!transitions.HasIntegrityLevelTransitionTo(source_map)) {
} else if (transitions.SearchSpecial(roots.sealed_symbol()) == map) { return info;
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();
} }
// Integrity-level transitions never change number of descriptors.
CHECK_EQ(map->NumberOfOwnDescriptors(), source_map->NumberOfOwnDescriptors());
info.has_integrity_level_transition = true; info.has_integrity_level_transition = true;
info.integrity_level_source_map = source_map; info.integrity_level_source_map = source_map;
return info; return info;
} }
......
...@@ -690,5 +690,23 @@ void TransitionArray::Sort() { ...@@ -690,5 +690,23 @@ void TransitionArray::Sort() {
DCHECK(IsSortedNoDuplicates()); 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 internal
} // namespace v8 } // namespace v8
...@@ -86,6 +86,10 @@ class TransitionsAccessor { ...@@ -86,6 +86,10 @@ class TransitionsAccessor {
static bool IsMatchingMap(Map target, Name name, PropertyKind kind, static bool IsMatchingMap(Map target, Name name, PropertyKind kind,
PropertyAttributes attributes); PropertyAttributes attributes);
bool HasIntegrityLevelTransitionTo(
Map to, Symbol* out_symbol = nullptr,
PropertyAttributes* out_integrity_level = nullptr);
// ===== ITERATION ===== // ===== ITERATION =====
typedef void (*TraverseCallback)(Map map, void* data); typedef void (*TraverseCallback)(Map map, void* data);
......
// 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.
(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);
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment