Revision: 6729
Author: [email protected]
Date: Thu Feb 10 06:41:16 2011
Log: Fix various places which do not check if SetProperty threw an
exception.
Review URL: http://codereview.chromium.org/6480003
http://code.google.com/p/v8/source/detail?r=6729
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/src/bootstrapper.cc
/branches/bleeding_edge/src/debug.cc
/branches/bleeding_edge/src/factory.cc
/branches/bleeding_edge/src/handles.cc
/branches/bleeding_edge/src/handles.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/top.cc
/branches/bleeding_edge/src/top.h
=======================================
--- /branches/bleeding_edge/src/api.cc Thu Feb 10 06:09:52 2011
+++ /branches/bleeding_edge/src/api.cc Thu Feb 10 06:41:16 2011
@@ -670,7 +670,7 @@
void Template::Set(v8::Handle<String> name, v8::Handle<Data> value,
v8::PropertyAttribute attribute) {
- if (IsDeadCheck("v8::Template::SetProperty()")) return;
+ if (IsDeadCheck("v8::Template::Set()")) return;
ENTER_V8;
HandleScope scope;
i::Handle<i::Object> list(Utils::OpenHandle(this)->property_list());
=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Wed Feb 2 05:31:52 2011
+++ /branches/bleeding_edge/src/bootstrapper.cc Thu Feb 10 06:41:16 2011
@@ -349,7 +349,7 @@
prototype,
call_code,
is_ecma_native);
- SetProperty(target, symbol, function, DONT_ENUM);
+ SetLocalPropertyNoThrow(target, symbol, function, DONT_ENUM);
if (is_ecma_native) {
function->shared()->set_instance_class_name(*symbol);
}
@@ -580,8 +580,8 @@
Handle<JSObject> prototype =
Handle<JSObject>(
JSObject::cast(js_global_function->instance_prototype()));
- SetProperty(prototype, Factory::constructor_symbol(),
- Top::object_function(), NONE);
+ SetLocalPropertyNoThrow(
+ prototype, Factory::constructor_symbol(), Top::object_function(),
NONE);
} else {
Handle<FunctionTemplateInfo> js_global_constructor(
FunctionTemplateInfo::cast(js_global_template->constructor()));
@@ -683,7 +683,8 @@
global_context()->set_security_token(*inner_global);
Handle<String> object_name = Handle<String>(Heap::Object_symbol());
- SetProperty(inner_global, object_name, Top::object_function(),
DONT_ENUM);
+ SetLocalPropertyNoThrow(inner_global, object_name,
+ Top::object_function(), DONT_ENUM);
Handle<JSObject> global = Handle<JSObject>(global_context()->global());
@@ -851,7 +852,7 @@
cons->SetInstanceClassName(*name);
Handle<JSObject> json_object = Factory::NewJSObject(cons, TENURED);
ASSERT(json_object->IsJSObject());
- SetProperty(global, name, json_object, DONT_ENUM);
+ SetLocalPropertyNoThrow(global, name, json_object, DONT_ENUM);
global_context()->set_json_object(*json_object);
}
@@ -880,12 +881,12 @@
global_context()->set_arguments_boilerplate(*result);
// Note: callee must be added as the first property and
// length must be added as the second property.
- SetProperty(result, Factory::callee_symbol(),
- Factory::undefined_value(),
- DONT_ENUM);
- SetProperty(result, Factory::length_symbol(),
- Factory::undefined_value(),
- DONT_ENUM);
+ SetLocalPropertyNoThrow(result, Factory::callee_symbol(),
+ Factory::undefined_value(),
+ DONT_ENUM);
+ SetLocalPropertyNoThrow(result, Factory::length_symbol(),
+ Factory::undefined_value(),
+ DONT_ENUM);
#ifdef DEBUG
LookupResult lookup;
@@ -1085,10 +1086,8 @@
static const PropertyAttributes attributes =
static_cast<PropertyAttributes>(READ_ONLY | DONT_DELETE);
Handle<String> global_symbol = Factory::LookupAsciiSymbol("global");
- SetProperty(builtins,
- global_symbol,
- Handle<Object>(global_context()->global()),
- attributes);
+ Handle<Object> global_obj(global_context()->global());
+ SetLocalPropertyNoThrow(builtins, global_symbol, global_obj, attributes);
// Setup the reference from the global object to the builtins object.
JSGlobalObject::cast(global_context()->global())->set_builtins(*builtins);
@@ -1480,17 +1479,17 @@
if (FLAG_expose_natives_as != NULL && strlen(FLAG_expose_natives_as) !=
0) {
Handle<String> natives_string =
Factory::LookupAsciiSymbol(FLAG_expose_natives_as);
- SetProperty(js_global, natives_string,
- Handle<JSObject>(js_global->builtins()), DONT_ENUM);
+ SetLocalPropertyNoThrow(js_global, natives_string,
+ Handle<JSObject>(js_global->builtins()),
DONT_ENUM);
}
Handle<Object> Error = GetProperty(js_global, "Error");
if (Error->IsJSObject()) {
Handle<String> name = Factory::LookupAsciiSymbol("stackTraceLimit");
- SetProperty(Handle<JSObject>::cast(Error),
- name,
- Handle<Smi>(Smi::FromInt(FLAG_stack_trace_limit)),
- NONE);
+ SetLocalPropertyNoThrow(Handle<JSObject>::cast(Error),
+ name,
+
Handle<Smi>(Smi::FromInt(FLAG_stack_trace_limit)),
+ NONE);
}
#ifdef ENABLE_DEBUGGER_SUPPORT
@@ -1507,8 +1506,8 @@
Handle<String> debug_string =
Factory::LookupAsciiSymbol(FLAG_expose_debug_as);
- SetProperty(js_global, debug_string,
- Handle<Object>(Debug::debug_context()->global_proxy()), DONT_ENUM);
+ Handle<Object> global_proxy(Debug::debug_context()->global_proxy());
+ SetLocalPropertyNoThrow(js_global, debug_string, global_proxy,
DONT_ENUM);
}
#endif
}
@@ -1679,7 +1678,7 @@
Handle<String> key = Handle<String>(descs->GetKey(i));
int index = descs->GetFieldIndex(i);
Handle<Object> value =
Handle<Object>(from->FastPropertyAt(index));
- SetProperty(to, key, value, details.attributes());
+ SetLocalPropertyNoThrow(to, key, value, details.attributes());
break;
}
case CONSTANT_FUNCTION: {
@@ -1687,7 +1686,7 @@
Handle<String> key = Handle<String>(descs->GetKey(i));
Handle<JSFunction> fun =
Handle<JSFunction>(descs->GetConstantFunction(i));
- SetProperty(to, key, fun, details.attributes());
+ SetLocalPropertyNoThrow(to, key, fun, details.attributes());
break;
}
case CALLBACKS: {
@@ -1737,7 +1736,7 @@
value =
Handle<Object>(JSGlobalPropertyCell::cast(*value)->value());
}
PropertyDetails details = properties->DetailsAt(i);
- SetProperty(to, key, value, details.attributes());
+ SetLocalPropertyNoThrow(to, key, value, details.attributes());
}
}
}
=======================================
--- /branches/bleeding_edge/src/debug.cc Thu Jan 6 05:14:32 2011
+++ /branches/bleeding_edge/src/debug.cc Thu Feb 10 06:41:16 2011
@@ -835,7 +835,9 @@
// Expose the builtins object in the debugger context.
Handle<String> key = Factory::LookupAsciiSymbol("builtins");
Handle<GlobalObject> global = Handle<GlobalObject>(context->global());
- SetProperty(global, key, Handle<Object>(global->builtins()), NONE);
+ RETURN_IF_EMPTY_HANDLE_VALUE(
+ SetProperty(global, key, Handle<Object>(global->builtins()), NONE),
+ false);
// Compile the JavaScript for the debugger in the debugger context.
Debugger::set_compiling_natives(true);
=======================================
--- /branches/bleeding_edge/src/factory.cc Thu Feb 10 04:02:36 2011
+++ /branches/bleeding_edge/src/factory.cc Thu Feb 10 06:41:16 2011
@@ -585,7 +585,9 @@
// Set function.prototype and give the prototype a constructor
// property that refers to the function.
SetPrototypeProperty(function, prototype);
- SetProperty(prototype, Factory::constructor_symbol(), function,
DONT_ENUM);
+ // Currently safe because it is only invoked from Genesis.
+ SetLocalPropertyNoThrow(
+ prototype, Factory::constructor_symbol(), function, DONT_ENUM);
return function;
}
=======================================
--- /branches/bleeding_edge/src/handles.cc Tue Feb 8 11:42:14 2011
+++ /branches/bleeding_edge/src/handles.cc Thu Feb 10 06:41:16 2011
@@ -288,6 +288,17 @@
CALL_HEAP_FUNCTION(object->
SetLocalPropertyIgnoreAttributes(*key, *value, attributes), Object);
}
+
+
+void SetLocalPropertyNoThrow(Handle<JSObject> object,
+ Handle<String> key,
+ Handle<Object> value,
+ PropertyAttributes attributes) {
+ ASSERT(!Top::has_pending_exception());
+ CHECK(!SetLocalPropertyIgnoreAttributes(
+ object, key, value, attributes).is_null());
+ CHECK(!Top::has_pending_exception());
+}
Handle<Object> SetPropertyWithInterceptor(Handle<JSObject> object,
=======================================
--- /branches/bleeding_edge/src/handles.h Thu Jan 6 06:00:50 2011
+++ /branches/bleeding_edge/src/handles.h Thu Feb 10 06:41:16 2011
@@ -223,6 +223,13 @@
Handle<Object> value,
PropertyAttributes attributes);
+// Used to set local properties on the object we totally control
+// and which therefore has no accessors and alikes.
+void SetLocalPropertyNoThrow(Handle<JSObject> object,
+ Handle<String> key,
+ Handle<Object> value,
+ PropertyAttributes attributes = NONE);
+
Handle<Object> SetPropertyWithInterceptor(Handle<JSObject> object,
Handle<String> key,
Handle<Object> value,
=======================================
--- /branches/bleeding_edge/src/runtime.cc Wed Feb 9 03:38:10 2011
+++ /branches/bleeding_edge/src/runtime.cc Thu Feb 10 06:41:16 2011
@@ -1089,11 +1089,7 @@
const char* type = (lookup.IsReadOnly()) ? "const" : "var";
return ThrowRedeclarationError(type, name);
}
- Handle<Object> result = SetProperty(global, name, value, attributes);
- if (result.is_null()) {
- ASSERT(Top::has_pending_exception());
- return Failure::Exception();
- }
+ RETURN_IF_EMPTY_HANDLE(SetProperty(global, name, value, attributes));
} else {
// If a property with this name does not already exist on the
// global object add the property locally. We take special
@@ -1101,12 +1097,8 @@
// of callbacks in the prototype chain (this rules out using
// SetProperty). Also, we must use the handle-based version to
// avoid GC issues.
- Handle<Object> result =
- SetLocalPropertyIgnoreAttributes(global, name, value,
attributes);
- if (result.is_null()) {
- ASSERT(Top::has_pending_exception());
- return Failure::Exception();
- }
+ RETURN_IF_EMPTY_HANDLE(
+ SetLocalPropertyIgnoreAttributes(global, name, value,
attributes));
}
}
@@ -1166,9 +1158,8 @@
} else {
// Slow case: The property is not in the FixedArray part of the
context.
Handle<JSObject> context_ext = Handle<JSObject>::cast(holder);
- Handle<Object> result =
- SetProperty(context_ext, name, initial_value, mode);
- if (result.is_null()) return Failure::Exception();
+ RETURN_IF_EMPTY_HANDLE(
+ SetProperty(context_ext, name, initial_value, mode));
}
}
@@ -1195,8 +1186,7 @@
ASSERT(!context_ext->HasLocalProperty(*name));
Handle<Object> value(Heap::undefined_value());
if (*initial_value != NULL) value = initial_value;
- Handle<Object> result = SetProperty(context_ext, name, value, mode);
- if (result.is_null()) return Failure::Exception();
+ RETURN_IF_EMPTY_HANDLE(SetProperty(context_ext, name, value, mode));
}
return Heap::undefined_value();
@@ -1345,12 +1335,12 @@
// with setting the value because the property is either absent or
// read-only. We also have to do redo the lookup.
HandleScope handle_scope;
- Handle<GlobalObject>global(Top::context()->global());
-
- // BUG 1213579: Handle the case where we have to set a read-only
+ Handle<GlobalObject> global(Top::context()->global());
+
+ // BUG 1213575: Handle the case where we have to set a read-only
// property through an interceptor and only do it if it's
// uninitialized, e.g. the hole. Nirk...
- SetProperty(global, name, value, attributes);
+ RETURN_IF_EMPTY_HANDLE(SetProperty(global, name, value, attributes));
return *value;
}
@@ -1432,7 +1422,7 @@
// context.
if (attributes == ABSENT) {
Handle<JSObject> global = Handle<JSObject>(Top::context()->global());
- SetProperty(global, name, value, NONE);
+ RETURN_IF_EMPTY_HANDLE(SetProperty(global, name, value, NONE));
return *value;
}
@@ -1469,14 +1459,8 @@
// The property was found in a different context extension object.
// Set it if it is not a read-only property.
if ((attributes & READ_ONLY) == 0) {
- Handle<Object> set = SetProperty(context_ext, name, value,
attributes);
- // Setting a property might throw an exception. Exceptions
- // are converted to empty handles in handle operations. We
- // need to convert back to exceptions here.
- if (set.is_null()) {
- ASSERT(Top::has_pending_exception());
- return Failure::Exception();
- }
+ RETURN_IF_EMPTY_HANDLE(
+ SetProperty(context_ext, name, value, attributes));
}
}
@@ -7424,14 +7408,7 @@
// extension object itself.
if ((attributes & READ_ONLY) == 0 ||
(context_ext->GetLocalPropertyAttribute(*name) == ABSENT)) {
- Handle<Object> result = SetProperty(context_ext, name, value, NONE);
- if (result.is_null()) {
- // Failure::Exception is converted to a null handle in the
- // handle-based methods such as SetProperty. We therefore need
- // to convert null handles back to exceptions.
- ASSERT(Top::has_pending_exception());
- return Failure::Exception();
- }
+ RETURN_IF_EMPTY_HANDLE(SetProperty(context_ext, name, value, NONE));
}
return *value;
}
@@ -9075,7 +9052,7 @@
// Copy all the context locals into an object used to materialize a scope.
-static void CopyContextLocalsToScopeObject(
+static bool CopyContextLocalsToScopeObject(
Handle<SerializedScopeInfo> serialized_scope_info,
ScopeInfo<>& scope_info,
Handle<Context> context,
@@ -9089,11 +9066,15 @@
// Don't include the arguments shadow (.arguments) context variable.
if (*scope_info.context_slot_name(i) !=
Heap::arguments_shadow_symbol()) {
- SetProperty(scope_object,
- scope_info.context_slot_name(i),
- Handle<Object>(context->get(context_index)), NONE);
+ RETURN_IF_EMPTY_HANDLE_VALUE(
+ SetProperty(scope_object,
+ scope_info.context_slot_name(i),
+ Handle<Object>(context->get(context_index)), NONE),
+ false);
}
}
+
+ return true;
}
@@ -9111,23 +9092,29 @@
// First fill all parameters.
for (int i = 0; i < scope_info.number_of_parameters(); ++i) {
- SetProperty(local_scope,
- scope_info.parameter_name(i),
- Handle<Object>(frame->GetParameter(i)), NONE);
+ RETURN_IF_EMPTY_HANDLE_VALUE(
+ SetProperty(local_scope,
+ scope_info.parameter_name(i),
+ Handle<Object>(frame->GetParameter(i)), NONE),
+ Handle<JSObject>());
}
// Second fill all stack locals.
for (int i = 0; i < scope_info.number_of_stack_slots(); i++) {
- SetProperty(local_scope,
- scope_info.stack_slot_name(i),
- Handle<Object>(frame->GetExpression(i)), NONE);
+ RETURN_IF_EMPTY_HANDLE_VALUE(
+ SetProperty(local_scope,
+ scope_info.stack_slot_name(i),
+ Handle<Object>(frame->GetExpression(i)), NONE),
+ Handle<JSObject>());
}
// Third fill all context locals.
Handle<Context> frame_context(Context::cast(frame->context()));
Handle<Context> function_context(frame_context->fcontext());
- CopyContextLocalsToScopeObject(serialized_scope_info, scope_info,
- function_context, local_scope);
+ if (!CopyContextLocalsToScopeObject(serialized_scope_info, scope_info,
+ function_context, local_scope)) {
+ return Handle<JSObject>();
+ }
// Finally copy any properties from the function context extension. This
will
// be variables introduced by eval.
@@ -9140,7 +9127,9 @@
// Names of variables introduced by eval are strings.
ASSERT(keys->get(i)->IsString());
Handle<String> key(String::cast(keys->get(i)));
- SetProperty(local_scope, key, GetProperty(ext, key), NONE);
+ RETURN_IF_EMPTY_HANDLE_VALUE(
+ SetProperty(local_scope, key, GetProperty(ext, key), NONE),
+ Handle<JSObject>());
}
}
}
@@ -9173,16 +9162,20 @@
for (int i = 0; i < scope_info.number_of_parameters(); ++i) {
// We don't expect exception-throwing getters on the arguments
shadow.
Object* element =
arguments_shadow->GetElement(i)->ToObjectUnchecked();
- SetProperty(closure_scope,
- scope_info.parameter_name(i),
- Handle<Object>(element),
- NONE);
+ RETURN_IF_EMPTY_HANDLE_VALUE(
+ SetProperty(closure_scope,
+ scope_info.parameter_name(i),
+ Handle<Object>(element),
+ NONE),
+ Handle<JSObject>());
}
}
// Fill all context locals to the context extension.
- CopyContextLocalsToScopeObject(serialized_scope_info, scope_info,
- context, closure_scope);
+ if (!CopyContextLocalsToScopeObject(serialized_scope_info, scope_info,
+ context, closure_scope)) {
+ return Handle<JSObject>();
+ }
// Finally copy any properties from the function context extension. This
will
// be variables introduced by eval.
@@ -9193,7 +9186,9 @@
// Names of variables introduced by eval are strings.
ASSERT(keys->get(i)->IsString());
Handle<String> key(String::cast(keys->get(i)));
- SetProperty(closure_scope, key, GetProperty(ext, key), NONE);
+ RETURN_IF_EMPTY_HANDLE_VALUE(
+ SetProperty(closure_scope, key, GetProperty(ext, key), NONE),
+ Handle<JSObject>());
}
}
@@ -9480,6 +9475,7 @@
// Fill in scope details.
details->set(kScopeDetailsTypeIndex, Smi::FromInt(it.Type()));
Handle<JSObject> scope_object = it.ScopeObject();
+ RETURN_IF_EMPTY_HANDLE(scope_object);
details->set(kScopeDetailsObjectIndex, *scope_object);
return *Factory::NewJSArrayWithElements(details);
@@ -9961,6 +9957,7 @@
// Materialize the content of the local scope into a JSObject.
Handle<JSObject> local_scope = MaterializeLocalScope(frame);
+ RETURN_IF_EMPTY_HANDLE(local_scope);
// Allocate a new context for the debug evaluation and set the extension
// object build.
=======================================
--- /branches/bleeding_edge/src/top.cc Wed Feb 9 11:34:04 2011
+++ /branches/bleeding_edge/src/top.cc Thu Feb 10 06:41:16 2011
@@ -372,18 +372,6 @@
return Factory::empty_symbol();
}
}
-
-
-static void SetLocalProperty(Handle<JSObject> object,
- Handle<String> key,
- Handle<Object> value) {
- // We set properties on freshly allocated JS object, nothing
- // should fail except for OOM which is handled by
- // SetLocalPropertyIgnoreAttributes.
- ASSERT(!Top::has_pending_exception());
- CHECK(!SetLocalPropertyIgnoreAttributes(object, key, value,
NONE).is_null());
- CHECK(!Top::has_pending_exception());
-}
Handle<JSArray> Top::CaptureCurrentStackTrace(
@@ -433,16 +421,16 @@
// tag.
column_offset += script->column_offset()->value();
}
- SetLocalProperty(stackFrame, column_key,
- Handle<Smi>(Smi::FromInt(column_offset + 1)));
- }
- SetLocalProperty(stackFrame, line_key,
- Handle<Smi>(Smi::FromInt(line_number + 1)));
+ SetLocalPropertyNoThrow(stackFrame, column_key,
+ Handle<Smi>(Smi::FromInt(column_offset +
1)));
+ }
+ SetLocalPropertyNoThrow(stackFrame, line_key,
+ Handle<Smi>(Smi::FromInt(line_number +
1)));
}
if (options & StackTrace::kScriptName) {
Handle<Object> script_name(script->name());
- SetLocalProperty(stackFrame, script_key, script_name);
+ SetLocalPropertyNoThrow(stackFrame, script_key, script_name);
}
if (options & StackTrace::kScriptNameOrSourceURL) {
@@ -458,7 +446,8 @@
if (caught_exception) {
result = Factory::undefined_value();
}
- SetLocalProperty(stackFrame, script_name_or_source_url_key,
result);
+ SetLocalPropertyNoThrow(stackFrame, script_name_or_source_url_key,
+ result);
}
if (options & StackTrace::kFunctionName) {
@@ -466,20 +455,20 @@
if (fun_name->ToBoolean()->IsFalse()) {
fun_name = Handle<Object>(fun->shared()->inferred_name());
}
- SetLocalProperty(stackFrame, function_key, fun_name);
+ SetLocalPropertyNoThrow(stackFrame, function_key, fun_name);
}
if (options & StackTrace::kIsEval) {
int type = Smi::cast(script->compilation_type())->value();
Handle<Object> is_eval = (type == Script::COMPILATION_TYPE_EVAL) ?
Factory::true_value() : Factory::false_value();
- SetLocalProperty(stackFrame, eval_key, is_eval);
+ SetLocalPropertyNoThrow(stackFrame, eval_key, is_eval);
}
if (options & StackTrace::kIsConstructor) {
Handle<Object> is_constructor = (frames[i].is_constructor()) ?
Factory::true_value() : Factory::false_value();
- SetLocalProperty(stackFrame, constructor_key, is_constructor);
+ SetLocalPropertyNoThrow(stackFrame, constructor_key,
is_constructor);
}
FixedArray::cast(stack_trace->elements())->set(frames_seen,
*stackFrame);
=======================================
--- /branches/bleeding_edge/src/top.h Fri Feb 4 05:43:38 2011
+++ /branches/bleeding_edge/src/top.h Thu Feb 10 06:41:16 2011
@@ -41,6 +41,15 @@
#define RETURN_IF_SCHEDULED_EXCEPTION() \
if (Top::has_scheduled_exception()) return
Top::PromoteScheduledException()
+#define RETURN_IF_EMPTY_HANDLE_VALUE(call, value) \
+ if (call.is_null()) { \
+ ASSERT(Top::has_pending_exception()); \
+ return value; \
+ }
+
+#define RETURN_IF_EMPTY_HANDLE(call) \
+ RETURN_IF_EMPTY_HANDLE_VALUE(call, Failure::Exception())
+
// Top has static variables used for JavaScript execution.
class SaveContext; // Forward declaration.
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
