Fix GC bug and ARM simulator timeout.

In the Runtime_DebugGetPropertyDetails the raw object pointers from a LookupResult could be used after a GC might have happened. Fixed the bug and restructured the code to make it less likely for changes to the code to re-introduce the bug.

Skipped a long running test from the ARM simulator in debug mode (and renamed the test).
Review URL: http://codereview.chromium.org/204039

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

diff --git a/src/runtime.cc b/src/runtime.cc
index 09ca290..0cf2825 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5755,55 +5755,51 @@ static Object* Runtime_DebugGetPropertyDetails(Arguments args) {
   int length = LocalPrototypeChainLength(*obj);
 
   // Try local lookup on each of the objects.
-  LookupResult result;
   Handle<JSObject> jsproto = obj;
   for (int i = 0; i < length; i++) {
+    LookupResult result;
     jsproto->LocalLookup(*name, &result);
     if (result.IsProperty()) {
-      break;
+      // LookupResult is not GC safe as it holds raw object pointers.
+      // GC can happen later in this code so put the required fields into
+      // local variables using handles when required for later use.
+      PropertyType result_type = result.type();
+      Handle<Object> result_callback_obj;
+      if (result_type == CALLBACKS) {
+        result_callback_obj = Handle<Object>(result.GetCallbackObject());
+      }
+      Smi* property_details = result.GetPropertyDetails().AsSmi();
+      // DebugLookupResultValue can cause GC so details from LookupResult needs
+      // to be copied to handles before this.
+      bool caught_exception = false;
+      Object* raw_value = DebugLookupResultValue(*obj, *name, &result,
+                                                 &caught_exception);
+      if (raw_value->IsFailure()) return raw_value;
+      Handle<Object> value(raw_value);
+
+      // If the callback object is a fixed array then it contains JavaScript
+      // getter and/or setter.
+      bool hasJavaScriptAccessors = result_type == CALLBACKS &&
+                                    result_callback_obj->IsFixedArray();
+      Handle<FixedArray> details =
+          Factory::NewFixedArray(hasJavaScriptAccessors ? 5 : 2);
+      details->set(0, *value);
+      details->set(1, property_details);
+      if (hasJavaScriptAccessors) {
+        details->set(2,
+                     caught_exception ? Heap::true_value()
+                                      : Heap::false_value());
+        details->set(3, FixedArray::cast(*result_callback_obj)->get(0));
+        details->set(4, FixedArray::cast(*result_callback_obj)->get(1));
+      }
+
+      return *Factory::NewJSArrayWithElements(details);
     }
     if (i < length - 1) {
       jsproto = Handle<JSObject>(JSObject::cast(jsproto->GetPrototype()));
     }
   }
 
-  if (result.IsProperty()) {
-    // LookupResult is not GC safe as all its members are raw object pointers.
-    // When calling DebugLookupResultValue GC can happen as this might invoke
-    // callbacks. After the call to DebugLookupResultValue the callback object
-    // in the LookupResult might still be needed. Put it into a handle for later
-    // use.
-    PropertyType result_type = result.type();
-    Handle<Object> result_callback_obj;
-    if (result_type == CALLBACKS) {
-      result_callback_obj = Handle<Object>(result.GetCallbackObject());
-    }
-
-    // Find the actual value. Don't use result after this call as it's content
-    // can be invalid.
-    bool caught_exception = false;
-    Object* value = DebugLookupResultValue(*obj, *name, &result,
-                                           &caught_exception);
-    if (value->IsFailure()) return value;
-    Handle<Object> value_handle(value);
-
-    // If the callback object is a fixed array then it contains JavaScript
-    // getter and/or setter.
-    bool hasJavaScriptAccessors = result_type == CALLBACKS &&
-                                  result_callback_obj->IsFixedArray();
-    Handle<FixedArray> details =
-        Factory::NewFixedArray(hasJavaScriptAccessors ? 5 : 2);
-    details->set(0, *value_handle);
-    details->set(1, result.GetPropertyDetails().AsSmi());
-    if (hasJavaScriptAccessors) {
-      details->set(2,
-                   caught_exception ? Heap::true_value() : Heap::false_value());
-      details->set(3, FixedArray::cast(result.GetCallbackObject())->get(0));
-      details->set(4, FixedArray::cast(result.GetCallbackObject())->get(1));
-    }
-
-    return *Factory::NewJSArrayWithElements(details);
-  }
   return Heap::undefined_value();
 }
 

diff --git a/test/mjsunit/array-construtor.js b/test/mjsunit/array-constructor.js
similarity index 100%
rename from test/mjsunit/array-construtor.js
rename to test/mjsunit/array-constructor.js

diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index 13e69ae..0b069cc 100644 (file)
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -41,6 +41,7 @@ big-object-literal: PASS, SKIP if ($arch == arm)
 # Slow tests which times out in debug mode.
 try: PASS, SKIP if $mode == debug
 debug-scripts-request: PASS, SKIP if $mode == debug
+array-constructor: PASS, SKIP if $mode == debug
 
 # Flaky test that can hit compilation-time stack overflow in debug mode.
 unicode-test: PASS, (PASS || FAIL) if $mode == debug