Use proper ToLength() operation in %ArrayConcat()

src/runtime/runtime-array.cc

Index: src/runtime/runtime-array.cc
diff --git a/src/runtime/runtime-array.cc b/src/runtime/runtime-array.cc
index 1df42c424c33f9ee06112b0e80d241b3ca0390f4..b72a23a48018be48d87804b44ddba917bbe02e92 100644
--- a/src/runtime/runtime-array.cc
+++ b/src/runtime/runtime-array.cc
@@ -461,8 +461,12 @@ static bool IterateElements(Isolate* isolate, Handle<JSObject> receiver,
     Handle<Object> key(isolate->heap()->length_string(), isolate);
     ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, val,
         Runtime::GetObjectProperty(isolate, receiver, key), false);
-    // TODO(caitp): implement ToLength() abstract operation for C++
-    val->ToUint32(&length);
+    // TODO(caitp): Support larger element indexes (up to 2^53-1).
+    if (!val->ToUint32(&length)) {
+      ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, val,
+          Execution::ToLength(isolate, val), false);
+      val->ToUint32(&length);

[Dmitry Lomov (no reviews), 2014/12/15 07:23:51]

This is not good. 
If ToUInt32 returns false, `length` is uninitialized, but you proceed as normal.
No idea what the code below would do.
It is ok to not support large lengths, but fail gracefully, do not do random
things. 

[caitp (gmail), 2014/12/15 14:11:00]

I don't think it can return false here, because `val` is (I think) guaranteed to
be a number here. In either case, `length` is initialized to 0 before any of
this happens, so if it does return false and doesn't initialize `length`, it's a
zero value.

[Dmitry Lomov (no reviews), 2014/12/15 14:18:20]

No, ToString calls user code, so anything can happen.
Good point about length 0 though.

+    }
   }
 
   switch (receiver->GetElementsKind()) {