From: mvstanton Date: Wed, 22 Apr 2015 09:56:39 +0000 (-0700) Subject: Revert of Protect the emptiness of Array prototype elements with a PropertyCell.... X-Git-Tag: upstream/4.7.83~3042 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=15b98a3328b2a2aafb9dfd73287dd7b268e51455;p=platform%2Fupstream%2Fv8.git Revert of Protect the emptiness of Array prototype elements with a PropertyCell. (patchset #7 id:120001 of https://codereview.chromium.org/1092043002/) Reason for revert: MAC GCSTRESS failure on new test. Original issue's description: > Protect the emptiness of Array prototype elements with a PropertyCell. > > Not just emptiness, but also a particular structure. > > BUG=v8:4044 > LOG=N TBR=jkummerow@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:4044 Review URL: https://codereview.chromium.org/1099203004 Cr-Commit-Position: refs/heads/master@{#27998} --- diff --git a/src/builtins.cc b/src/builtins.cc index 5b1eeed15..3c3c7dda1 100644 --- a/src/builtins.cc +++ b/src/builtins.cc @@ -199,21 +199,7 @@ static bool ArrayPrototypeHasNoElements(Heap* heap, PrototypeIterator* iter) { static inline bool IsJSArrayFastElementMovingAllowed(Heap* heap, JSArray* receiver) { DisallowHeapAllocation no_gc; - Isolate* isolate = heap->isolate(); - if (!isolate->IsFastArrayConstructorPrototypeChainIntact()) { - return false; - } - - // If the array prototype chain is intact (and free of elements), and if the - // receiver's prototype is the array prototype, then we are done. - Object* prototype = receiver->map()->prototype(); - if (prototype->IsJSArray() && - isolate->is_initial_array_prototype(JSArray::cast(prototype))) { - return true; - } - - // Slow case. - PrototypeIterator iter(isolate, receiver); + PrototypeIterator iter(heap->isolate(), receiver); return ArrayPrototypeHasNoElements(heap, &iter); } @@ -250,7 +236,7 @@ static inline MaybeHandle EnsureJSArrayWithWritableFastElements( // Adding elements to the array prototype would break code that makes sure // it has no elements. Handle that elsewhere. - if (isolate->IsAnyInitialArrayPrototype(array)) { + if (array->GetIsolate()->is_initial_array_prototype(*array)) { return MaybeHandle(); } diff --git a/src/compilation-dependencies.h b/src/compilation-dependencies.h index 1ed6e5d9b..ddd0be83b 100644 --- a/src/compilation-dependencies.h +++ b/src/compilation-dependencies.h @@ -25,6 +25,9 @@ class CompilationDependencies { void AssumeInitialMapCantChange(Handle map) { Insert(DependentCode::kInitialMapChangedGroup, map); } + void AssumeElementsCantBeAdded(Handle map) { + Insert(DependentCode::kElementsCantBeAddedGroup, map); + } void AssumeFieldType(Handle map) { Insert(DependentCode::kFieldTypeGroup, map); } diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 84d383bfa..45c26dfaa 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -3077,10 +3077,6 @@ void Heap::CreateInitialObjects() { // Handling of script id generation is in Factory::NewScript. set_last_script_id(Smi::FromInt(v8::UnboundScript::kNoScriptId)); - Handle cell = factory->NewPropertyCell(); - cell->set_value(Smi::FromInt(1)); - set_array_protector(*cell); - set_allocation_sites_scratchpad( *factory->NewFixedArray(kAllocationSiteScratchpadSize, TENURED)); InitializeAllocationSitesScratchpad(); diff --git a/src/heap/heap.h b/src/heap/heap.h index 7573c676a..ebeab181d 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -185,8 +185,7 @@ namespace internal { V(FixedArray, keyed_load_dummy_vector, KeyedLoadDummyVector) \ V(FixedArray, detached_contexts, DetachedContexts) \ V(ArrayList, retained_maps, RetainedMaps) \ - V(WeakHashTable, weak_object_to_code_table, WeakObjectToCodeTable) \ - V(PropertyCell, array_protector, ArrayProtector) + V(WeakHashTable, weak_object_to_code_table, WeakObjectToCodeTable) // Entries in this list are limited to Smis and are not visited during GC. #define SMI_ROOT_LIST(V) \ diff --git a/src/hydrogen.h b/src/hydrogen.h index b11d85636..412b53d1c 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -416,8 +416,10 @@ class HGraph final : public ZoneObject { void MarkDependsOnEmptyArrayProtoElements() { // Add map dependency if not already added. if (depends_on_empty_array_proto_elements_) return; - info()->dependencies()->AssumePropertyCell( - isolate()->factory()->array_protector()); + info()->dependencies()->AssumeElementsCantBeAdded( + handle(isolate()->initial_object_prototype()->map())); + info()->dependencies()->AssumeElementsCantBeAdded( + handle(isolate()->initial_array_prototype()->map())); depends_on_empty_array_proto_elements_ = true; } diff --git a/src/isolate.cc b/src/isolate.cc index f7a2740ef..8ffa7c8a1 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2374,91 +2374,29 @@ bool Isolate::use_crankshaft() const { bool Isolate::IsFastArrayConstructorPrototypeChainIntact() { - Handle no_elements_cell = - handle(heap()->array_protector(), this); - bool cell_reports_intact = no_elements_cell->value()->IsSmi() && - Smi::cast(no_elements_cell->value())->value() == 1; - -#ifdef DEBUG Map* root_array_map = get_initial_js_array_map(GetInitialFastElementsKind()); + DCHECK(root_array_map != NULL); JSObject* initial_array_proto = JSObject::cast(*initial_array_prototype()); - JSObject* initial_object_proto = JSObject::cast(*initial_object_prototype()); - - if (root_array_map == NULL || initial_array_proto == initial_object_proto) { - // We are in the bootstrapping process, and the entire check sequence - // shouldn't be performed. - return cell_reports_intact; - } // Check that the array prototype hasn't been altered WRT empty elements. - if (root_array_map->prototype() != initial_array_proto) { - DCHECK_EQ(false, cell_reports_intact); - return cell_reports_intact; - } - + if (root_array_map->prototype() != initial_array_proto) return false; if (initial_array_proto->elements() != heap()->empty_fixed_array()) { - DCHECK_EQ(false, cell_reports_intact); - return cell_reports_intact; + return false; } // Check that the object prototype hasn't been altered WRT empty elements. + JSObject* initial_object_proto = JSObject::cast(*initial_object_prototype()); PrototypeIterator iter(this, initial_array_proto); if (iter.IsAtEnd() || iter.GetCurrent() != initial_object_proto) { - DCHECK_EQ(false, cell_reports_intact); - return cell_reports_intact; + return false; } if (initial_object_proto->elements() != heap()->empty_fixed_array()) { - DCHECK_EQ(false, cell_reports_intact); - return cell_reports_intact; + return false; } iter.Advance(); - if (!iter.IsAtEnd()) { - DCHECK_EQ(false, cell_reports_intact); - return cell_reports_intact; - } - -#endif - - return cell_reports_intact; -} - - -void Isolate::UpdateArrayProtectorOnSetElement(Handle object) { - Handle array_protector = factory()->array_protector(); - if (IsFastArrayConstructorPrototypeChainIntact() && - object->map()->is_prototype_map()) { - Object* context = heap()->native_contexts_list(); - while (!context->IsUndefined()) { - Context* current_context = Context::cast(context); - if (current_context->get(Context::INITIAL_OBJECT_PROTOTYPE_INDEX) == - *object || - current_context->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX) == - *object) { - PropertyCell::SetValueWithInvalidation(array_protector, - handle(Smi::FromInt(0), this)); - break; - } - context = current_context->get(Context::NEXT_CONTEXT_LINK); - } - } -} - - -bool Isolate::IsAnyInitialArrayPrototype(Handle array) { - if (array->map()->is_prototype_map()) { - Object* context = heap()->native_contexts_list(); - while (!context->IsUndefined()) { - Context* current_context = Context::cast(context); - if (current_context->get(Context::INITIAL_ARRAY_PROTOTYPE_INDEX) == - *array) { - return true; - } - context = current_context->get(Context::NEXT_CONTEXT_LINK); - } - } - return false; + return iter.IsAtEnd(); } diff --git a/src/isolate.h b/src/isolate.h index fb4e069ad..ab2efd78f 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -1016,21 +1016,6 @@ class Isolate { bool IsFastArrayConstructorPrototypeChainIntact(); - // On intent to set an element in object, make sure that appropriate - // notifications occur if the set is on the elements of the array or - // object prototype. Also ensure that changes to prototype chain between - // Array and Object fire notifications. - void UpdateArrayProtectorOnSetElement(Handle object); - void UpdateArrayProtectorOnSetPrototype(Handle object) { - UpdateArrayProtectorOnSetElement(object); - } - void UpdateArrayProtectorOnNormalizeElements(Handle object) { - UpdateArrayProtectorOnSetElement(object); - } - - // Returns true if array is the initial array prototype in any native context. - bool IsAnyInitialArrayPrototype(Handle array); - CallInterfaceDescriptorData* call_descriptor_data(int index); void IterateDeferredHandles(ObjectVisitor* visitor); diff --git a/src/objects.cc b/src/objects.cc index 76e97b1b2..c11491208 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -4899,11 +4899,6 @@ Handle JSObject::NormalizeElements( DCHECK(object->HasFastSmiOrObjectElements() || object->HasFastDoubleElements() || object->HasFastArgumentsElements()); - - // Ensure that notifications fire if the array or object prototypes are - // normalizing. - isolate->UpdateArrayProtectorOnNormalizeElements(object); - // Compute the effective length and allocate a new backing store. int length = object->IsJSArray() ? Smi::cast(Handle::cast(object)->length())->value() @@ -5758,7 +5753,6 @@ MaybeHandle JSObject::PreventExtensionsWithTransition( Handle new_element_dictionary; if (!object->elements()->IsDictionary()) { new_element_dictionary = GetNormalizedElementDictionary(object); - isolate->UpdateArrayProtectorOnNormalizeElements(object); } Handle transition_marker; @@ -12432,6 +12426,8 @@ const char* DependentCode::DependencyGroupName(DependencyGroup group) { return "transition"; case kPrototypeCheckGroup: return "prototype-check"; + case kElementsCantBeAddedGroup: + return "elements-cant-be-added"; case kPropertyCellChangedGroup: return "property-cell-changed"; case kFieldTypeGroup: @@ -12530,8 +12526,6 @@ MaybeHandle JSObject::SetPrototype(Handle object, // Nothing to do if prototype is already set. if (map->prototype() == *value) return value; - isolate->UpdateArrayProtectorOnSetPrototype(real_receiver); - PrototypeOptimizationMode mode = from_javascript ? REGULAR_PROTOTYPE : FAST_PROTOTYPE; Handle new_map = Map::TransitionToPrototype(map, value, mode); @@ -12752,7 +12746,11 @@ MaybeHandle JSObject::SetFastElement(Handle object, // Array optimizations rely on the prototype lookups of Array objects always // returning undefined. If there is a store to the initial prototype object, // make sure all of these optimizations are invalidated. - isolate->UpdateArrayProtectorOnSetElement(object); + if (isolate->is_initial_object_prototype(*object) || + isolate->is_initial_array_prototype(*object)) { + object->map()->dependent_code()->DeoptimizeDependentCodeGroup(isolate, + DependentCode::kElementsCantBeAddedGroup); + } Handle backing_store(FixedArray::cast(object->elements())); if (backing_store->map() == @@ -17100,15 +17098,4 @@ Handle PropertyCell::UpdateCell(Handle dictionary, return value; } - -// static -void PropertyCell::SetValueWithInvalidation(Handle cell, - Handle new_value) { - if (cell->value() != *new_value) { - cell->set_value(*new_value); - Isolate* isolate = cell->GetIsolate(); - cell->dependent_code()->DeoptimizeDependentCodeGroup( - isolate, DependentCode::kPropertyCellChangedGroup); - } -} } } // namespace v8::internal diff --git a/src/objects.h b/src/objects.h index d6fbaee93..efe4b4cc5 100644 --- a/src/objects.h +++ b/src/objects.h @@ -5751,6 +5751,9 @@ class DependentCode: public FixedArray { // described by this map changes shape (and transitions to a new map), // possibly invalidating the assumptions embedded in the code. kPrototypeCheckGroup, + // Group of code that depends on elements not being added to objects with + // this map. + kElementsCantBeAddedGroup, // Group of code that depends on global property values in property cells // not being changed. kPropertyCellChangedGroup, @@ -9833,9 +9836,6 @@ class PropertyCell : public HeapObject { static Handle InvalidateEntry(Handle dictionary, int entry); - static void SetValueWithInvalidation(Handle cell, - Handle new_value); - DECLARE_CAST(PropertyCell) // Dispatched behavior. diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 7954598c6..ba636b234 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -16652,52 +16652,6 @@ UNINITIALIZED_TEST(DisposeIsolateWhenInUse) { } -static void BreakArrayGuarantees(const char* script) { - v8::Isolate* isolate1 = v8::Isolate::New(); - isolate1->Enter(); - v8::Persistent context1; - { - v8::HandleScope scope(isolate1); - context1.Reset(isolate1, Context::New(isolate1)); - } - - { - v8::HandleScope scope(isolate1); - v8::Local context = - v8::Local::New(isolate1, context1); - v8::Context::Scope context_scope(context); - v8::internal::Isolate* i_isolate = - reinterpret_cast(isolate1); - CHECK_EQ(true, i_isolate->IsFastArrayConstructorPrototypeChainIntact()); - // Run something in new isolate. - CompileRun(script); - CHECK_EQ(false, i_isolate->IsFastArrayConstructorPrototypeChainIntact()); - } - isolate1->Exit(); - isolate1->Dispose(); -} - - -TEST(VerifyArrayPrototypeGuarantees) { - // Break fast array hole handling by element changes. - BreakArrayGuarantees("[].__proto__[1] = 3;"); - BreakArrayGuarantees("Object.prototype[3] = 'three';"); - BreakArrayGuarantees("Array.prototype.push(1);"); - BreakArrayGuarantees("Array.prototype.unshift(1);"); - // Break fast array hole handling by prototype structure changes. - BreakArrayGuarantees("[].__proto__.__proto__ = { funny: true };"); - // By sending elements to dictionary mode. - BreakArrayGuarantees("Object.freeze(Array.prototype);"); - BreakArrayGuarantees("Object.freeze(Object.prototype);"); - BreakArrayGuarantees( - "Object.defineProperty(Array.prototype, 0, {" - " get: function() { return 3; }});"); - BreakArrayGuarantees( - "Object.defineProperty(Object.prototype, 0, {" - " get: function() { return 3; }});"); -} - - TEST(RunTwoIsolatesOnSingleThread) { // Run isolate 1. v8::Isolate* isolate1 = v8::Isolate::New(); diff --git a/test/mjsunit/concurrent-initial-prototype-change.js b/test/mjsunit/concurrent-initial-prototype-change.js index 6c9a9f4d5..1b6f97b43 100644 --- a/test/mjsunit/concurrent-initial-prototype-change.js +++ b/test/mjsunit/concurrent-initial-prototype-change.js @@ -27,14 +27,6 @@ // Flags: --allow-natives-syntax // Flags: --concurrent-recompilation --block-concurrent-recompilation -// Flags: --nostress-opt - -// --nostress-opt is in place because this particular optimization -// (guaranteeing that the Array prototype chain has no elements) is -// maintained isolate-wide. Once it's been "broken" by the change -// to the Object prototype below, future compiles will not use the -// optimization anymore, and the code will remain optimized despite -// additional changes to the prototype chain. if (!%IsConcurrentRecompilationSupported()) { print("Concurrent recompilation is disabled. Skipping this test."); diff --git a/test/mjsunit/elide-double-hole-check-12.js b/test/mjsunit/elide-double-hole-check-12.js deleted file mode 100644 index 758734db8..000000000 --- a/test/mjsunit/elide-double-hole-check-12.js +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright 2015 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 f1(a, i) { - return a[i] + 0.5; -} - -var other_realm = Realm.create(); -var arr = [,0.0,2.5]; -assertEquals(0.5, f1(arr, 1)); -assertEquals(0.5, f1(arr, 1)); -%OptimizeFunctionOnNextCall(f1); -assertEquals(0.5, f1(arr, 1)); - -Realm.shared = arr.__proto__; - -// The call syntax is useful to make sure the native context is that of the -// other realm. -Realm.eval(other_realm, "Array.prototype.push.call(Realm.shared, 1);"); -assertEquals(1.5, f1(arr, 0));