Revision: 7039
Author: [email protected]
Date: Thu Mar 3 02:16:22 2011
Log: 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
http://code.google.com/p/v8/source/detail?r=7039
Modified:
/branches/bleeding_edge/src/jsregexp.cc
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/runtime.cc
=======================================
--- /branches/bleeding_edge/src/jsregexp.cc Wed Jan 12 03:56:41 2011
+++ /branches/bleeding_edge/src/jsregexp.cc Thu Mar 3 02:16:22 2011
@@ -97,9 +97,10 @@
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 @@
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);
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Mar 1 20:53:43 2011
+++ /branches/bleeding_edge/src/objects.cc Thu Mar 3 02:16:22 2011
@@ -6520,13 +6520,6 @@
for (int i = 0; i < old_size; i++) new_backing->set(i,
old_backing->get(i));
self->SetContent(*new_backing);
}
-
-
-// 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() {
=======================================
--- /branches/bleeding_edge/src/objects.h Tue Mar 1 20:53:43 2011
+++ /branches/bleeding_edge/src/objects.h Thu Mar 3 02:16:22 2011
@@ -1514,6 +1514,12 @@
// Tells whether the index'th element is present.
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 {
=======================================
--- /branches/bleeding_edge/src/parser.cc Tue Mar 1 20:53:43 2011
+++ /branches/bleeding_edge/src/parser.cc Thu Mar 3 02:16:22 2011
@@ -803,10 +803,12 @@
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 @@
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 @@
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);
=======================================
--- /branches/bleeding_edge/src/runtime.cc Wed Mar 2 00:10:38 2011
+++ /branches/bleeding_edge/src/runtime.cc Thu Mar 3 02:16:22 2011
@@ -789,6 +789,7 @@
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 @@
// 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 @@
// 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 @@
* 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 @@
// 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 @@
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 @@
break;
}
visitor->increase_index_offset(length);
+ return true;
}
@@ -8559,7 +8565,9 @@
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 @@
Handle<JSObject> jsobject = Handle<JSObject>::cast(object);
Handle<Object> tmp1 = GetElement(jsobject, index1);
+ RETURN_IF_EMPTY_HANDLE(tmp1);
Handle<Object> tmp2 = GetElement(jsobject, index2);
-
- SetElement(jsobject, index1, tmp2);
- SetElement(jsobject, index2, tmp1);
+ RETURN_IF_EMPTY_HANDLE(tmp2);
+
+ RETURN_IF_EMPTY_HANDLE(SetElement(jsobject, index1, tmp2));
+ RETURN_IF_EMPTY_HANDLE(SetElement(jsobject, index2, tmp1));
return Heap::undefined_value();
}
@@ -11266,7 +11276,8 @@
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 @@
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_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 @@
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 @@
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
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
