Fix various places which do not check if SetProperty threw an exception.

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

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

src/api.cc

@@ -670,7 +670,7 @@ static void InitializeTemplate(i::Handle<i::TemplateInfo> that, int type) {
 
 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());

src/bootstrapper.cc

@@ -349,7 +349,7 @@ static Handle<JSFunction> InstallFunction(Handle<JSObject> target,
                                       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<JSGlobalProxy> Genesis::CreateNewGlobals(
     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 @@ void Genesis::InitializeGlobal(Handle<GlobalObject> inner_global,
   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 @@ void Genesis::InitializeGlobal(Handle<GlobalObject> inner_global,
     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 @@ void Genesis::InitializeGlobal(Handle<GlobalObject> inner_global,
     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 @@ bool Genesis::InstallNatives() {
   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 @@ void Genesis::InstallSpecialObjects(Handle<Context> global_context) {
   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 @@ void Genesis::InstallSpecialObjects(Handle<Context> global_context) {
 
     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 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
           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 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
           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 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
           value = Handle<Object>(JSGlobalPropertyCell::cast(*value)->value());
         }
         PropertyDetails details = properties->DetailsAt(i);
-        SetProperty(to, key, value, details.attributes());
+        SetLocalPropertyNoThrow(to, key, value, details.attributes());
       }
     }
   }

src/debug.cc

@@ -835,7 +835,9 @@ bool Debug::Load() {
   // 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);

src/factory.cc

@@ -585,7 +585,9 @@ Handle<JSFunction> Factory::NewFunctionWithPrototype(Handle<String> name,
   // 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;
 }
 

src/handles.cc

@@ -290,6 +290,17 @@ Handle<Object> SetLocalPropertyIgnoreAttributes(
 }
 
 
+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,
                                           Handle<String> key,
                                           Handle<Object> value,

src/handles.h

@@ -223,6 +223,13 @@ Handle<Object> SetLocalPropertyIgnoreAttributes(
     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,

src/top.cc

@@ -374,18 +374,6 @@ Handle<String> Top::StackTraceString() {
 }
 
 
-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(
     int frame_limit, StackTrace::StackTraceOptions options) {
   // Ensure no negative values.
@@ -433,16 +421,16 @@ Handle<JSArray> Top::CaptureCurrentStackTrace(
             // tag.
             column_offset += script->column_offset()->value();
           }
-          SetLocalProperty(stackFrame, column_key,
-                           Handle<Smi>(Smi::FromInt(column_offset + 1)));
+          SetLocalPropertyNoThrow(stackFrame, column_key,
+                                  Handle<Smi>(Smi::FromInt(column_offset + 1)));
         }
-        SetLocalProperty(stackFrame, line_key,
-                         Handle<Smi>(Smi::FromInt(line_number + 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 @@ Handle<JSArray> Top::CaptureCurrentStackTrace(
         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 @@ Handle<JSArray> Top::CaptureCurrentStackTrace(
         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);

src/top.h

@@ -41,6 +41,15 @@ class Simulator;
 #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.