Get gcc to check that we don't ignore return values of functions that can

fail to allocate because we need a GC.
Review URL: http://codereview.chromium.org/3274008

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

src/accessors.h

@@ -75,8 +75,10 @@ class Accessors : public AllStatic {
   };
 
   // Accessor functions called directly from the runtime system.
-  static Object* FunctionGetPrototype(Object* object, void*);
-  static Object* FunctionSetPrototype(JSObject* object, Object* value, void*);
+  MUST_USE_RESULT static Object* FunctionGetPrototype(Object* object, void*);
+  MUST_USE_RESULT static Object* FunctionSetPrototype(JSObject* object,
+                                                      Object* value,
+                                                      void*);
  private:
   // Accessor functions only used through the descriptor.
   static Object* FunctionGetLength(Object* object, void*);

src/builtins.cc

@@ -243,7 +243,7 @@ BUILTIN(ArrayCodeGeneric) {
 }
 
 
-static Object* AllocateJSArray() {
+MUST_USE_RESULT static Object* AllocateJSArray() {
   JSFunction* array_function =
       Top::context()->global_context()->array_function();
   Object* result = Heap::AllocateJSObject(array_function);
@@ -252,7 +252,7 @@ static Object* AllocateJSArray() {
 }
 
 
-static Object* AllocateEmptyJSArray() {
+MUST_USE_RESULT static Object* AllocateEmptyJSArray() {
   Object* result = AllocateJSArray();
   if (result->IsFailure()) return result;
   JSArray* result_array = JSArray::cast(result);

src/debug.cc

@@ -1007,17 +1007,18 @@ Handle<Object> Debug::CheckBreakPoints(Handle<Object> break_point_objects) {
     for (int i = 0; i < array->length(); i++) {
       Handle<Object> o(array->get(i));
       if (CheckBreakPoint(o)) {
-        break_points_hit->SetElement(break_points_hit_count++, *o);
+        SetElement(break_points_hit, break_points_hit_count++, o);
       }
     }
   } else {
     if (CheckBreakPoint(break_point_objects)) {
-      break_points_hit->SetElement(break_points_hit_count++,
-                                   *break_point_objects);
+      SetElement(break_points_hit,
+                 break_points_hit_count++,
+                 break_point_objects);
     }
   }
 
-  // Return undefined if no break points where triggered.
+  // Return undefined if no break points were triggered.
   if (break_points_hit_count == 0) {
     return Factory::undefined_value();
   }

src/globals.h

@@ -664,7 +664,7 @@ F FUNCTION_CAST(Address addr) {
 #define TRACK_MEMORY(name)
 #endif
 
-// define used for helping GCC to make better inlining. Don't bother for debug
+// Define used for helping GCC to make better inlining. Don't bother for debug
 // builds. On GCC 3.4.5 using __attribute__((always_inline)) causes compilation
 // errors in debug build.
 #if defined(__GNUC__) && !defined(DEBUG)
@@ -680,6 +680,14 @@ F FUNCTION_CAST(Address addr) {
 #define NO_INLINE(header) header
 #endif
 
+
+#if defined(__GNUC__) && __GNUC__ >= 4
+#define MUST_USE_RESULT __attribute__ ((warn_unused_result))
+#else
+#define MUST_USE_RESULT
+#endif
+
+
 // Feature flags bit positions. They are mostly based on the CPUID spec.
 // (We assign CPUID itself to one of the currently reserved bits --
 // feel free to change this if needed.)

src/liveedit.cc

@@ -739,7 +739,7 @@ void LiveEdit::WrapSharedFunctionInfos(Handle<JSArray> array) {
     Handle<String> name_handle(String::cast(info->name()));
     info_wrapper.SetProperties(name_handle, info->start_position(),
                                info->end_position(), info);
-    array->SetElement(i, *(info_wrapper.GetJSArray()));
+    SetElement(array, i, info_wrapper.GetJSArray());
   }
 }
 
@@ -1359,8 +1359,9 @@ static const char* DropActivationsInActiveThread(
   for (int i = 0; i < array_len; i++) {
     if (result->GetElement(i) ==
         Smi::FromInt(LiveEdit::FUNCTION_BLOCKED_ON_ACTIVE_STACK)) {
-      result->SetElement(i, Smi::FromInt(
-          LiveEdit::FUNCTION_REPLACED_ON_ACTIVE_STACK));
+      Handle<Object> replaced(
+          Smi::FromInt(LiveEdit::FUNCTION_REPLACED_ON_ACTIVE_STACK));
+      SetElement(result, i, replaced);
     }
   }
   return NULL;

src/objects.cc

@@ -54,7 +54,8 @@ const int kGetterIndex = 0;
 const int kSetterIndex = 1;
 
 
-static Object* CreateJSValue(JSFunction* constructor, Object* value) {
+MUST_USE_RESULT static Object* CreateJSValue(JSFunction* constructor,
+                                             Object* value) {
   Object* result = Heap::AllocateJSObject(constructor);
   if (result->IsFailure()) return result;
   JSValue::cast(result)->set_value(value);

src/parser.cc

@@ -4282,7 +4282,11 @@ Expression* Parser::NewThrowError(Handle<String> constructor,
   for (int i = 0; i < argc; i++) {
     Handle<Object> element = arguments[i];
     if (!element.is_null()) {
-      array->SetFastElement(i, *element);
+      Object* ok = array->SetFastElement(i, *element);
+      USE(ok);  // Don't get an unused variable warning.
+      // We know this doesn't cause a GC here because we allocated the JSArray
+      // large enough.
+      ASSERT(!ok->IsFailure());
     }
   }
   ZoneList<Expression*>* args = new ZoneList<Expression*>(2);

src/runtime.cc

@@ -98,7 +98,7 @@ namespace internal {
 static StaticResource<StringInputBuffer> runtime_string_input_buffer;
 
 
-static Object* DeepCopyBoilerplate(JSObject* boilerplate) {
+MUST_USE_RESULT static Object* DeepCopyBoilerplate(JSObject* boilerplate) {
   StackLimitCheck check;
   if (check.HasOverflowed()) return Top::StackOverflow();
 
@@ -980,7 +980,9 @@ static Object* Runtime_DeclareContextSlot(Arguments args) {
             context->set(index, *initial_value);
           }
         } else {
-          Handle<JSObject>::cast(holder)->SetElement(index, *initial_value);
+          // The holder is an arguments object.
+          Handle<JSObject> arguments(Handle<JSObject>::cast(holder));
+          SetElement(arguments, index, initial_value);
         }
       } else {
         // Slow case: The property is not in the FixedArray part of the context.
@@ -1238,7 +1240,8 @@ static Object* Runtime_InitializeConstContextSlot(Arguments args) {
     } else {
       // The holder is an arguments object.
       ASSERT((attributes & READ_ONLY) == 0);
-      Handle<JSObject>::cast(holder)->SetElement(index, *value);
+      Handle<JSObject> arguments(Handle<JSObject>::cast(holder));
+      SetElement(arguments, index, value);
     }
     return *value;
   }
@@ -4105,7 +4108,8 @@ static Object* Runtime_DefineOrRedefineAccessorProperty(Arguments args) {
   if (result.IsProperty() &&
       (result.type() == FIELD || result.type() == NORMAL
        || result.type() == CONSTANT_FUNCTION)) {
-    obj->DeleteProperty(name, JSObject::NORMAL_DELETION);
+    Object* ok = obj->DeleteProperty(name, JSObject::NORMAL_DELETION);
+    if (ok->IsFailure()) return ok;
   }
   return obj->DefineAccessor(name, flag_setter->value() == 0, fun, attr);
 }

test/cctest/test-heap.cc

@@ -637,22 +637,27 @@ TEST(JSArray) {
   // Allocate the object.
   Handle<JSObject> object = Factory::NewJSObject(function);
   Handle<JSArray> array = Handle<JSArray>::cast(object);
-  array->Initialize(0);
+  Object* ok = array->Initialize(0);
+  // We just initialized the VM, no heap allocation failure yet.
+  CHECK(!ok->IsFailure());
 
   // Set array length to 0.
-  array->SetElementsLength(Smi::FromInt(0));
+  ok = array->SetElementsLength(Smi::FromInt(0));
+  CHECK(!ok->IsFailure());
   CHECK_EQ(Smi::FromInt(0), array->length());
   CHECK(array->HasFastElements());  // Must be in fast mode.
 
   // array[length] = name.
-  array->SetElement(0, *name);
+  ok = array->SetElement(0, *name);
+  CHECK(!ok->IsFailure());
   CHECK_EQ(Smi::FromInt(1), array->length());
   CHECK_EQ(array->GetElement(0), *name);
 
   // Set array length with larger than smi value.
   Handle<Object> length =
       Factory::NewNumberFromUint(static_cast<uint32_t>(Smi::kMaxValue) + 1);
-  array->SetElementsLength(*length);
+  ok = array->SetElementsLength(*length);
+  CHECK(!ok->IsFailure());
 
   uint32_t int_length = 0;
   CHECK(length->ToArrayIndex(&int_length));
@@ -660,7 +665,8 @@ TEST(JSArray) {
   CHECK(array->HasDictionaryElements());  // Must be in slow mode.
 
   // array[length] = name.
-  array->SetElement(int_length, *name);
+  ok = array->SetElement(int_length, *name);
+  CHECK(!ok->IsFailure());
   uint32_t new_int_length = 0;
   CHECK(array->length()->ToArrayIndex(&new_int_length));
   CHECK_EQ(static_cast<double>(int_length), new_int_length - 1);
@@ -684,8 +690,11 @@ TEST(JSObjectCopy) {
   obj->SetProperty(*first, Smi::FromInt(1), NONE);
   obj->SetProperty(*second, Smi::FromInt(2), NONE);
 
-  obj->SetElement(0, *first);
-  obj->SetElement(1, *second);
+  Object* ok = obj->SetElement(0, *first);
+  CHECK(!ok->IsFailure());
+
+  ok = obj->SetElement(1, *second);
+  CHECK(!ok->IsFailure());
 
   // Make the clone.
   Handle<JSObject> clone = Copy(obj);
@@ -701,8 +710,10 @@ TEST(JSObjectCopy) {
   clone->SetProperty(*first, Smi::FromInt(2), NONE);
   clone->SetProperty(*second, Smi::FromInt(1), NONE);
 
-  clone->SetElement(0, *second);
-  clone->SetElement(1, *first);
+  ok = clone->SetElement(0, *second);
+  CHECK(!ok->IsFailure());
+  ok = clone->SetElement(1, *first);
+  CHECK(!ok->IsFailure());
 
   CHECK_EQ(obj->GetElement(1), clone->GetElement(0));
   CHECK_EQ(obj->GetElement(0), clone->GetElement(1));

test/mjsunit/const-eval-init.js

@@ -67,7 +67,9 @@ function testAssignmentArgument(x) {
   assertEquals(7, x);
 }
 
-testAssignmentArgument();
+for (var i = 0; i < 10000; i++) {
+  testAssignmentArgument();
+}
 assertEquals(6, x);
 
 __defineSetter__('x', function() { throw 42; });