Revert of Protect the emptiness of Array prototype elements with a Pr…

…opertyCell. (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}

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<FixedArrayBase> 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<FixedArrayBase>();
   }
 

src/compilation-dependencies.h

@@ -25,6 +25,9 @@ class CompilationDependencies {
   void AssumeInitialMapCantChange(Handle<Map> map) {
     Insert(DependentCode::kInitialMapChangedGroup, map);
   }
+  void AssumeElementsCantBeAdded(Handle<Map> map) {
+    Insert(DependentCode::kElementsCantBeAddedGroup, map);
+  }
   void AssumeFieldType(Handle<Map> map) {
     Insert(DependentCode::kFieldTypeGroup, map);
   }

src/heap/heap.cc

@@ -3081,10 +3081,6 @@ void Heap::CreateInitialObjects() {
   // Handling of script id generation is in Factory::NewScript.
   set_last_script_id(Smi::FromInt(v8::UnboundScript::kNoScriptId));
 
-  Handle<PropertyCell> cell = factory->NewPropertyCell();
-  cell->set_value(Smi::FromInt(1));
-  set_array_protector(*cell);
-
   set_allocation_sites_scratchpad(
       *factory->NewFixedArray(kAllocationSiteScratchpadSize, TENURED));
   InitializeAllocationSitesScratchpad();

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)                                                   \

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;
   }
 

src/isolate.cc

@@ -2374,91 +2374,29 @@ bool Isolate::use_crankshaft() const {
 
 
 bool Isolate::IsFastArrayConstructorPrototypeChainIntact() {
-  Handle<PropertyCell> 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<JSObject> object) {
-  Handle<PropertyCell> 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<JSArray> 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();
 }
 
 

src/isolate.h

@@ -1025,21 +1025,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<JSObject> object);
-  void UpdateArrayProtectorOnSetPrototype(Handle<JSObject> object) {
-    UpdateArrayProtectorOnSetElement(object);
-  }
-  void UpdateArrayProtectorOnNormalizeElements(Handle<JSObject> object) {
-    UpdateArrayProtectorOnSetElement(object);
-  }
-
-  // Returns true if array is the initial array prototype in any native context.
-  bool IsAnyInitialArrayPrototype(Handle<JSArray> array);
-
   CallInterfaceDescriptorData* call_descriptor_data(int index);
 
   void IterateDeferredHandles(ObjectVisitor* visitor);

src/objects.cc

@@ -4899,11 +4899,6 @@ Handle<SeededNumberDictionary> 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<JSArray>::cast(object)->length())->value()
@@ -5758,7 +5753,6 @@ MaybeHandle<Object> JSObject::PreventExtensionsWithTransition(
   Handle<SeededNumberDictionary> new_element_dictionary;
   if (!object->elements()->IsDictionary()) {
     new_element_dictionary = GetNormalizedElementDictionary(object);
-    isolate->UpdateArrayProtectorOnNormalizeElements(object);
   }
 
   Handle<Symbol> 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<Object> JSObject::SetPrototype(Handle<JSObject> 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<Map> new_map = Map::TransitionToPrototype(map, value, mode);
@@ -12752,7 +12746,11 @@ MaybeHandle<Object> JSObject::SetFastElement(Handle<JSObject> 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<FixedArray> backing_store(FixedArray::cast(object->elements()));
   if (backing_store->map() ==
@@ -17100,15 +17098,4 @@ Handle<Object> PropertyCell::UpdateCell(Handle<NameDictionary> dictionary,
   return value;
 }
 
-
-// static
-void PropertyCell::SetValueWithInvalidation(Handle<PropertyCell> cell,
-                                            Handle<Object> 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

src/objects.h

@@ -5753,6 +5753,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,
@@ -9835,9 +9838,6 @@ class PropertyCell : public HeapObject {
   static Handle<PropertyCell> InvalidateEntry(Handle<NameDictionary> dictionary,
                                               int entry);
 
-  static void SetValueWithInvalidation(Handle<PropertyCell> cell,
-                                       Handle<Object> new_value);
-
   DECLARE_CAST(PropertyCell)
 
   // Dispatched behavior.

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<v8::Context> context1;
-  {
-    v8::HandleScope scope(isolate1);
-    context1.Reset(isolate1, Context::New(isolate1));
-  }
-
-  {
-    v8::HandleScope scope(isolate1);
-    v8::Local<v8::Context> context =
-        v8::Local<v8::Context>::New(isolate1, context1);
-    v8::Context::Scope context_scope(context);
-    v8::internal::Isolate* i_isolate =
-        reinterpret_cast<v8::internal::Isolate*>(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();

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.");