Removed dangerous Factory::NewUninitializedFixedArray.

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 439d8381867372bb54a8a81f9d99c7de709d7885..d13a55d4fd0995977e3eda0be6fd9faa1bdb330f 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -4216,21 +4216,33 @@ static Object* Runtime_StringTrim(Arguments args) {
 
 // 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;
 }
 
 
@@ -4244,24 +4256,23 @@ static Object* Runtime_StringToArray(Arguments args) {
   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)));
     }