Revert of Revert of Protect the emptiness of Array prototype elements…

… with a PropertyCell. (patchset #1 id:1 of https://codereview.chromium.org/1099203004/)

Reason for revert:
This was probably an infrastructure problem caused by the mac ninja/goma switch.

Original issue's description:
> 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

TBR=jkummerow@chromium.org,mvstanton@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4044

Review URL: https://codereview.chromium.org/1052253003

Cr-Commit-Position: refs/heads/master@{#28000}

src/builtins.cc

@@ -199,7 +199,21 @@ static bool ArrayPrototypeHasNoElements(Heap* heap, PrototypeIterator* iter) {
 static inline bool IsJSArrayFastElementMovingAllowed(Heap* heap,
                                                      JSArray* receiver) {
   DisallowHeapAllocation no_gc;
-  PrototypeIterator iter(heap->isolate(), receiver);
+  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);
   return ArrayPrototypeHasNoElements(heap, &iter);
 }
 
@@ -236,7 +250,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 (array->GetIsolate()->is_initial_array_prototype(*array)) {
+  if (isolate->IsAnyInitialArrayPrototype(array)) {
     return MaybeHandle<FixedArrayBase>();
   }
 

src/compilation-dependencies.h

@@ -25,9 +25,6 @@ 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

@@ -3077,6 +3077,10 @@ 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,7 +185,8 @@ 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(WeakHashTable, weak_object_to_code_table, WeakObjectToCodeTable)           \
+  V(PropertyCell, array_protector, ArrayProtector)
 
 // Entries in this list are limited to Smis and are not visited during GC.
 #define SMI_ROOT_LIST(V)                                                   \

src/hydrogen.h

@@ -416,10 +416,8 @@ class HGraph final : public ZoneObject {
   void MarkDependsOnEmptyArrayProtoElements() {
     // Add map dependency if not already added.
     if (depends_on_empty_array_proto_elements_) return;
-    info()->dependencies()->AssumeElementsCantBeAdded(
-        handle(isolate()->initial_object_prototype()->map()));
-    info()->dependencies()->AssumeElementsCantBeAdded(
-        handle(isolate()->initial_array_prototype()->map()));
+    info()->dependencies()->AssumePropertyCell(
+        isolate()->factory()->array_protector());
     depends_on_empty_array_proto_elements_ = true;
   }
 

src/isolate.cc

@@ -2374,29 +2374,91 @@ 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) return false;
+  if (root_array_map->prototype() != initial_array_proto) {
+    DCHECK_EQ(false, cell_reports_intact);
+    return cell_reports_intact;
+  }
+
   if (initial_array_proto->elements() != heap()->empty_fixed_array()) {
-    return false;
+    DCHECK_EQ(false, cell_reports_intact);
+    return cell_reports_intact;
   }
 
   // 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) {
-    return false;
+    DCHECK_EQ(false, cell_reports_intact);
+    return cell_reports_intact;
   }
   if (initial_object_proto->elements() != heap()->empty_fixed_array()) {
-    return false;
+    DCHECK_EQ(false, cell_reports_intact);
+    return cell_reports_intact;
   }
 
   iter.Advance();
-  return iter.IsAtEnd();
+  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;
 }
 
 

src/isolate.h

@@ -1016,6 +1016,21 @@ 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,6 +4899,11 @@ 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()
@@ -5753,6 +5758,7 @@ 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;
@@ -12426,8 +12432,6 @@ 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:
@@ -12526,6 +12530,8 @@ 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);
@@ -12746,11 +12752,7 @@ 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.
-  if (isolate->is_initial_object_prototype(*object) ||
-      isolate->is_initial_array_prototype(*object)) {
-    object->map()->dependent_code()->DeoptimizeDependentCodeGroup(isolate,
-        DependentCode::kElementsCantBeAddedGroup);
-  }
+  isolate->UpdateArrayProtectorOnSetElement(object);
 
   Handle<FixedArray> backing_store(FixedArray::cast(object->elements()));
   if (backing_store->map() ==
@@ -17098,4 +17100,15 @@ 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

@@ -5751,9 +5751,6 @@ 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,
@@ -9836,6 +9833,9 @@ 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,6 +16652,52 @@ 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,6 +27,14 @@
 
 // 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.");

test/mjsunit/elide-double-hole-check-12.js

@@ -0,0 +1,23 @@
+// 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));