Removed dangerous Factory::NewUninitializedFixedArray.

src/runtime.cc

 // Copyright 2006-2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 4198 matching lines @@
     while (right > left && IsTrimWhiteSpace(s->Get(right - 1))) {
       right--;
     }
   }
   return s->SubString(left, right);
 }
 
 
 // Copies ascii characters to the given fixed array looking up
 // one-char strings in the cache. Gives up on the first char that is
-// not in the cache. Returns the length of the successfully copied
-// prefix.
+// not in the cache and fills the remainder with smi zeros. Returns
+// the length of the successfully copied prefix.
 static int CopyCachedAsciiCharsToArray(const char* chars,
                                        FixedArray* elements,
                                        int length) {
   AssertNoAllocation nogc;
   FixedArray* ascii_cache = Heap::single_character_string_cache();
   Object* undefined = Heap::undefined_value();
-  for (int i = 0; i < length; ++i) {
+  int i;
+  for (i = 0; i < length; ++i) {
     Object* value = ascii_cache->get(chars[i]);
-    if (value == undefined) return i;
+    if (value == undefined) break;
     ASSERT(!Heap::InNewSpace(value));
     elements->set(i, value, SKIP_WRITE_BARRIER);
   }
-  return length;
+  if (i < length) {

[antonm, 2010/03/05 12:18:05]

maybe lift this filling into the loop itself:

if (value == undefined) {
  // fill the rest of array
  return
}

?

[Vitaly Repeshko, 2010/03/05 12:32:10]

I'd like to keep the fast loop as simple as possible to avoid confusing the poor
compiler.

+    ASSERT(kSmiTag == 0);
+    memset(elements->data_start() + i, 0, length - i);

[antonm, 2010/03/05 12:18:05]

not insisting, but something like Smi::FromInt(0) == 0 might be helpful for next
generations.

[Vitaly Repeshko, 2010/03/05 12:32:10]

Done.

+  }
+#ifdef DEBUG
+  for (int j = 0; j < length; ++j) {
+    Object* element = elements->get(j);
+    ASSERT(element == Smi::FromInt(0) ||
+           (element->IsString() && String::cast(element)->LooksValid()));
+  }
+#endif
+  return i;
 }
 
 
 // Converts a String to JSArray.
 // For example, "foo" => ["f", "o", "o"].
 static Object* Runtime_StringToArray(Arguments args) {
   HandleScope scope;
   ASSERT(args.length() == 1);
   CONVERT_ARG_CHECKED(String, s, 0);
 
   s->TryFlatten();
   const int length = s->length();
 
-  Handle<FixedArray> elements = Factory::NewUninitializedFixedArray(length);
-  if (s->IsFlat()) {
-    if (s->IsAsciiRepresentation()) {
-      Vector<const char> chars = s->ToAsciiVector();
-      int num_copied_from_cache = CopyCachedAsciiCharsToArray(chars.start(),
-                                                              *elements,
-                                                              length);
-      for (int i = num_copied_from_cache; i < length; ++i) {
-        elements->set(i, *LookupSingleCharacterStringFromCode(chars[i]));
-      }
-    } else {
-      ASSERT(s->IsTwoByteRepresentation());
-      Vector<const uc16> chars = s->ToUC16Vector();
-      for (int i = 0; i < length; ++i) {
-        elements->set(i, *LookupSingleCharacterStringFromCode(chars[i]));
-      }
+  Handle<FixedArray> elements;
+  if (s->IsFlat() && s->IsAsciiRepresentation()) {
+    Object* obj = Heap::AllocateUninitializedFixedArray(length);

[antonm, 2010/03/05 12:18:05]

maybe add a comment that uninitialized array will never leak into GC?

[Vitaly Repeshko, 2010/03/05 12:32:10]

Done.

+    if (obj->IsFailure()) return obj;
+    FixedArray* raw_elements = FixedArray::cast(obj);

[Mads Ager (chromium), 2010/03/05 12:17:16]

Let's put elements in a handle right away here and pass *elements to
CopyCachedAsciiCharsToArray.

[Vitaly Repeshko, 2010/03/05 12:32:10]

Done.

+
+    Vector<const char> chars = s->ToAsciiVector();
+    int num_copied_from_cache = CopyCachedAsciiCharsToArray(chars.start(),
+                                                            raw_elements,
+                                                            length);
+
+    elements = Handle<FixedArray>(raw_elements);
+    for (int i = num_copied_from_cache; i < length; ++i) {
+      elements->set(i, *LookupSingleCharacterStringFromCode(chars[i]));
     }
   } else {
+    elements = Factory::NewFixedArray(length);
     for (int i = 0; i < length; ++i) {
       elements->set(i, *LookupSingleCharacterStringFromCode(s->Get(i)));
     }
   }
 
 #ifdef DEBUG
   for (int i = 0; i < length; ++i) {
     ASSERT(String::cast(elements->get(i))->length() == 1);
   }
 #endif
@@ skipping 4271 matching lines @@
   } else {
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     Counters::gc_last_resort_from_js.Increment();
     Heap::CollectAllGarbage(false);
   }
 }
 
 
 } }  // namespace v8::internal