From: yangguo@chromium.org Date: Wed, 24 Jul 2013 12:16:02 +0000 (+0000) Subject: Restore test and behavior prior to deferred stack trace formatting. X-Git-Tag: upstream/4.7.83~13227 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=eaedafad4bfd43ea20a75ee459263984f7cb28b2;p=platform%2Fupstream%2Fv8.git Restore test and behavior prior to deferred stack trace formatting. R=mstarzinger@chromium.org TEST=stack-traces-overflow.js Review URL: https://codereview.chromium.org/19805003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15856 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/messages.js b/src/messages.js index 761b311..8b647dd 100644 --- a/src/messages.js +++ b/src/messages.js @@ -1145,24 +1145,29 @@ function captureStackTrace(obj, cons_opt) { } var error_string = FormatErrorString(obj); - // Note that 'obj' and 'this' maybe different when called on objects that - // have the error object on its prototype chain. The getter replaces itself - // with a data property as soon as the stack trace has been formatted. - // The getter must not change the object layout as it may be called after GC. + // The holder of this getter ('obj') may not be the receiver ('this'). + // When this getter is called the first time, we use the context values to + // format a stack trace string and turn this accessor pair into a data + // property (on the holder). var getter = function() { - if (IS_STRING(stack)) return stack; // Stack is still a raw array awaiting to be formatted. - stack = FormatStackTrace(error_string, GetStackFrames(stack)); - // Release context value. - error_string = void 0; - return stack; + var result = FormatStackTrace(error_string, GetStackFrames(stack)); + // Turn this accessor into a data property. + %DefineOrRedefineDataProperty(obj, 'stack', result, NONE); + // Release context values. + stack = error_string = void 0; + return result; }; - %MarkOneShotGetter(getter); - // The 'stack' property of the receiver is set as data property. If - // the receiver is the same as holder, this accessor pair is replaced. + // Set the 'stack' property on the receiver. If the receiver is the same as + // holder of this setter, the accessor pair is turned into a data property. var setter = function(v) { + // Set data property on the receiver (not necessarily holder). %DefineOrRedefineDataProperty(this, 'stack', v, NONE); + if (this === obj) { + // Release context values if holder is the same as the receiver. + stack = error_string = void 0; + } }; %DefineOrRedefineAccessorProperty(obj, 'stack', getter, setter, DONT_ENUM); @@ -1300,38 +1305,36 @@ InstallFunctions($Error.prototype, DONT_ENUM, ['toString', ErrorToString]); function SetUpStackOverflowBoilerplate() { var boilerplate = MakeRangeError('stack_overflow', []); - // The raw stack trace is stored as hidden property of the copy of this - // boilerplate error object. Note that the receiver 'this' may not be that - // error object copy, but can be found on the prototype chain of 'this'. - // When the stack trace is formatted, this accessor property is replaced by - // a data property. var error_string = boilerplate.name + ": " + boilerplate.message; - // The getter must not change the object layout as it may be called after GC. - function getter() { + // The raw stack trace is stored as a hidden property on the holder of this + // getter, which may not be the same as the receiver. Find the holder to + // retrieve the raw stack trace and then turn this accessor pair into a + // data property. + var getter = function() { var holder = this; while (!IS_ERROR(holder)) { holder = %GetPrototype(holder); - if (holder == null) return MakeSyntaxError('illegal_access', []); + if (IS_NULL(holder)) return MakeSyntaxError('illegal_access', []); } - var stack = %GetOverflowedStackTrace(holder); - if (IS_STRING(stack)) return stack; - if (IS_ARRAY(stack)) { - var result = FormatStackTrace(error_string, GetStackFrames(stack)); - %SetOverflowedStackTrace(holder, result); - return result; - } - return void 0; - } - %MarkOneShotGetter(getter); + var stack = %GetAndClearOverflowedStackTrace(holder); + // We may not have captured any stack trace. + if (IS_UNDEFINED(stack)) return stack; + + var result = FormatStackTrace(error_string, GetStackFrames(stack)); + // Replace this accessor with a data property. + %DefineOrRedefineDataProperty(holder, 'stack', result, NONE); + return result; + }; - // The 'stack' property of the receiver is set as data property. If - // the receiver is the same as holder, this accessor pair is replaced. - function setter(v) { + // Set the 'stack' property on the receiver. If the receiver is the same as + // holder of this setter, the accessor pair is turned into a data property. + var setter = function(v) { %DefineOrRedefineDataProperty(this, 'stack', v, NONE); - // Release the stack trace that is stored as hidden property, if exists. - %SetOverflowedStackTrace(this, void 0); - } + // Tentatively clear the hidden property. If the receiver is the same as + // holder, we release the raw stack trace this way. + %GetAndClearOverflowedStackTrace(this); + }; %DefineOrRedefineAccessorProperty( boilerplate, 'stack', getter, setter, DONT_ENUM); diff --git a/src/runtime.cc b/src/runtime.cc index 0c88cae..ddfa474 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -13415,50 +13415,21 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CollectStackTrace) { } -// Mark a function to recognize when called after GC to format the stack trace. -RUNTIME_FUNCTION(MaybeObject*, Runtime_MarkOneShotGetter) { - HandleScope scope(isolate); - ASSERT_EQ(args.length(), 1); - CONVERT_ARG_HANDLE_CHECKED(JSFunction, fun, 0); - Handle key = isolate->factory()->hidden_stack_trace_string(); - JSObject::SetHiddenProperty(fun, key, key); - return *fun; -} - - -// Retrieve the stack trace. This could be the raw stack trace collected -// on stack overflow or the already formatted stack trace string. -RUNTIME_FUNCTION(MaybeObject*, Runtime_GetOverflowedStackTrace) { +// Retrieve the stack trace. This is the raw stack trace that yet has to +// be formatted. Since we only need this once, clear it afterwards. +RUNTIME_FUNCTION(MaybeObject*, Runtime_GetAndClearOverflowedStackTrace) { HandleScope scope(isolate); ASSERT_EQ(args.length(), 1); CONVERT_ARG_CHECKED(JSObject, error_object, 0); String* key = isolate->heap()->hidden_stack_trace_string(); Object* result = error_object->GetHiddenProperty(key); - if (result->IsTheHole()) result = isolate->heap()->undefined_value(); - RUNTIME_ASSERT(result->IsJSArray() || - result->IsString() || - result->IsUndefined()); + if (result->IsTheHole()) return isolate->heap()->undefined_value(); + RUNTIME_ASSERT(result->IsJSArray() || result->IsUndefined()); + error_object->DeleteHiddenProperty(key); return result; } -// Set or clear the stack trace attached to an stack overflow error object. -RUNTIME_FUNCTION(MaybeObject*, Runtime_SetOverflowedStackTrace) { - HandleScope scope(isolate); - ASSERT_EQ(args.length(), 2); - CONVERT_ARG_HANDLE_CHECKED(JSObject, error_object, 0); - CONVERT_ARG_HANDLE_CHECKED(HeapObject, value, 1); - Handle key = isolate->factory()->hidden_stack_trace_string(); - if (value->IsUndefined()) { - error_object->DeleteHiddenProperty(*key); - } else { - RUNTIME_ASSERT(value->IsString()); - JSObject::SetHiddenProperty(error_object, key, value); - } - return *error_object; -} - - // Returns V8 version as a string. RUNTIME_FUNCTION(MaybeObject*, Runtime_GetV8Version) { SealHandleScope shs(isolate); diff --git a/src/runtime.h b/src/runtime.h index 2fe9b0e..400145f 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -244,9 +244,7 @@ namespace internal { F(FunctionIsBuiltin, 1, 1) \ F(GetScript, 1, 1) \ F(CollectStackTrace, 3, 1) \ - F(MarkOneShotGetter, 1, 1) \ - F(GetOverflowedStackTrace, 1, 1) \ - F(SetOverflowedStackTrace, 2, 1) \ + F(GetAndClearOverflowedStackTrace, 1, 1) \ F(GetV8Version, 0, 1) \ \ F(ClassOf, 1, 1) \ diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 9e8f608..0d3ff15 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -49,9 +49,6 @@ test-log/EquivalenceOfLoggingAndTraversal: PASS || FAIL test-weakmaps/Shrinking: FAIL test-weaksets/WeakSet_Shrinking: FAIL -# Deferred stack trace formatting is temporarily disabled. -test-heap/ReleaseStackTraceData: PASS || FAIL - # Boot up memory use is bloated in debug mode. test-mark-compact/BootUpMemoryUse: PASS, PASS || FAIL if $mode == debug diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index a55238b..6af9962 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -2820,20 +2820,26 @@ class SourceResource: public v8::String::ExternalAsciiStringResource { }; -void ReleaseStackTraceDataTest(const char* source) { +void ReleaseStackTraceDataTest(const char* source, const char* accessor) { // Test that the data retained by the Error.stack accessor is released // after the first time the accessor is fired. We use external string // to check whether the data is being released since the external string // resource's callback is fired when the external string is GC'ed. + FLAG_use_ic = false; // ICs retain objects. CcTest::InitializeVM(); v8::HandleScope scope(CcTest::isolate()); SourceResource* resource = new SourceResource(i::StrDup(source)); { v8::HandleScope scope(CcTest::isolate()); v8::Handle source_string = v8::String::NewExternal(resource); + HEAP->CollectAllAvailableGarbage(); v8::Script::Compile(source_string)->Run(); CHECK(!resource->IsDisposed()); } + // HEAP->CollectAllAvailableGarbage(); + CHECK(!resource->IsDisposed()); + + CompileRun(accessor); HEAP->CollectAllAvailableGarbage(); // External source has been released. @@ -2855,8 +2861,33 @@ TEST(ReleaseStackTraceData) { "} catch (e) { " " error = e; " "} "; - ReleaseStackTraceDataTest(source1); - ReleaseStackTraceDataTest(source2); + static const char* source3 = "var error = null; " + /* Normal Error */ "try { " + /* as prototype */ " throw new Error(); " + "} catch (e) { " + " error = {}; " + " error.__proto__ = e; " + "} "; + static const char* source4 = "var error = null; " + /* Stack overflow */ "try { " + /* as prototype */ " (function f() { f(); })(); " + "} catch (e) { " + " error = {}; " + " error.__proto__ = e; " + "} "; + static const char* getter = "error.stack"; + static const char* setter = "error.stack = 0"; + + ReleaseStackTraceDataTest(source1, setter); + ReleaseStackTraceDataTest(source2, setter); + // We do not test source3 and source4 with setter, since the setter is + // supposed to (untypically) write to the receiver, not the holder. This is + // to emulate the behavior of a data property. + + ReleaseStackTraceDataTest(source1, getter); + ReleaseStackTraceDataTest(source2, getter); + ReleaseStackTraceDataTest(source3, getter); + ReleaseStackTraceDataTest(source4, getter); } diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 839aa3b..37e6e0f 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -43,9 +43,6 @@ regress/regress-524: SKIP # This test non-deterministically runs out of memory on Windows ia32. regress/regress-crbug-160010: SKIP -# Deferred stack trace formatting is temporarily disabled. -stack-traces-gc: PASS || FAIL - ############################################################################## # Too slow in debug mode with --stress-opt mode. compiler/regress-stacktrace-methods: PASS, SKIP if $mode == debug diff --git a/test/mjsunit/stack-traces-gc.js b/test/mjsunit/stack-traces-gc.js deleted file mode 100644 index dd878f2..0000000 --- a/test/mjsunit/stack-traces-gc.js +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright 2012 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. - -// Flags: --expose-gc --allow-natives-syntax - -var fired = []; -for (var i = 0; i < 100; i++) fired[i] = false; - -function getter_function(i) { - return %MarkOneShotGetter( function() { fired[i] = true; } ); -} - -// Error objects that die young. -for (var i = 0; i < 100; i++) { - var error = new Error(); - // Replace the getter to observe whether it has been fired, - // and disguise it as original getter. - var getter = getter_function(i); - error.__defineGetter__("stack", getter); - - error = undefined; -} - -gc(); -for (var i = 0; i < 100; i++) { - assertFalse(fired[i]); -} - -// Error objects that are kept alive. -var array = []; -for (var i = 0; i < 100; i++) { - var error = new Error(); - var getter = getter_function(i); - // Replace the getter to observe whether it has been fired, - // and disguise it as original getter. - error.__defineGetter__("stack", getter); - - array.push(error); - error = undefined; -} - -gc(); -// We don't expect all stack traces to be formatted after only one GC. -assertTrue(fired[0]); - -for (var i = 0; i < 10; i++) gc(); -for (var i = 0; i < 100; i++) assertTrue(fired[i]); - -// Error objects with custom stack getter. -var custom_error = new Error(); -var custom_getter_fired = false; -custom_error.__defineGetter__("stack", - function() { custom_getter_fired = true; }); -gc(); -assertFalse(custom_getter_fired); - -// Check that formatting caused by GC is not somehow observable. -var error; - -var obj = { foo: function foo() { throw new Error(); } }; - -try { - obj.foo(); -} catch (e) { - delete obj.foo; - Object.defineProperty(obj, 'foo', { - get: function() { assertUnreachable(); } - }); - error = e; -} - -gc(); - -Object.defineProperty(Array.prototype, '0', { - get: function() { assertUnreachable(); } -}); - -try { - throw new Error(); -} catch (e) { - error = e; -} - -gc(); - -String.prototype.indexOf = function() { assertUnreachable(); }; -String.prototype.lastIndexOf = function() { assertUnreachable(); }; -var obj = { method: function() { throw Error(); } }; -try { - obj.method(); -} catch (e) { - error = e; -} - -gc();