From 40471b41daf343261fe0a515ce936cd15ad7a354 Mon Sep 17 00:00:00 2001 From: "sgjesse@chromium.org" Date: Wed, 16 Sep 2009 13:09:26 +0000 Subject: [PATCH] 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 --- src/runtime.cc | 74 ++++++++++------------ .../{array-construtor.js => array-constructor.js} | 0 test/mjsunit/mjsunit.status | 1 + 3 files changed, 36 insertions(+), 39 deletions(-) rename test/mjsunit/{array-construtor.js => array-constructor.js} (100%) diff --git a/src/runtime.cc b/src/runtime.cc index 09ca290..0cf2825 100644 --- 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 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 result_callback_obj; + if (result_type == CALLBACKS) { + result_callback_obj = Handle(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 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 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::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 result_callback_obj; - if (result_type == CALLBACKS) { - result_callback_obj = Handle(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 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 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 --- 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 -- 2.7.4