Handled return-value of SetElement in some cases, or avoided it in other.

SetElement can cause an exception to be thrown. If its return value
isn't checked, this exception might not be handled at the correct time.
In some cases, it's a matter of returning Exception::Failure() from
a runtime function.
In other cases, code using SetElement on a JSArray has been changed
to setting directly on a FixedArray and only creating the JSArray
at the end.

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7039 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/jsregexp.cc b/src/jsregexp.cc
index 8e7c35f582a54408685fc7447bd9e35971b8430e..b271b027f9c111bcddd380edce7d29590e29a5ec 100644 (file)
--- a/src/jsregexp.cc
+++ b/src/jsregexp.cc
@@ -97,9 +97,10 @@ static inline void ThrowRegExpException(Handle<JSRegExp> re,
                                         Handle<String> pattern,
                                         Handle<String> error_text,
                                         const char* message) {
-  Handle<JSArray> array = Factory::NewJSArray(2);
-  SetElement(array, 0, pattern);
-  SetElement(array, 1, error_text);
+  Handle<FixedArray> elements = Factory::NewFixedArray(2);
+  elements->set(0, *pattern);
+  elements->set(1, *error_text);
+  Handle<JSArray> array = Factory::NewJSArrayWithElements(elements);
   Handle<Object> regexp_err = Factory::NewSyntaxError(message, array);
   Top::Throw(*regexp_err);
 }
