Revision: 2902 Author: [email protected] Date: Wed Sep 16 06:09:26 2009 Log: 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 http://code.google.com/p/v8/source/detail?r=2902 Added: /branches/bleeding_edge/test/mjsunit/array-constructor.js Deleted: /branches/bleeding_edge/test/mjsunit/array-construtor.js Modified: /branches/bleeding_edge/src/runtime.cc /branches/bleeding_edge/test/mjsunit/mjsunit.status ======================================= --- /dev/null +++ /branches/bleeding_edge/test/mjsunit/array-constructor.js Wed Sep 16 06:09:26 2009 @@ -0,0 +1,119 @@ +// Copyright 2008 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + + +var loop_count = 5 + + +for (var i = 0; i < loop_count; i++) { + var a = new Array(); + var b = Array(); + assertEquals(0, a.length); + assertEquals(0, b.length); + for (var k = 0; k < 10; k++) { + assertEquals('undefined', typeof a[k]); + assertEquals('undefined', typeof b[k]); + } +} + + +for (var i = 0; i < loop_count; i++) { + for (var j = 0; j < 100; j++) { + var a = new Array(j); + var b = Array(j); + assertEquals(j, a.length); + assertEquals(j, b.length); + for (var k = 0; k < j; k++) { + assertEquals('undefined', typeof a[k]); + assertEquals('undefined', typeof b[k]); + } + } +} + + +for (var i = 0; i < loop_count; i++) { + a = new Array(0, 1); + assertArrayEquals([0, 1], a); + a = new Array(0, 1, 2); + assertArrayEquals([0, 1, 2], a); + a = new Array(0, 1, 2, 3); + assertArrayEquals([0, 1, 2, 3], a); + a = new Array(0, 1, 2, 3, 4); + assertArrayEquals([0, 1, 2, 3, 4], a); + a = new Array(0, 1, 2, 3, 4, 5); + assertArrayEquals([0, 1, 2, 3, 4, 5], a); + a = new Array(0, 1, 2, 3, 4, 5, 6); + assertArrayEquals([0, 1, 2, 3, 4, 5, 6], a); + a = new Array(0, 1, 2, 3, 4, 5, 6, 7); + assertArrayEquals([0, 1, 2, 3, 4, 5, 6, 7], a); + a = new Array(0, 1, 2, 3, 4, 5, 6, 7, 8); + assertArrayEquals([0, 1, 2, 3, 4, 5, 6, 7, 8], a); + a = new Array(0, 1, 2, 3, 4, 5, 6, 7, 8, 9); + assertArrayEquals([0, 1, 2, 3, 4, 5, 6, 7, 8, 9], a); +} + + +function innerArrayLiteral(n) { + var a = new Array(n); + for (var i = 0; i < n; i++) { + a[i] = i * 2 + 7; + } + return a.join(); +} + + +function testConstructOfSizeSize(n) { + var str = innerArrayLiteral(n); + var a = eval('[' + str + ']'); + var b = eval('new Array(' + str + ')') + var c = eval('Array(' + str + ')') + assertEquals(n, a.length); + assertArrayEquals(a, b); + assertArrayEquals(a, c); +} + + +for (var i = 0; i < loop_count; i++) { + // JSObject::kInitialMaxFastElementArray is 10000. + for (var j = 1000; j < 12000; j += 1000) { + testConstructOfSizeSize(j); + } +} + + +for (var i = 0; i < loop_count; i++) { + assertArrayEquals(['xxx'], new Array('xxx')); + assertArrayEquals(['xxx'], Array('xxx')); + assertArrayEquals([true], new Array(true)); + assertArrayEquals([false], Array(false)); + assertArrayEquals([{a:1}], new Array({a:1})); + assertArrayEquals([{b:2}], Array({b:2})); +} + + +assertThrows('new Array(3.14)'); +assertThrows('Array(2.72)'); ======================================= --- /branches/bleeding_edge/test/mjsunit/array-construtor.js Wed Sep 16 04:17:57 2009 +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright 2008 the V8 project authors. All rights reserved. -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following -// disclaimer in the documentation and/or other materials provided -// with the distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived -// from this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - - -var loop_count = 5 - - -for (var i = 0; i < loop_count; i++) { - var a = new Array(); - var b = Array(); - assertEquals(0, a.length); - assertEquals(0, b.length); - for (var k = 0; k < 10; k++) { - assertEquals('undefined', typeof a[k]); - assertEquals('undefined', typeof b[k]); - } -} - - -for (var i = 0; i < loop_count; i++) { - for (var j = 0; j < 100; j++) { - var a = new Array(j); - var b = Array(j); - assertEquals(j, a.length); - assertEquals(j, b.length); - for (var k = 0; k < j; k++) { - assertEquals('undefined', typeof a[k]); - assertEquals('undefined', typeof b[k]); - } - } -} - - -for (var i = 0; i < loop_count; i++) { - a = new Array(0, 1); - assertArrayEquals([0, 1], a); - a = new Array(0, 1, 2); - assertArrayEquals([0, 1, 2], a); - a = new Array(0, 1, 2, 3); - assertArrayEquals([0, 1, 2, 3], a); - a = new Array(0, 1, 2, 3, 4); - assertArrayEquals([0, 1, 2, 3, 4], a); - a = new Array(0, 1, 2, 3, 4, 5); - assertArrayEquals([0, 1, 2, 3, 4, 5], a); - a = new Array(0, 1, 2, 3, 4, 5, 6); - assertArrayEquals([0, 1, 2, 3, 4, 5, 6], a); - a = new Array(0, 1, 2, 3, 4, 5, 6, 7); - assertArrayEquals([0, 1, 2, 3, 4, 5, 6, 7], a); - a = new Array(0, 1, 2, 3, 4, 5, 6, 7, 8); - assertArrayEquals([0, 1, 2, 3, 4, 5, 6, 7, 8], a); - a = new Array(0, 1, 2, 3, 4, 5, 6, 7, 8, 9); - assertArrayEquals([0, 1, 2, 3, 4, 5, 6, 7, 8, 9], a); -} - - -function innerArrayLiteral(n) { - var a = new Array(n); - for (var i = 0; i < n; i++) { - a[i] = i * 2 + 7; - } - return a.join(); -} - - -function testConstructOfSizeSize(n) { - var str = innerArrayLiteral(n); - var a = eval('[' + str + ']'); - var b = eval('new Array(' + str + ')') - var c = eval('Array(' + str + ')') - assertEquals(n, a.length); - assertArrayEquals(a, b); - assertArrayEquals(a, c); -} - - -for (var i = 0; i < loop_count; i++) { - // JSObject::kInitialMaxFastElementArray is 10000. - for (var j = 1000; j < 12000; j += 1000) { - testConstructOfSizeSize(j); - } -} - - -for (var i = 0; i < loop_count; i++) { - assertArrayEquals(['xxx'], new Array('xxx')); - assertArrayEquals(['xxx'], Array('xxx')); - assertArrayEquals([true], new Array(true)); - assertArrayEquals([false], Array(false)); - assertArrayEquals([{a:1}], new Array({a:1})); - assertArrayEquals([{b:2}], Array({b:2})); -} - - -assertThrows('new Array(3.14)'); -assertThrows('Array(2.72)'); ======================================= --- /branches/bleeding_edge/src/runtime.cc Tue Sep 15 04:51:40 2009 +++ /branches/bleeding_edge/src/runtime.cc Wed Sep 16 06:09:26 2009 @@ -5755,55 +5755,51 @@ 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(); } ======================================= --- /branches/bleeding_edge/test/mjsunit/mjsunit.status Sun Sep 13 23:57:24 2009 +++ /branches/bleeding_edge/test/mjsunit/mjsunit.status Wed Sep 16 06:09:26 2009 @@ -41,6 +41,7 @@ # 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 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
