[fastcall] DCHECK failure in IsNumber() in objects-inl.h

Function CopyAndConvertArrayToCppBuffer doesn't work correctly with
holey arrays.

Bug: chromium:1230431, v8:11739
Change-Id: If24f645ebf4947f9efdccd1470ddf9e68c0b6780
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3037989Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#75823}

src/api/api-inl.h

@@ -281,15 +281,16 @@ bool CopyAndConvertArrayToCppBuffer(Local<Array> src, T* dst,
   i::JSArray obj = *reinterpret_cast<i::JSArray*>(*src);
 
   i::FixedArrayBase elements = obj.elements();
-  if (obj.HasSmiElements()) {
-    CopySmiElementsToTypedBuffer(dst, length, i::FixedArray::cast(elements));
-    return true;
-  } else if (obj.HasDoubleElements()) {
-    CopyDoubleElementsToTypedBuffer(dst, length,
-                                    i::FixedDoubleArray::cast(elements));
-    return true;
-  } else {
-    return false;
+  switch (obj.GetElementsKind()) {
+    case i::PACKED_SMI_ELEMENTS:
+      CopySmiElementsToTypedBuffer(dst, length, i::FixedArray::cast(elements));
+      return true;
+    case i::PACKED_DOUBLE_ELEMENTS:
+      CopyDoubleElementsToTypedBuffer(dst, length,
+                                      i::FixedDoubleArray::cast(elements));
+      return true;
+    default:
+      return false;
   }
 }
 

src/d8/d8-test.cc

@@ -166,18 +166,25 @@ class FastCApiObject {
           "Invalid length of array, must be between 0 and 1024.");
       return;
     }
-    Type buffer[1024];
-    bool result =
-        CopyAndConvertArrayToCppBuffer<&type_info, Type>(seq_arg, buffer, 1024);
-    if (!result) {
-      isolate->ThrowError("Array conversion unsuccessful.");
-      return;
-    }
-    DCHECK_EQ(seq_arg->Length(), length);
 
     Type sum = 0;
     for (uint32_t i = 0; i < length; ++i) {
-      sum += buffer[i];
+      v8::Local<v8::Value> element =
+          seq_arg
+              ->Get(isolate->GetCurrentContext(),
+                    v8::Integer::NewFromUnsigned(isolate, i))
+              .ToLocalChecked();
+      if (element->IsNumber()) {
+        double value = element->ToNumber(isolate->GetCurrentContext())
+                           .ToLocalChecked()
+                           ->Value();
+        sum += value;
+      } else if (element->IsUndefined()) {
+        // Hole: ignore the element.
+      } else {
+        isolate->ThrowError("unexpected element type in JSArray");
+        return;
+      }
     }
     args.GetReturnValue().Set(Number::New(isolate, sum));
   }

test/mjsunit/compiler/fast-api-sequences.js

@@ -62,6 +62,23 @@ if (fast_c_api.supports_fp_params) {
   assertEquals(0, fast_c_api.slow_call_count());
 }
 
+// Test holey arrays
+fast_c_api.reset_counts();
+if (fast_c_api.supports_fp_params) {
+  // Test that regular call hits the fast path.
+  assertEquals(add_all_result_full, add_all_sequence_full([, ...full_array]));
+  assertOptimized(add_all_sequence_full);
+  assertEquals(1, fast_c_api.fast_call_count());
+  assertEquals(1, fast_c_api.slow_call_count());
+} else {
+  // Smi only test - regular call hits the fast path.
+  assertEquals(3, add_all_sequence_smi([-42, , 45]));
+  assertOptimized(add_all_sequence_smi);
+  assertEquals(1, fast_c_api.fast_call_count());
+  assertEquals(1, fast_c_api.slow_call_count());
+}
+
+
 function add_all_sequence_mismatch(arg) {
   return fast_c_api.add_all_sequence(false /*should_fallback*/, arg);
 }