@@ -325,11 +326,12 @@ bool RegExpImpl::CompileIrregexp(Handle<JSRegExp> re, bool is_ascii) {
                             is_ascii);
   if (result.error_message != NULL) {
     // Unable to compile regexp.
-    Handle<JSArray> array = Factory::NewJSArray(2);
-    SetElement(array, 0, pattern);
-    SetElement(array,
-               1,
-               Factory::NewStringFromUtf8(CStrVector(result.error_message)));
+    Handle<FixedArray> elements = Factory::NewFixedArray(2);
+    elements->set(0, *pattern);
+    Handle<String> error_message =
+        Factory::NewStringFromUtf8(CStrVector(result.error_message));
+    elements->set(1, *error_message);
+    Handle<JSArray> array = Factory::NewJSArrayWithElements(elements);
     Handle<Object> regexp_err =
         Factory::NewSyntaxError("malformed_regexp", array);
     Top::Throw(*regexp_err);

diff --git a/src/objects.cc b/src/objects.cc
index 0b7f60a908f07f45efe0b2a48c26e1362f74a76d..0975a0364138bb6708a8326269ed9abeab90f425 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -6522,13 +6522,6 @@ void JSArray::Expand(int required_size) {
 }
 
 
-// Computes the new capacity when expanding the elements of a JSObject.
-static int NewElementsCapacity(int old_capacity) {
-  // (old_capacity + 50%) + 16
-  return old_capacity + (old_capacity >> 1) + 16;
-}
-
-
 static Failure* ArrayLengthRangeError() {
   HandleScope scope;
   return Top::Throw(*Factory::NewRangeError("invalid_array_length",

diff --git a/src/objects.h b/src/objects.h
index de15a7398d7fd8c86242247e778a724153cef8a7..e9e9f635c0b26b5b1b33dce74aaf3c5258bb6c95 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -1515,6 +1515,12 @@ class JSObject: public HeapObject {
   inline bool HasElement(uint32_t index);
   bool HasElementWithReceiver(JSObject* receiver, uint32_t index);
 
+  // Computes the new capacity when expanding the elements of a JSObject.
+  static int NewElementsCapacity(int old_capacity) {
+    // (old_capacity + 50%) + 16
+    return old_capacity + (old_capacity >> 1) + 16;
+  }
+
   // Tells whether the index'th element is present and how it is stored.
   enum LocalElementType {
     // There is no element with given index.

diff --git a/src/parser.cc b/src/parser.cc
index 856031070209f43a469bdc7420b5aac02654a9e5..c8b921b4b1da143868b5a539a641948ec536f90d 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -803,10 +803,12 @@ void Parser::ReportMessageAt(Scanner::Location source_location,
   MessageLocation location(script_,
                            source_location.beg_pos,
                            source_location.end_pos);
-  Handle<JSArray> array = Factory::NewJSArray(args.length());
+  Handle<FixedArray> elements = Factory::NewFixedArray(args.length());
   for (int i = 0; i < args.length(); i++) {
-    SetElement(array, i, Factory::NewStringFromUtf8(CStrVector(args[i])));
+    Handle<String> arg_string = Factory::NewStringFromUtf8(CStrVector(args[i]));
+    elements->set(i, *arg_string);
   }
+  Handle<JSArray> array = Factory::NewJSArrayWithElements(elements);
   Handle<Object> result = Factory::NewSyntaxError(type, array);
   Top::Throw(*result, &location);
 }
@@ -818,10 +820,11 @@ void Parser::ReportMessageAt(Scanner::Location source_location,
   MessageLocation location(script_,
                            source_location.beg_pos,
                            source_location.end_pos);
-  Handle<JSArray> array = Factory::NewJSArray(args.length());
+  Handle<FixedArray> elements = Factory::NewFixedArray(args.length());
   for (int i = 0; i < args.length(); i++) {
-    SetElement(array, i, args[i]);
+    elements->set(i, *args[i]);
   }
+  Handle<JSArray> array = Factory::NewJSArrayWithElements(elements);
   Handle<Object> result = Factory::NewSyntaxError(type, array);
   Top::Throw(*result, &location);
 }
@@ -4035,12 +4038,14 @@ Handle<Object> JsonParser::ParseJson(Handle<String> script,
       MessageLocation location(Factory::NewScript(script),
                                source_location.beg_pos,
                                source_location.end_pos);
-      int argc = (name_opt == NULL) ? 0 : 1;
-      Handle<JSArray> array = Factory::NewJSArray(argc);
-      if (name_opt != NULL) {
-        SetElement(array,
-                   0,
-                   Factory::NewStringFromUtf8(CStrVector(name_opt)));
+      Handle<JSArray> array;
+      if (name_opt == NULL) {
+        array = Factory::NewJSArray(0);
+      } else {
+        Handle<String> name = Factory::NewStringFromUtf8(CStrVector(name_opt));
+        Handle<FixedArray> element = Factory::NewFixedArray(1);
+        element->set(0, *name);
+        array = Factory::NewJSArrayWithElements(element);
       }
       Handle<Object> result = Factory::NewSyntaxError(message, array);
       Top::Throw(*result, &location);

diff --git a/src/runtime.cc b/src/runtime.cc
index 0c15f60f30677bc02c172b4f6929e5ac559273cd..f7b6ad4487cafc7cca2123ca7f331e87b802e795 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -789,6 +789,7 @@ static MaybeObject* Runtime_GetOwnProperty(Arguments args) {
       case JSObject::FAST_ELEMENT: {
         elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
         Handle<Object> value = GetElement(obj, index);
+        RETURN_IF_EMPTY_HANDLE(value);
         elms->set(VALUE_INDEX, *value);
         elms->set(WRITABLE_INDEX, Heap::true_value());
         elms->set(ENUMERABLE_INDEX,  Heap::true_value());
@@ -826,6 +827,7 @@ static MaybeObject* Runtime_GetOwnProperty(Arguments args) {
             // This is a data property.
             elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
             Handle<Object> value = GetElement(obj, index);
+            ASSERT(!value.is_null());
             elms->set(VALUE_INDEX, *value);
             elms->set(WRITABLE_INDEX, Heap::ToBoolean(!details.IsReadOnly()));
             break;
@@ -1469,7 +1471,7 @@ static MaybeObject* Runtime_InitializeConstContextSlot(Arguments args) {
       // The holder is an arguments object.
       ASSERT((attributes & READ_ONLY) == 0);
       Handle<JSObject> arguments(Handle<JSObject>::cast(holder));
-      SetElement(arguments, index, value);
+      RETURN_IF_EMPTY_HANDLE(SetElement(arguments, index, value));
     }
     return *value;
   }
@@ -8384,8 +8386,9 @@ static void CollectElementIndices(Handle<JSObject> object,
  * with the element index and the element's value.
  * Afterwards it increments the base-index of the visitor by the array
  * length.
+ * Returns false if any access threw an exception, otherwise true.
  */
-static void IterateElements(Handle<JSArray> receiver,
+static bool IterateElements(Handle<JSArray> receiver,
                             ArrayConcatVisitor* visitor) {
   uint32_t length = static_cast<uint32_t>(receiver->length()->Number());
   switch (receiver->GetElementsKind()) {
@@ -8404,6 +8407,7 @@ static void IterateElements(Handle<JSArray> receiver,
           // Call GetElement on receiver, not its prototype, or getters won't
           // have the correct receiver.
           element_value = GetElement(receiver, j);
+          if (element_value.is_null()) return false;
           visitor->visit(j, element_value);
         }
       }
@@ -8422,6 +8426,7 @@ static void IterateElements(Handle<JSArray> receiver,
         HandleScope loop_scope;
         uint32_t index = indices[j];
         Handle<Object> element = GetElement(receiver, index);
+        if (element.is_null()) return false;
         visitor->visit(index, element);
         // Skip to next different index (i.e., omit duplicates).
         do {
@@ -8478,6 +8483,7 @@ static void IterateElements(Handle<JSArray> receiver,
       break;
   }
   visitor->increase_index_offset(length);
+  return true;
 }
 
 
@@ -8559,7 +8565,9 @@ static MaybeObject* Runtime_ArrayConcat(Arguments args) {
     Handle<Object> obj(elements->get(i));
     if (obj->IsJSArray()) {
       Handle<JSArray> array = Handle<JSArray>::cast(obj);
-      IterateElements(array, &visitor);
+      if (!IterateElements(array, &visitor)) {
+        return Failure::Exception();
+      }
     } else {
       visitor.visit(0, obj);
       visitor.increase_index_offset(1);
@@ -8657,10 +8665,12 @@ static MaybeObject* Runtime_SwapElements(Arguments args) {
 
   Handle<JSObject> jsobject = Handle<JSObject>::cast(object);
   Handle<Object> tmp1 = GetElement(jsobject, index1);
+  RETURN_IF_EMPTY_HANDLE(tmp1);
   Handle<Object> tmp2 = GetElement(jsobject, index2);
+  RETURN_IF_EMPTY_HANDLE(tmp2);
 
-  SetElement(jsobject, index1, tmp2);
-  SetElement(jsobject, index2, tmp1);
+  RETURN_IF_EMPTY_HANDLE(SetElement(jsobject, index1, tmp2));
+  RETURN_IF_EMPTY_HANDLE(SetElement(jsobject, index2, tmp1));
 
   return Heap::undefined_value();
 }
@@ -11266,7 +11276,8 @@ static MaybeObject* Runtime_CollectStackTrace(Arguments args) {
 
   limit = Max(limit, 0);  // Ensure that limit is not negative.
   int initial_size = Min(limit, 10);
-  Handle<JSArray> result = Factory::NewJSArray(initial_size * 4);
+  Handle<FixedArray> elements =
+      Factory::NewFixedArrayWithHoles(initial_size * 4);
 
   StackFrameIterator iter;
   // If the caller parameter is a function we skip frames until we're
@@ -11282,27 +11293,30 @@ static MaybeObject* Runtime_CollectStackTrace(Arguments args) {
       List<FrameSummary> frames(3);  // Max 2 levels of inlining.
       frame->Summarize(&frames);
       for (int i = frames.length() - 1; i >= 0; i--) {
+        if (cursor + 4 > elements->length()) {
+          int new_capacity = JSObject::NewElementsCapacity(elements->length());
+          Handle<FixedArray> new_elements =
+              Factory::NewFixedArrayWithHoles(new_capacity);
+          for (int i = 0; i < cursor; i++) {
+            new_elements->set(i, elements->get(i));
+          }
+          elements = new_elements;
+        }
+        ASSERT(cursor + 4 <= elements->length());
+
         Handle<Object> recv = frames[i].receiver();
         Handle<JSFunction> fun = frames[i].function();
         Handle<Code> code = frames[i].code();
         Handle<Smi> offset(Smi::FromInt(frames[i].offset()));
-        FixedArray* elements = FixedArray::cast(result->elements());
-        if (cursor + 3 < elements->length()) {
-          elements->set(cursor++, *recv);
-          elements->set(cursor++, *fun);
-          elements->set(cursor++, *code);
-          elements->set(cursor++, *offset);
-        } else {
-          SetElement(result, cursor++, recv);
-          SetElement(result, cursor++, fun);
-          SetElement(result, cursor++, code);
-          SetElement(result, cursor++, offset);
-        }
+        elements->set(cursor++, *recv);
+        elements->set(cursor++, *fun);
+        elements->set(cursor++, *code);
+        elements->set(cursor++, *offset);
       }
     }
     iter.Advance();
   }
-
+  Handle<JSArray> result = Factory::NewJSArrayWithElements(elements);
   result->set_length(Smi::FromInt(cursor));
   return *result;
 }
@@ -11467,7 +11481,13 @@ static MaybeObject* Runtime_MessageGetScript(Arguments args) {
 static MaybeObject* Runtime_ListNatives(Arguments args) {
   ASSERT(args.length() == 0);
   HandleScope scope;
-  Handle<JSArray> result = Factory::NewJSArray(0);
+#define COUNT_ENTRY(Name, argc, ressize) + 1
+  int entry_count = 0
+      RUNTIME_FUNCTION_LIST(COUNT_ENTRY)
+      INLINE_FUNCTION_LIST(COUNT_ENTRY)
+      INLINE_RUNTIME_FUNCTION_LIST(COUNT_ENTRY);
+#undef COUNT_ENTRY
+  Handle<FixedArray> elements = Factory::NewFixedArray(entry_count);
   int index = 0;
   bool inline_runtime_functions = false;
 #define ADD_ENTRY(Name, argc, ressize)                                       \
@@ -11482,10 +11502,11 @@ static MaybeObject* Runtime_ListNatives(Arguments args) {
       name = Factory::NewStringFromAscii(                                    \
           Vector<const char>(#Name, StrLength(#Name)));                      \
     }                                                                        \
-    Handle<JSArray> pair = Factory::NewJSArray(0);                           \
-    SetElement(pair, 0, name);                                               \
-    SetElement(pair, 1, Handle<Smi>(Smi::FromInt(argc)));                    \
-    SetElement(result, index++, pair);                                       \
+    Handle<FixedArray> pair_elements = Factory::NewFixedArray(2);            \
+    pair_elements->set(0, *name);                                            \
+    pair_elements->set(1, Smi::FromInt(argc));                               \
+    Handle<JSArray> pair = Factory::NewJSArrayWithElements(pair_elements);   \
+    elements->set(index++, *pair);                                           \
   }
   inline_runtime_functions = false;
   RUNTIME_FUNCTION_LIST(ADD_ENTRY)
@@ -11493,6 +11514,8 @@ static MaybeObject* Runtime_ListNatives(Arguments args) {
   INLINE_FUNCTION_LIST(ADD_ENTRY)
   INLINE_RUNTIME_FUNCTION_LIST(ADD_ENTRY)
 #undef ADD_ENTRY
+  ASSERT_EQ(index, entry_count);
+  Handle<JSArray> result = Factory::NewJSArrayWithElements(elements);
   return *result;
 }
 #endif