From 9ce2f728d51d41bc2e3eb817a72fc329bb2b81ea Mon Sep 17 00:00:00 2001
From: "vitalyr@chromium.org"
 <vitalyr@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Fri, 5 Mar 2010 12:30:59 +0000
Subject: [PATCH] Removed dangerous Factory::NewUninitializedFixedArray.

This was used in runtime StringToArray function which I simplified
keeping its performance for ascii strings.

Review URL: http://codereview.chromium.org/669156

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4035 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/factory.cc |  6 ------
 src/factory.h  |  4 ----
 src/runtime.cc | 54 ++++++++++++++++++++++++++++++--------------------
 3 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/src/factory.cc b/src/factory.cc
index fa3790c4a..8d2074964 100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -43,12 +43,6 @@ Handle<FixedArray> Factory::NewFixedArray(int size, PretenureFlag pretenure) {
 }
 
 
-Handle<FixedArray> Factory::NewUninitializedFixedArray(int size) {
-  ASSERT(0 <= size);
-  CALL_HEAP_FUNCTION(Heap::AllocateUninitializedFixedArray(size), FixedArray);
-}
-
-
 Handle<FixedArray> Factory::NewFixedArrayWithHoles(int size) {
   ASSERT(0 <= size);
   CALL_HEAP_FUNCTION(Heap::AllocateFixedArrayWithHoles(size), FixedArray);
diff --git a/src/factory.h b/src/factory.h
index 74f492b6e..2a347cd6f 100644
--- a/src/factory.h
+++ b/src/factory.h
@@ -45,10 +45,6 @@ class Factory : public AllStatic {
       int size,
       PretenureFlag pretenure = NOT_TENURED);
 
-  // Allocate a new uninitialized fixed array. It must be filled by
-  // the caller.
-  static Handle<FixedArray> NewUninitializedFixedArray(int size);
-
   // Allocate a new fixed array with non-existing entries (the hole).
   static Handle<FixedArray> NewFixedArrayWithHoles(int size);
 
diff --git a/src/runtime.cc b/src/runtime.cc
index 439d83818..d0f90818d 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) {
+    ASSERT(Smi::FromInt(0) == 0);
+    memset(elements->data_start() + i, 0, length - i);
+  }
+#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,24 @@ 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);
+    if (obj->IsFailure()) return obj;
+    elements = Handle<FixedArray>(FixedArray::cast(obj));
+
+    Vector<const char> chars = s->ToAsciiVector();
+    // Note, this will initialize all elements (not only the prefix)
+    // to prevent GC from seeing partially initialized array.
+    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 {
+    elements = Factory::NewFixedArray(length);
     for (int i = 0; i < length; ++i) {
       elements->set(i, *LookupSingleCharacterStringFromCode(s->Get(i)));
     }
-- 
2.34.1