[runtime] Do not shrink fixed arrays to length 0.

Instead use the canonical empty fixed array. Some code assumes
that this is the only fixed array of length 0.

Bug: chromium:843062
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: If780acf50147c061a81f2ff2b31779fbd1c78559
Reviewed-on: https://chromium-review.googlesource.com/1064052
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53320}

src/api.cc

@@ -6399,10 +6399,12 @@ i::Object** GetSerializedDataFromFixedArray(i::Isolate* isolate,
     i::Object* object = list->get(int_index);
     if (!object->IsTheHole(isolate)) {
       list->set_the_hole(isolate, int_index);
-      // Shrink the list so that the last element is not the hole.
+      // Shrink the list so that the last element is not the hole (unless it's
+      // the first element, because we don't want to end up with a non-canonical
+      // empty FixedArray).
       int last = list->length() - 1;
       while (last >= 0 && list->is_the_hole(isolate, last)) last--;
-      list->Shrink(last + 1);
+      if (last != -1) list->Shrink(last + 1);
       return i::Handle<i::Object>(object, isolate).location();
     }
   }
@@ -6413,7 +6415,7 @@ i::Object** GetSerializedDataFromFixedArray(i::Isolate* isolate,
 i::Object** Context::GetDataFromSnapshotOnce(size_t index) {
   auto context = Utils::OpenHandle(this);
   i::Isolate* i_isolate = context->GetIsolate();
-  i::FixedArray* list = i::FixedArray::cast(context->serialized_objects());
+  i::FixedArray* list = context->serialized_objects();
   return GetSerializedDataFromFixedArray(i_isolate, list, index);
 }
 

src/compiler/effect-control-linearizer.cc

@@ -2586,6 +2586,11 @@ Node* EffectControlLinearizer::LowerNewDoubleElements(Node* node) {
   PretenureFlag const pretenure = PretenureFlagOf(node->op());
   Node* length = node->InputAt(0);
 
+  auto done = __ MakeLabel(MachineRepresentation::kTaggedPointer);
+  Node* zero_length = __ Word32Equal(length, __ Int32Constant(0));
+  __ GotoIf(zero_length, &done,
+            jsgraph()->HeapConstant(factory()->empty_fixed_array()));
+
   // Compute the effective size of the backing store.
   Node* size =
       __ Int32Add(__ Word32Shl(length, __ Int32Constant(kDoubleSizeLog2)),
@@ -2604,14 +2609,13 @@ Node* EffectControlLinearizer::LowerNewDoubleElements(Node* node) {
   Node* the_hole =
       __ LoadField(AccessBuilder::ForHeapNumberValue(), __ TheHoleConstant());
   auto loop = __ MakeLoopLabel(MachineType::PointerRepresentation());
-  auto done_loop = __ MakeLabel();
   __ Goto(&loop, __ IntPtrConstant(0));
   __ Bind(&loop);
   {
     // Check if we've initialized everything.
     Node* index = loop.PhiAt(0);
     Node* check = __ UintLessThan(index, limit);
-    __ GotoIfNot(check, &done_loop);
+    __ GotoIfNot(check, &done, result);
 
     // Storing "the_hole" doesn't need a write barrier.
     StoreRepresentation rep(MachineRepresentation::kFloat64, kNoWriteBarrier);
@@ -2625,14 +2629,19 @@ Node* EffectControlLinearizer::LowerNewDoubleElements(Node* node) {
     __ Goto(&loop, index);
   }
 
-  __ Bind(&done_loop);
-  return result;
+  __ Bind(&done);
+  return done.PhiAt(0);
 }
 
 Node* EffectControlLinearizer::LowerNewSmiOrObjectElements(Node* node) {
   PretenureFlag const pretenure = PretenureFlagOf(node->op());
   Node* length = node->InputAt(0);
 
+  auto done = __ MakeLabel(MachineRepresentation::kTaggedPointer);
+  Node* zero_length = __ Word32Equal(length, __ Int32Constant(0));
+  __ GotoIf(zero_length, &done,
+            jsgraph()->HeapConstant(factory()->empty_fixed_array()));
+
   // Compute the effective size of the backing store.
   Node* size =
       __ Int32Add(__ Word32Shl(length, __ Int32Constant(kPointerSizeLog2)),
@@ -2648,14 +2657,13 @@ Node* EffectControlLinearizer::LowerNewSmiOrObjectElements(Node* node) {
   Node* limit = ChangeUint32ToUintPtr(length);
   Node* the_hole = __ TheHoleConstant();
   auto loop = __ MakeLoopLabel(MachineType::PointerRepresentation());
-  auto done_loop = __ MakeLabel();
   __ Goto(&loop, __ IntPtrConstant(0));
   __ Bind(&loop);
   {
     // Check if we've initialized everything.
     Node* index = loop.PhiAt(0);
     Node* check = __ UintLessThan(index, limit);
-    __ GotoIfNot(check, &done_loop);
+    __ GotoIfNot(check, &done, result);
 
     // Storing "the_hole" doesn't need a write barrier.
     StoreRepresentation rep(MachineRepresentation::kTagged, kNoWriteBarrier);
@@ -2669,8 +2677,8 @@ Node* EffectControlLinearizer::LowerNewSmiOrObjectElements(Node* node) {
     __ Goto(&loop, index);
   }
 
-  __ Bind(&done_loop);
-  return result;
+  __ Bind(&done);
+  return done.PhiAt(0);
 }
 
 Node* EffectControlLinearizer::LowerNewArgumentsElements(Node* node) {

src/debug/debug.cc

@@ -1680,8 +1680,7 @@ Handle<FixedArray> Debug::GetLoadedScripts() {
       if (script->HasValidSource()) results->set(length++, script);
     }
   }
-  results->Shrink(length);
-  return results;
+  return FixedArray::ShrinkOrEmpty(results, length);
 }
 
 

src/elements.cc

@@ -1265,7 +1265,7 @@ class ElementsAccessorBase : public InternalElementsAccessor {
       // Shrink combined_keys to the final size.
       int final_size = nof_indices + nof_property_keys;
       DCHECK_LE(final_size, combined_keys->length());
-      combined_keys->Shrink(final_size);
+      return FixedArray::ShrinkOrEmpty(combined_keys, final_size);
     }
 
     return combined_keys;

src/heap/heap.cc

@@ -2859,9 +2859,11 @@ void Heap::RightTrimFixedArray(FixedArrayBase* object, int elements_to_trim) {
     bytes_to_trim = ByteArray::SizeFor(len) - new_size;
     DCHECK_GE(bytes_to_trim, 0);
   } else if (object->IsFixedArray()) {
+    CHECK_NE(elements_to_trim, len);
     bytes_to_trim = elements_to_trim * kPointerSize;
   } else {
     DCHECK(object->IsFixedDoubleArray());
+    CHECK_NE(elements_to_trim, len);
     bytes_to_trim = elements_to_trim * kDoubleSize;
   }
 

src/isolate.cc

@@ -866,8 +866,7 @@ Handle<FixedArray> Isolate::CaptureCurrentStackTrace(
       frames_seen++;
     }
   }
-  stack_trace_elems->Shrink(frames_seen);
-  return stack_trace_elems;
+  return FixedArray::ShrinkOrEmpty(stack_trace_elems, frames_seen);
 }
 
 

src/keys.cc

@@ -135,9 +135,7 @@ MaybeHandle<FixedArray> FilterProxyKeys(KeyAccumulator* accumulator,
     }
     store_position++;
   }
-  if (store_position == 0) return isolate->factory()->empty_fixed_array();
-  keys->Shrink(store_position);
-  return keys;
+  return FixedArray::ShrinkOrEmpty(keys, store_position);
 }
 
 // Returns "nothing" in case of exception, "true" on success.

src/objects-debug.cc

@@ -1094,6 +1094,9 @@ void JSArray::JSArrayVerify() {
   if (!ElementsAreSafeToExamine()) return;
   if (elements()->IsUndefined(isolate)) return;
   CHECK(elements()->IsFixedArray() || elements()->IsFixedDoubleArray());
+  if (elements()->length() == 0) {
+    CHECK_EQ(elements(), isolate->heap()->empty_fixed_array());
+  }
   if (!length()->IsNumber()) return;
   // Verify that the length and the elements backing store are in sync.
   if (length()->IsSmi() && HasFastElements()) {

src/objects.cc

@@ -8819,8 +8819,8 @@ V8_WARN_UNUSED_RESULT Maybe<bool> FastGetOwnValuesOrEntries(
     count++;
   }
 
-  if (count < values_or_entries->length()) values_or_entries->Shrink(count);
-  *result = values_or_entries;
+  DCHECK_LE(count, values_or_entries->length());
+  *result = FixedArray::ShrinkOrEmpty(values_or_entries, count);
   return Just(true);
 }
 
@@ -8878,8 +8878,8 @@ MaybeHandle<FixedArray> GetOwnValuesOrEntries(Isolate* isolate,
     values_or_entries->set(length, *value);
     length++;
   }
-  if (length < values_or_entries->length()) values_or_entries->Shrink(length);
-  return values_or_entries;
+  DCHECK_LE(length, values_or_entries->length());
+  return FixedArray::ShrinkOrEmpty(values_or_entries, length);
 }
 
 MaybeHandle<FixedArray> JSReceiver::GetOwnValues(Handle<JSReceiver> object,
@@ -10118,8 +10118,18 @@ bool FixedArray::ContainsSortedNumbers() {
   return true;
 }
 
+Handle<FixedArray> FixedArray::ShrinkOrEmpty(Handle<FixedArray> array,
+                                             int new_length) {
+  if (new_length == 0) {
+    return array->GetIsolate()->factory()->empty_fixed_array();
+  } else {
+    array->Shrink(new_length);
+    return array;
+  }
+}
+
 void FixedArray::Shrink(int new_length) {
-  DCHECK(0 <= new_length && new_length <= length());
+  DCHECK(0 < new_length && new_length <= length());
   if (new_length < length()) {
     GetHeap()->RightTrimFixedArray(this, length() - new_length);
   }
@@ -17984,8 +17994,7 @@ Handle<FixedArray> BaseNameDictionary<Derived, Shape>::IterationIndices(
             array->GetFirstElementAddress());
     std::sort(start, start + array_size, cmp);
   }
-  array->Shrink(array_size);
-  return array;
+  return FixedArray::ShrinkOrEmpty(array, array_size);
 }
 
 template <typename Derived, typename Shape>

src/objects/fixed-array.h

@@ -135,8 +135,12 @@ class FixedArray : public FixedArrayBase {
 
   inline void FillWithHoles(int from, int to);
 
-  // Shrink length and insert filler objects.
-  void Shrink(int length);
+  // Shrink the array and insert filler objects. {new_length} must be > 0.
+  void Shrink(int new_length);
+  // If {new_length} is 0, return the canonical empty FixedArray. Otherwise
+  // like above.
+  static Handle<FixedArray> ShrinkOrEmpty(Handle<FixedArray> array,
+                                          int new_length);
 
   // Copy a sub array from the receiver to dest.
   void CopyTo(int pos, FixedArray* dest, int dest_pos, int len) const;

src/objects/literal-objects.cc

@@ -384,10 +384,8 @@ class ObjectDescriptor {
     if (HasDictionaryProperties()) {
       properties_dictionary_template_->SetNextEnumerationIndex(
           next_enumeration_index_);
-
-      isolate->heap()->RightTrimFixedArray(
-          *computed_properties_,
-          computed_properties_->length() - current_computed_index_);
+      computed_properties_ = FixedArray::ShrinkOrEmpty(computed_properties_,
+                                                       current_computed_index_);
     } else {
       DCHECK(descriptor_array_template_->IsSortedNoDuplicates());
     }

src/objects/ordered-hash-table.cc

@@ -138,8 +138,7 @@ Handle<FixedArray> OrderedHashSet::ConvertToKeysArray(
     }
     result->set(i, key);
   }
-  result->Shrink(length);
-  return result;
+  return FixedArray::ShrinkOrEmpty(result, length);
 }
 
 HeapObject* OrderedHashSet::GetEmpty(Isolate* isolate) {

src/runtime/runtime-array.cc

@@ -500,10 +500,7 @@ RUNTIME_FUNCTION(Runtime_GetArrayKeys) {
     j++;
   }
 
-  if (j != keys->length()) {
-    isolate->heap()->RightTrimFixedArray(*keys, keys->length() - j);
-  }
-
+  keys = FixedArray::ShrinkOrEmpty(keys, j);
   return *isolate->factory()->NewJSArrayWithElements(keys);
 }
 

src/runtime/runtime-regexp.cc

@@ -1282,8 +1282,8 @@ static Object* SearchRegExpMultiple(Isolate* isolate, Handle<String> subject,
       for (int i = 0; i < capture_registers; i++) {
         last_match_cache->set(i, Smi::FromInt(last_match[i]));
       }
-      Handle<FixedArray> result_fixed_array = builder.array();
-      result_fixed_array->Shrink(builder.length());
+      Handle<FixedArray> result_fixed_array =
+          FixedArray::ShrinkOrEmpty(builder.array(), builder.length());
       // Cache the result and copy the FixedArray into a COW array.
       Handle<FixedArray> copied_fixed_array =
           isolate->factory()->CopyFixedArrayWithMap(
@@ -1561,8 +1561,8 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> ToUint32(Isolate* isolate,
 Handle<JSArray> NewJSArrayWithElements(Isolate* isolate,
                                        Handle<FixedArray> elems,
                                        int num_elems) {
-  elems->Shrink(num_elems);
-  return isolate->factory()->NewJSArrayWithElements(elems);
+  return isolate->factory()->NewJSArrayWithElements(
+      FixedArray::ShrinkOrEmpty(elems, num_elems));
 }
 
 }  // namespace

test/mjsunit/regress/regress-843062-1.js

@@ -0,0 +1,18 @@
+// Copyright 2018 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.
+
+var sparse_array = [];
+
+sparse_array[100] = 3;
+sparse_array[200] = undefined;
+sparse_array[300] = 4;
+sparse_array[400] = 5;
+sparse_array[500] = 6;
+sparse_array[600] = 5;
+sparse_array[700] = 4;
+sparse_array[800] = undefined;
+sparse_array[900] = 3
+sparse_array[41999] = "filler";
+
+sparse_array.lastIndexOf(3, 99);

test/mjsunit/regress/regress-843062-2.js

@@ -0,0 +1,8 @@
+// Copyright 2018 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.
+
+class C {
+  [1]() { return 'B'; }
+}
+Object.keys(C.prototype);

test/mjsunit/regress/regress-843062-3.js

@@ -0,0 +1,10 @@
+// Copyright 2018 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 bar(len) { return new Array(len); }
+bar(0);
+%OptimizeFunctionOnNextCall(bar);
+bar(0);