Improve elements validation and object printing

src/elements.cc

Index: src/elements.cc
diff --git a/src/elements.cc b/src/elements.cc
index 9ab70bbfaf692e6aa572320cd37515ba91c886e6..ff163f89b138d77bb548f3271937183049a04bbc 100644
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -1572,15 +1572,18 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
   static void ValidateContents(Handle<JSObject> holder, int length) {
 #if DEBUG
     Isolate* isolate = holder->GetIsolate();
+    Heap* heap = isolate->heap();
     HandleScope scope(isolate);
     Handle<FixedArrayBase> elements(holder->elements(), isolate);
     Map* map = elements->map();
-    DCHECK((IsFastSmiOrObjectElementsKind(KindTraits::Kind) &&
-            (map == isolate->heap()->fixed_array_map() ||
-             map == isolate->heap()->fixed_cow_array_map())) ||
-           (IsFastDoubleElementsKind(KindTraits::Kind) ==
-            ((map == isolate->heap()->fixed_array_map() && length == 0) ||
-             map == isolate->heap()->fixed_double_array_map())));
+    if (IsFastSmiOrObjectElementsKind(KindTraits::Kind)) {
+      DCHECK(map != heap->fixed_double_array_map());
+    } else if (IsFastDoubleElementsKind(KindTraits::Kind)) {
+      DCHECK(map != heap->fixed_cow_array_map());

[Toon Verwaest, 2016/04/07 13:01:39]

DCHECK_NE Would already be included in the suggested check below.

+      if (map == heap->fixed_array_map()) DCHECK_EQ(0, length);

[Toon Verwaest, 2016/04/07 13:01:39]

DCHECK(map == fixed_double_array_map() ||
       (map == fixed_array_map() && 0 == length)); ?

[Camillo Bruni, 2016/04/07 13:12:38]

I prefer the separate checks, makes it easier for debugging and adding more
checks.

+    } else {
+      UNREACHABLE();
+    }
     if (length == 0) return;  // nothing to do!
     DisallowHeapAllocation no_gc;
     Handle<BackingStore> backing_store = Handle<BackingStore>::cast(elements);
@@ -1590,6 +1593,13 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
                (IsFastHoleyElementsKind(KindTraits::Kind) &&
                 backing_store->is_the_hole(i)));
       }
+    } else if (KindTraits::Kind == FAST_ELEMENTS ||
+               KindTraits::Kind == FAST_DOUBLE_ELEMENTS) {
+      for (int i = 0; i < length; i++) {

[Toon Verwaest, 2016/04/07 13:01:39]

This probably should be SLOW_ASSERTS

[Camillo Bruni, 2016/04/07 13:12:38]

yup

+        DCHECK(!backing_store->is_the_hole(i));
+      }
+    } else {
+      DCHECK(IsFastHoleyElementsKind(KindTraits::Kind));
     }
 #endif
   }
@@ -1805,7 +1815,7 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
   static uint32_t AddArguments(Handle<JSArray> receiver,
                                Handle<FixedArrayBase> backing_store,
                                Arguments* args, uint32_t add_size,
-                               Where remove_position) {
+                               Where add_position) {
     uint32_t length = Smi::cast(receiver->length())->value();
     DCHECK(0 < add_size);
     uint32_t elms_len = backing_store->length();
@@ -1817,13 +1827,13 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
       // New backing storage is needed.
       uint32_t capacity = JSObject::NewElementsCapacity(new_length);
       // If we add arguments to the start we have to shift the existing objects.
-      int copy_dst_index = remove_position == AT_START ? add_size : 0;
+      int copy_dst_index = add_position == AT_START ? add_size : 0;
       // Copy over all objects to a new backing_store.
       backing_store = Subclass::ConvertElementsWithCapacity(
           receiver, backing_store, KindTraits::Kind, capacity, 0,
           copy_dst_index, ElementsAccessor::kCopyToEndAndInitializeToHole);
       receiver->set_elements(*backing_store);
-    } else if (remove_position == AT_START) {
+    } else if (add_position == AT_START) {
       // If the backing store has enough capacity and we add elements to the
       // start we have to shift the existing objects.
       Isolate* isolate = receiver->GetIsolate();
@@ -1831,7 +1841,7 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
                              length, 0, 0);
     }
 
-    int insertion_index = remove_position == AT_START ? 0 : length;
+    int insertion_index = add_position == AT_START ? 0 : length;
     // Copy the arguments to the start.
     Subclass::CopyArguments(args, backing_store, add_size, 1, insertion_index);
     // Set the length.
@@ -1847,8 +1857,9 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
     FixedArrayBase* raw_backing_store = *dst_store;
     WriteBarrierMode mode = raw_backing_store->GetWriteBarrierMode(no_gc);
     for (uint32_t i = 0; i < copy_size; i++) {
-      Object* argument = (*args)[i + src_index];
-      Subclass::SetImpl(raw_backing_store, i + dst_index, argument, mode);
+      Object* argument = (*args)[src_index + i];
+      DCHECK(!argument->IsTheHole());
+      Subclass::SetImpl(raw_backing_store, dst_index + i, argument, mode);
     }
   }
 };
@@ -2916,7 +2927,7 @@ MaybeHandle<Object> ArrayConstructInitializeElements(Handle<JSArray> array,
   }
 
   // Fill in the content
-  switch (array->GetElementsKind()) {
+  switch (elements_kind) {
     case FAST_HOLEY_SMI_ELEMENTS:
     case FAST_SMI_ELEMENTS: {
       Handle<FixedArray> smi_elms = Handle<FixedArray>::cast(elms);