[runtime] making heap verification more aggressive

- check that packed elements do not contain the_hole (with fix)
- verify argument objects with elements kind
- use JSObjectVerifiy in all JSObject "subclasses"
- change initialization order for ArrayLiteralBoilerplate to simplify verification

BUG=v8:5188

Review-Url: https://codereview.chromium.org/2126613002
Cr-Commit-Position: refs/heads/master@{#37680}

src/factory.cc

@@ -1647,19 +1647,29 @@ void Factory::NewJSArrayStorage(Handle<JSArray> array,
   Handle<FixedArrayBase> elms;
   ElementsKind elements_kind = array->GetElementsKind();
   if (IsFastDoubleElementsKind(elements_kind)) {
-    if (mode == DONT_INITIALIZE_ARRAY_ELEMENTS) {
-      elms = NewFixedDoubleArray(capacity);
-    } else {
-      DCHECK(mode == INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
-      elms = NewFixedDoubleArrayWithHoles(capacity);
+    switch (mode) {
+      case DONT_INITIALIZE_ARRAY_ELEMENTS:
+        elms = NewFixedDoubleArray(capacity);
+        break;
+      case INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE:
+        elms = NewFixedDoubleArrayWithHoles(capacity);
+        break;
+      case INITIALIZE_ARRAY_ELEMENTS_WITH_UNDEFINED:
+        UNREACHABLE();
+        break;
     }
   } else {
     DCHECK(IsFastSmiOrObjectElementsKind(elements_kind));
-    if (mode == DONT_INITIALIZE_ARRAY_ELEMENTS) {
-      elms = NewUninitializedFixedArray(capacity);
-    } else {
-      DCHECK(mode == INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
-      elms = NewFixedArrayWithHoles(capacity);
+    switch (mode) {
+      case DONT_INITIALIZE_ARRAY_ELEMENTS:
+        elms = NewUninitializedFixedArray(capacity);
+        break;
+      case INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE:
+        elms = NewFixedArrayWithHoles(capacity);
+        break;
+      case INITIALIZE_ARRAY_ELEMENTS_WITH_UNDEFINED:
+        elms = NewFixedArray(capacity);
+        break;
     }
   }
 

src/heap/heap.h

@@ -441,10 +441,10 @@ class PromotionQueue {
   DISALLOW_COPY_AND_ASSIGN(PromotionQueue);
 };
 
-
 enum ArrayStorageAllocationMode {
   DONT_INITIALIZE_ARRAY_ELEMENTS,
-  INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE
+  INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE,
+  INITIALIZE_ARRAY_ELEMENTS_WITH_UNDEFINED
 };
 
 enum class ClearRecordedSlots { kYes, kNo };

src/objects-inl.h

@@ -796,6 +796,7 @@ TYPE_CHECKER(Cell, CELL_TYPE)
 TYPE_CHECKER(PropertyCell, PROPERTY_CELL_TYPE)
 TYPE_CHECKER(WeakCell, WEAK_CELL_TYPE)
 TYPE_CHECKER(SharedFunctionInfo, SHARED_FUNCTION_INFO_TYPE)
+TYPE_CHECKER(JSArgumentsObject, JS_ARGUMENTS_TYPE)
 TYPE_CHECKER(JSDate, JS_DATE_TYPE)
 TYPE_CHECKER(JSError, JS_ERROR_TYPE)
 TYPE_CHECKER(JSGeneratorObject, JS_GENERATOR_OBJECT_TYPE)
@@ -2355,8 +2356,8 @@ void FixedArray::set(int index, Object* value) {
 
 
 double FixedDoubleArray::get_scalar(int index) {
-  DCHECK(map() != GetHeap()->fixed_cow_array_map() &&
-         map() != GetHeap()->fixed_array_map());
+  DCHECK_NE(GetHeap()->fixed_cow_array_map(), map());
+  DCHECK_NE(GetHeap()->fixed_array_map(), map());
   DCHECK(index >= 0 && index < this->length());
   DCHECK(!is_the_hole(index));
   return READ_DOUBLE_FIELD(this, kHeaderSize + index * kDoubleSize);
@@ -2364,8 +2365,8 @@ double FixedDoubleArray::get_scalar(int index) {
 
 
 uint64_t FixedDoubleArray::get_representation(int index) {
-  DCHECK(map() != GetHeap()->fixed_cow_array_map() &&
-         map() != GetHeap()->fixed_array_map());
+  DCHECK_NE(GetHeap()->fixed_cow_array_map(), map());
+  DCHECK_NE(GetHeap()->fixed_array_map(), map());
   DCHECK(index >= 0 && index < this->length());
   int offset = kHeaderSize + index * kDoubleSize;
   return READ_UINT64_FIELD(this, offset);
@@ -3207,6 +3208,7 @@ CAST_ACCESSOR(HeapObject)
 CAST_ACCESSOR(Int16x8)
 CAST_ACCESSOR(Int32x4)
 CAST_ACCESSOR(Int8x16)
+CAST_ACCESSOR(JSArgumentsObject)
 CAST_ACCESSOR(JSArray)
 CAST_ACCESSOR(JSArrayBuffer)
 CAST_ACCESSOR(JSArrayBufferView)
@@ -6880,30 +6882,6 @@ void JSRegExp::SetDataAt(int index, Object* value) {
 
 ElementsKind JSObject::GetElementsKind() {
   ElementsKind kind = map()->elements_kind();
-#if VERIFY_HEAP && DEBUG
-  FixedArrayBase* fixed_array =
-      reinterpret_cast<FixedArrayBase*>(READ_FIELD(this, kElementsOffset));
-
-  // If a GC was caused while constructing this object, the elements
-  // pointer may point to a one pointer filler map.
-  if (ElementsAreSafeToExamine()) {
-    Map* map = fixed_array->map();
-    if (IsFastSmiOrObjectElementsKind(kind)) {
-      DCHECK(map == GetHeap()->fixed_array_map() ||
-             map == GetHeap()->fixed_cow_array_map());
-    } else if (IsFastDoubleElementsKind(kind)) {
-      DCHECK(fixed_array->IsFixedDoubleArray() ||
-             fixed_array == GetHeap()->empty_fixed_array());
-    } else if (kind == DICTIONARY_ELEMENTS) {
-      DCHECK(fixed_array->IsFixedArray());
-      DCHECK(fixed_array->IsDictionary());
-    } else {
-      DCHECK(kind > DICTIONARY_ELEMENTS);
-    }
-    DCHECK(!IsSloppyArgumentsElements(kind) ||
-           (elements()->IsFixedArray() && elements()->length() >= 2));
-  }
-#endif
   return kind;
 }
 

src/objects.h

@@ -962,6 +962,7 @@ template <class C> inline bool Is(Object* obj);
   V(StringWrapper)               \
   V(Foreign)                     \
   V(Boolean)                     \
+  V(JSArgumentsObject)           \
   V(JSArray)                     \
   V(JSArrayBuffer)               \
   V(JSArrayBufferView)           \
@@ -2578,6 +2579,9 @@ class JSArgumentsObject: public JSObject {
   // Indices of in-object properties.
   static const int kLengthIndex = 0;
 
+  DECLARE_VERIFIER(JSArgumentsObject)
+  DECLARE_CAST(JSArgumentsObject)
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSArgumentsObject);
 };
@@ -2593,6 +2597,8 @@ class JSSloppyArgumentsObject: public JSArgumentsObject {
   // Indices of in-object properties.
   static const int kCalleeIndex = 1;
 
+  DECLARE_CAST(JSSloppyArgumentsObject)
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSSloppyArgumentsObject);
 };
@@ -2605,6 +2611,8 @@ class JSStrictArgumentsObject: public JSArgumentsObject {
   // Offsets of object fields.
   static const int kSize = JSArgumentsObject::kHeaderSize;
 
+  DECLARE_CAST(JSStrictArgumentsObject)
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSStrictArgumentsObject);
 };

src/runtime/runtime-literals.cc

@@ -114,30 +114,13 @@ MUST_USE_RESULT static MaybeHandle<Object> CreateObjectLiteralBoilerplate(
 static MaybeHandle<Object> CreateArrayLiteralBoilerplate(
     Isolate* isolate, Handle<LiteralsArray> literals,
     Handle<FixedArray> elements) {
-  // Create the JSArray.
-  Handle<JSFunction> constructor = isolate->array_function();
-
-  PretenureFlag pretenure_flag =
-      isolate->heap()->InNewSpace(*literals) ? NOT_TENURED : TENURED;
-
-  Handle<JSArray> object = Handle<JSArray>::cast(
-      isolate->factory()->NewJSObject(constructor, pretenure_flag));
-
   ElementsKind constant_elements_kind =
       static_cast<ElementsKind>(Smi::cast(elements->get(0))->value());
+  DCHECK(IsFastElementsKind(constant_elements_kind));
   Handle<FixedArrayBase> constant_elements_values(
       FixedArrayBase::cast(elements->get(1)));
-
-  {
-    DisallowHeapAllocation no_gc;
-    DCHECK(IsFastElementsKind(constant_elements_kind));
-    Context* native_context = isolate->context()->native_context();
-    Object* map =
-        native_context->get(Context::ArrayMapIndex(constant_elements_kind));
-    object->set_map(Map::cast(map));
-  }
-
   Handle<FixedArrayBase> copied_elements_values;
+
   if (IsFastDoubleElementsKind(constant_elements_kind)) {
     copied_elements_values = isolate->factory()->CopyFixedDoubleArray(
         Handle<FixedDoubleArray>::cast(constant_elements_values));
@@ -176,9 +159,12 @@ static MaybeHandle<Object> CreateArrayLiteralBoilerplate(
           });
     }
   }
-  object->set_elements(*copied_elements_values);
-  object->set_length(Smi::FromInt(copied_elements_values->length()));
-
+  // Create the JSArray.
+  PretenureFlag pretenure_flag =
+      isolate->heap()->InNewSpace(*literals) ? NOT_TENURED : TENURED;
+  Handle<JSArray> object = isolate->factory()->NewJSArrayWithElements(
+      copied_elements_values, constant_elements_kind,
+      copied_elements_values->length(), pretenure_flag);
   JSObject::ValidateElements(object);
   return object;
 }

src/runtime/runtime-regexp.cc

@@ -734,7 +734,7 @@ RUNTIME_FUNCTION(Runtime_StringSplit) {
 
   Handle<JSArray> result =
       isolate->factory()->NewJSArray(FAST_ELEMENTS, part_count, part_count,
-                                     INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
+                                     INITIALIZE_ARRAY_ELEMENTS_WITH_UNDEFINED);
 
   DCHECK(result->HasFastObjectElements());
 

test/mjsunit/regress/regress-undefined-nan.js

@@ -3,11 +3,6 @@
 // found in the LICENSE file.
 
 // Flags: --allow-natives-syntax
-
-function loader(dst, src, i) {
-  dst[i] = src[i];
-}
-
 var ab = new ArrayBuffer(8);
 var i_view = new Int32Array(ab);
 i_view[0] = %GetHoleNaNUpper()
@@ -26,10 +21,20 @@ opt_store();
 var i32 = new Int32Array(fixed_double_elements.buffer);
 assertEquals(i_view[0], i32[0]);
 assertEquals(i_view[1], i32[1]);
+assertTrue(isNaN(f_view [0]));
+
+
+function loader(dst, src, i) {
+  dst[i] = src[i];
+}
 
 var doubles = [0.5];
 loader(doubles, fixed_double_elements, 0);
 loader(doubles, fixed_double_elements, 0);
+assertTrue(doubles[0] !== undefined);
+assertTrue(isNaN(doubles[0]));
+
 %OptimizeFunctionOnNextCall(loader);
 loader(doubles, fixed_double_elements, 0);
 assertTrue(doubles[0] !== undefined);
+assertTrue(isNaN(doubles[0]));