Revision: 5509
Author: [email protected]
Date: Thu Sep 23 01:27:51 2010
Log: Fix possible evaluation order problems.
We should not allow handle dereference and GC inside the same expression
because order of subexpression evalution are not defined.
Review URL: http://codereview.chromium.org/3398014
http://code.google.com/p/v8/source/detail?r=5509
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/bootstrapper.cc
/branches/bleeding_edge/src/compilation-cache.cc
/branches/bleeding_edge/src/compiler.cc
/branches/bleeding_edge/src/handles.cc
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/runtime.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Mon Sep 20 02:29:12 2010
+++ /branches/bleeding_edge/src/api.cc Thu Sep 23 01:27:51 2010
@@ -765,6 +765,12 @@
}
return 0;
}
+
+
+#define SET_FIELD_WRAPPED(obj, setter, cdata) do { \
+ i::Handle<i::Object> proxy = FromCData(cdata); \
+ (obj)->setter(*proxy); \
+ } while (false)
void FunctionTemplate::SetCallHandler(InvocationCallback callback,
@@ -776,7 +782,7 @@
i::Factory::NewStruct(i::CALL_HANDLER_INFO_TYPE);
i::Handle<i::CallHandlerInfo> obj =
i::Handle<i::CallHandlerInfo>::cast(struct_obj);
- obj->set_callback(*FromCData(callback));
+ SET_FIELD_WRAPPED(obj, set_callback, callback);
if (data.IsEmpty()) data = v8::Undefined();
obj->set_data(*Utils::OpenHandle(*data));
Utils::OpenHandle(this)->set_call_code(*obj);
@@ -792,8 +798,8 @@
v8::PropertyAttribute attributes) {
i::Handle<i::AccessorInfo> obj = i::Factory::NewAccessorInfo();
ASSERT(getter != NULL);
- obj->set_getter(*FromCData(getter));
- obj->set_setter(*FromCData(setter));
+ SET_FIELD_WRAPPED(obj, set_getter, getter);
+ SET_FIELD_WRAPPED(obj, set_setter, setter);
if (data.IsEmpty()) data = v8::Undefined();
obj->set_data(*Utils::OpenHandle(*data));
obj->set_name(*Utils::OpenHandle(*name));
@@ -877,11 +883,13 @@
i::Factory::NewStruct(i::INTERCEPTOR_INFO_TYPE);
i::Handle<i::InterceptorInfo> obj =
i::Handle<i::InterceptorInfo>::cast(struct_obj);
- if (getter != 0) obj->set_getter(*FromCData(getter));
- if (setter != 0) obj->set_setter(*FromCData(setter));
- if (query != 0) obj->set_query(*FromCData(query));
- if (remover != 0) obj->set_deleter(*FromCData(remover));
- if (enumerator != 0) obj->set_enumerator(*FromCData(enumerator));
+
+ if (getter != 0) SET_FIELD_WRAPPED(obj, set_getter, getter);
+ if (setter != 0) SET_FIELD_WRAPPED(obj, set_setter, setter);
+ if (query != 0) SET_FIELD_WRAPPED(obj, set_query, query);
+ if (remover != 0) SET_FIELD_WRAPPED(obj, set_deleter, remover);
+ if (enumerator != 0) SET_FIELD_WRAPPED(obj, set_enumerator, enumerator);
+
if (data.IsEmpty()) data = v8::Undefined();
obj->set_data(*Utils::OpenHandle(*data));
Utils::OpenHandle(this)->set_named_property_handler(*obj);
@@ -905,11 +913,13 @@
i::Factory::NewStruct(i::INTERCEPTOR_INFO_TYPE);
i::Handle<i::InterceptorInfo> obj =
i::Handle<i::InterceptorInfo>::cast(struct_obj);
- if (getter != 0) obj->set_getter(*FromCData(getter));
- if (setter != 0) obj->set_setter(*FromCData(setter));
- if (query != 0) obj->set_query(*FromCData(query));
- if (remover != 0) obj->set_deleter(*FromCData(remover));
- if (enumerator != 0) obj->set_enumerator(*FromCData(enumerator));
+
+ if (getter != 0) SET_FIELD_WRAPPED(obj, set_getter, getter);
+ if (setter != 0) SET_FIELD_WRAPPED(obj, set_setter, setter);
+ if (query != 0) SET_FIELD_WRAPPED(obj, set_query, query);
+ if (remover != 0) SET_FIELD_WRAPPED(obj, set_deleter, remover);
+ if (enumerator != 0) SET_FIELD_WRAPPED(obj, set_enumerator, enumerator);
+
if (data.IsEmpty()) data = v8::Undefined();
obj->set_data(*Utils::OpenHandle(*data));
Utils::OpenHandle(this)->set_indexed_property_handler(*obj);
@@ -928,7 +938,7 @@
i::Factory::NewStruct(i::CALL_HANDLER_INFO_TYPE);
i::Handle<i::CallHandlerInfo> obj =
i::Handle<i::CallHandlerInfo>::cast(struct_obj);
- obj->set_callback(*FromCData(callback));
+ SET_FIELD_WRAPPED(obj, set_callback, callback);
if (data.IsEmpty()) data = v8::Undefined();
obj->set_data(*Utils::OpenHandle(*data));
Utils::OpenHandle(this)->set_instance_call_handler(*obj);
@@ -1043,8 +1053,10 @@
i::Factory::NewStruct(i::ACCESS_CHECK_INFO_TYPE);
i::Handle<i::AccessCheckInfo> info =
i::Handle<i::AccessCheckInfo>::cast(struct_info);
- info->set_named_callback(*FromCData(named_callback));
- info->set_indexed_callback(*FromCData(indexed_callback));
+
+ SET_FIELD_WRAPPED(info, set_named_callback, named_callback);
+ SET_FIELD_WRAPPED(info, set_indexed_callback, indexed_callback);
+
if (data.IsEmpty()) data = v8::Undefined();
info->set_data(*Utils::OpenHandle(*data));
=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Tue Sep 21 06:04:42 2010
+++ /branches/bleeding_edge/src/bootstrapper.cc Thu Sep 23 01:27:51 2010
@@ -1413,8 +1413,14 @@
Handle<FixedArray> caches = Factory::NewFixedArray(kNumberOfCaches,
TENURED);
int index = 0;
-#define F(size, func) caches->set(index++, CreateCache(size, func));
- JSFUNCTION_RESULT_CACHE_LIST(F)
+
+#define F(size, func) do { \
+ FixedArray* cache = CreateCache((size), (func)); \
+ caches->set(index++, cache); \
+ } while (false)
+
+ JSFUNCTION_RESULT_CACHE_LIST(F);
+
#undef F
global_context()->set_jsfunction_result_caches(*caches);
=======================================
--- /branches/bleeding_edge/src/compilation-cache.cc Tue Aug 17 04:44:01
2010
+++ /branches/bleeding_edge/src/compilation-cache.cc Thu Sep 23 01:27:51
2010
@@ -110,6 +110,9 @@
void Put(Handle<String> source, Handle<SharedFunctionInfo>
function_info);
private:
+ MUST_USE_RESULT Object* TryTablePut(
+ Handle<String> source, Handle<SharedFunctionInfo> function_info);
+
// Note: Returns a new hash table if operation results in expansion.
Handle<CompilationCacheTable> TablePut(
Handle<String> source, Handle<SharedFunctionInfo> function_info);
@@ -137,6 +140,12 @@
Handle<SharedFunctionInfo> function_info);
private:
+ MUST_USE_RESULT Object* TryTablePut(
+ Handle<String> source,
+ Handle<Context> context,
+ Handle<SharedFunctionInfo> function_info);
+
+
// Note: Returns a new hash table if operation results in expansion.
Handle<CompilationCacheTable> TablePut(
Handle<String> source,
@@ -159,6 +168,10 @@
JSRegExp::Flags flags,
Handle<FixedArray> data);
private:
+ MUST_USE_RESULT Object* TryTablePut(Handle<String> source,
+ JSRegExp::Flags flags,
+ Handle<FixedArray> data);
+
// Note: Returns a new hash table if operation results in expansion.
Handle<CompilationCacheTable> TablePut(Handle<String> source,
JSRegExp::Flags flags,
@@ -318,13 +331,20 @@
return Handle<SharedFunctionInfo>::null();
}
}
+
+
+Object* CompilationCacheScript::TryTablePut(
+ Handle<String> source,
+ Handle<SharedFunctionInfo> function_info) {
+ Handle<CompilationCacheTable> table = GetFirstTable();
+ return table->Put(*source, *function_info);
+}
Handle<CompilationCacheTable> CompilationCacheScript::TablePut(
Handle<String> source,
Handle<SharedFunctionInfo> function_info) {
- CALL_HEAP_FUNCTION(GetFirstTable()->Put(*source, *function_info),
- CompilationCacheTable);
+ CALL_HEAP_FUNCTION(TryTablePut(source, function_info),
CompilationCacheTable);
}
@@ -364,15 +384,22 @@
return Handle<SharedFunctionInfo>::null();
}
}
+
+
+Object* CompilationCacheEval::TryTablePut(
+ Handle<String> source,
+ Handle<Context> context,
+ Handle<SharedFunctionInfo> function_info) {
+ Handle<CompilationCacheTable> table = GetFirstTable();
+ return table->PutEval(*source, *context, *function_info);
+}
Handle<CompilationCacheTable> CompilationCacheEval::TablePut(
Handle<String> source,
Handle<Context> context,
Handle<SharedFunctionInfo> function_info) {
- CALL_HEAP_FUNCTION(GetFirstTable()->PutEval(*source,
- *context,
- *function_info),
+ CALL_HEAP_FUNCTION(TryTablePut(source, context, function_info),
CompilationCacheTable);
}
@@ -413,14 +440,22 @@
return Handle<FixedArray>::null();
}
}
+
+
+Object* CompilationCacheRegExp::TryTablePut(
+ Handle<String> source,
+ JSRegExp::Flags flags,
+ Handle<FixedArray> data) {
+ Handle<CompilationCacheTable> table = GetFirstTable();
+ return table->PutRegExp(*source, flags, *data);
+}
Handle<CompilationCacheTable> CompilationCacheRegExp::TablePut(
Handle<String> source,
JSRegExp::Flags flags,
Handle<FixedArray> data) {
- CALL_HEAP_FUNCTION(GetFirstTable()->PutRegExp(*source, flags, *data),
- CompilationCacheTable);
+ CALL_HEAP_FUNCTION(TryTablePut(source, flags, data),
CompilationCacheTable);
}
=======================================
--- /branches/bleeding_edge/src/compiler.cc Thu Sep 9 04:49:21 2010
+++ /branches/bleeding_edge/src/compiler.cc Thu Sep 23 01:27:51 2010
@@ -120,8 +120,9 @@
Handle<Context> context = Handle<Context>::null();
Handle<Code> code = MakeCode(context, info);
if (!info->shared_info().is_null()) {
- info->shared_info()->set_scope_info(
- *SerializedScopeInfo::Create(info->scope()));
+ Handle<SerializedScopeInfo> scope_info =
+ SerializedScopeInfo::Create(info->scope());
+ info->shared_info()->set_scope_info(*scope_info);
}
return code;
}
@@ -420,10 +421,12 @@
// Update the shared function info with the compiled code and the scope
info.
// Please note, that the order of the sharedfunction initialization is
- // important since set_scope_info might trigger a GC, causing the ASSERT
- // below to be invalid if the code was flushed. By settting the code
+ // important since SerializedScopeInfo::Create might trigger a GC,
causing
+ // the ASSERT below to be invalid if the code was flushed. By setting
the code
// object last we avoid this.
- shared->set_scope_info(*SerializedScopeInfo::Create(info->scope()));
+ Handle<SerializedScopeInfo> scope_info =
+ SerializedScopeInfo::Create(info->scope());
+ shared->set_scope_info(*scope_info);
shared->set_code(*code);
if (!info->closure().is_null()) {
info->closure()->set_code(*code);
=======================================
--- /branches/bleeding_edge/src/handles.cc Tue Sep 14 07:52:53 2010
+++ /branches/bleeding_edge/src/handles.cc Thu Sep 23 01:27:51 2010
@@ -466,7 +466,8 @@
if (!script->source()->IsString()) {
ASSERT(script->source()->IsUndefined());
- script->set_line_ends(*(Factory::NewFixedArray(0)));
+ Handle<FixedArray> empty = Factory::NewFixedArray(0);
+ script->set_line_ends(*empty);
ASSERT(script->line_ends()->IsFixedArray());
return;
}
=======================================
--- /branches/bleeding_edge/src/objects.cc Thu Sep 16 03:55:37 2010
+++ /branches/bleeding_edge/src/objects.cc Thu Sep 23 01:27:51 2010
@@ -8721,11 +8721,11 @@
// No free slot - extend break point info array.
Handle<FixedArray> old_break_points =
Handle<FixedArray>(FixedArray::cast(debug_info->break_points()));
- debug_info->set_break_points(*Factory::NewFixedArray(
- old_break_points->length() +
- Debug::kEstimatedNofBreakPointsInFunction));
Handle<FixedArray> new_break_points =
- Handle<FixedArray>(FixedArray::cast(debug_info->break_points()));
+ Factory::NewFixedArray(old_break_points->length() +
+ Debug::kEstimatedNofBreakPointsInFunction);
+
+ debug_info->set_break_points(*new_break_points);
for (int i = 0; i < old_break_points->length(); i++) {
new_break_points->set(i, old_break_points->get(i));
}
=======================================
--- /branches/bleeding_edge/src/runtime.cc Fri Sep 17 01:34:53 2010
+++ /branches/bleeding_edge/src/runtime.cc Thu Sep 23 01:27:51 2010
@@ -678,7 +678,8 @@
// Elements that are stored as array elements always has:
// writable: true, configurable: true, enumerable: true.
elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
- elms->set(VALUE_INDEX, obj->GetElement(index));
+ Object* element = obj->GetElement(index);
+ elms->set(VALUE_INDEX, element);
elms->set(WRITABLE_INDEX, Heap::true_value());
elms->set(ENUMERABLE_INDEX, Heap::true_value());
elms->set(CONFIGURABLE_INDEX, Heap::true_value());
@@ -2837,7 +2838,8 @@
for (int i = 0; i < matches ; i++) {
int from = offsets.at(i * 2);
int to = offsets.at(i * 2 + 1);
- elements->set(i, *Factory::NewSubString(subject, from, to));
+ Handle<String> match = Factory::NewSubString(subject, from, to);
+ elements->set(i, *match);
}
Handle<JSArray> result = Factory::NewJSArrayWithElements(elements);
result->set_length(Smi::FromInt(matches));
@@ -3105,9 +3107,10 @@
// Arguments array to replace function is match, captures, index
and
// subject, i.e., 3 + capture count in total.
Handle<FixedArray> elements = Factory::NewFixedArray(3 +
capture_count);
- elements->set(0, *Factory::NewSubString(subject,
- match_start,
- match_end));
+ Handle<String> match = Factory::NewSubString(subject,
+ match_start,
+ match_end);
+ elements->set(0, *match);
for (int i = 1; i <= capture_count; i++) {
int start = register_vector[i * 2];
if (start >= 0) {
@@ -4953,12 +4956,14 @@
length);
for (int i = num_copied_from_cache; i < length; ++i) {
- elements->set(i, *LookupSingleCharacterStringFromCode(chars[i]));
+ Handle<Object> str = LookupSingleCharacterStringFromCode(chars[i]);
+ elements->set(i, *str);
}
} else {
elements = Factory::NewFixedArray(length);
for (int i = 0; i < length; ++i) {
- elements->set(i, *LookupSingleCharacterStringFromCode(s->Get(i)));
+ Handle<Object> str = LookupSingleCharacterStringFromCode(s->Get(i));
+ elements->set(i, *str);
}
}
@@ -7826,7 +7831,8 @@
uint32_t index;
if (name->AsArrayIndex(&index)) {
Handle<FixedArray> details = Factory::NewFixedArray(2);
- details->set(0, Runtime::GetElementOrCharAt(obj, index));
+ Object* element_or_char = Runtime::GetElementOrCharAt(obj, index);
+ details->set(0, element_or_char);
details->set(1, PropertyDetails(NONE, NORMAL).AsSmi());
return *Factory::NewJSArrayWithElements(details);
}
@@ -8628,7 +8634,8 @@
// Fill in scope details.
details->set(kScopeDetailsTypeIndex, Smi::FromInt(it.Type()));
- details->set(kScopeDetailsObjectIndex, *it.ScopeObject());
+ Handle<JSObject> scope_object = it.ScopeObject();
+ details->set(kScopeDetailsObjectIndex, *scope_object);
return *Factory::NewJSArrayWithElements(details);
}
@@ -8673,10 +8680,10 @@
Handle<FixedArray> frames_array = Factory::NewFixedArray(frames_count);
for (int i = 0; i < frames_count; i++) {
Handle<JSObject> frame_value =
Factory::NewJSObject(Top::object_function());
- frame_value->SetProperty(
- *address_str,
-
*Factory::NewNumberFromInt(reinterpret_cast<int>(frames[i].address)),
- NONE);
+ Handle<Object> frame_address =
+
Factory::NewNumberFromInt(reinterpret_cast<int>(frames[i].address));
+
+ frame_value->SetProperty(*address_str, *frame_address, NONE);
// Get the stack walk text for this frame.
Handle<String> frame_text;
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
