[runtime] Don't set sticky bit on empty_slow_element_dictionary

Empty slow element dictionary had the sticky bit set. This bit was
used to indicate that the dictionary cannot go to the fast mode either
because the dictionary had elements with attributed or elements at large
indices. There is no reason for the empty dictionary to have this bit set.
This causes bugs in some corner cases.

Bug: chromium:1003732
Change-Id: Ib29e1cda784869b9deb9361d8e6b5539f7154a38
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1833686
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64158}

src/heap/setup-heap-internal.cc

@@ -788,7 +788,6 @@ void Heap::CreateInitialObjects() {
   Handle<NumberDictionary> slow_element_dictionary = NumberDictionary::New(
       isolate(), 1, AllocationType::kReadOnly, USE_CUSTOM_MINIMUM_CAPACITY);
   DCHECK(!slow_element_dictionary->HasSufficientCapacityToAdd(1));
-  slow_element_dictionary->set_requires_slow_elements();
   set_empty_slow_element_dictionary(*slow_element_dictionary);
 
   set_materialized_objects(*factory->NewFixedArray(0, AllocationType::kOld));

src/objects/js-objects.cc

@@ -3446,6 +3446,8 @@ void JSObject::MigrateSlowToFast(Handle<JSObject> object,
 }
 
 void JSObject::RequireSlowElements(NumberDictionary dictionary) {
+  DCHECK_NE(dictionary,
+            ReadOnlyRoots(GetIsolate()).empty_slow_element_dictionary());
   if (dictionary.requires_slow_elements()) return;
   dictionary.set_requires_slow_elements();
   if (map().is_prototype_map()) {
@@ -3714,7 +3716,9 @@ Maybe<bool> JSObject::PreventExtensions(Handle<JSObject> object,
            object->HasSlowArgumentsElements());
 
     // Make sure that we never go back to fast case.
-    object->RequireSlowElements(*dictionary);
+    if (*dictionary != ReadOnlyRoots(isolate).empty_slow_element_dictionary()) {
+      object->RequireSlowElements(*dictionary);
+    }
   }
 
   // Do a map transition, other objects with this map may still

src/objects/objects.cc

@@ -7311,8 +7311,13 @@ Handle<NumberDictionary> NumberDictionary::Set(
     Isolate* isolate, Handle<NumberDictionary> dictionary, uint32_t key,
     Handle<Object> value, Handle<JSObject> dictionary_holder,
     PropertyDetails details) {
-  dictionary->UpdateMaxNumberKey(key, dictionary_holder);
-  return AtPut(isolate, dictionary, key, value, details);
+  // We could call Set with empty dictionaries. UpdateMaxNumberKey doesn't
+  // expect empty dictionaries so make sure to call AtPut that correctly handles
+  // them by creating new dictionary when required.
+  Handle<NumberDictionary> new_dictionary =
+      AtPut(isolate, dictionary, key, value, details);
+  new_dictionary->UpdateMaxNumberKey(key, dictionary_holder);
+  return new_dictionary;
 }
 
 void NumberDictionary::CopyValuesTo(FixedArray elements) {

test/mjsunit/regress/regress-crbug-1003732.js

@@ -0,0 +1,25 @@
+// 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 f_1() {
+  var v = new Array();
+  v[0] = 10;
+  return v;
+}
+
+function test() {
+  var setter_called = false;
+  // Turn array to NumberDictionary
+  Array.prototype[123456789] = 42;
+  assertEquals(f_1().length, 1);
+
+  // Reset to empty_slow_dictionary
+  Array.prototype.length = 0;
+
+  // This should reset the prototype validity cell.
+  Array.prototype.__defineSetter__("0", function() {setter_called = true});
+  f_1();
+  assertEquals(setter_called, true);
+}
+test();