From a47caee095db51c451437bd502a1ee0ecd4a7bda Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Thu, 20 Oct 2011 12:31:33 +0000 Subject: [PATCH] Make builtin functions be skipped in stack traces. Does include exposed builtin functions ("native functions"). Review URL: http://codereview.chromium.org/8345039 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9723 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/bootstrapper.cc | 1 + src/flag-definitions.h | 2 + src/messages.js | 1 + src/runtime.cc | 33 ++++++------- test/mjsunit/stack-traces-2.js | 87 ++++++++++++++++++++++++++++++++++ test/mjsunit/stack-traces.js | 58 +++++++++++++++++++++++ 6 files changed, 164 insertions(+), 18 deletions(-) create mode 100644 test/mjsunit/stack-traces-2.js diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 04eb009c71..84e9377829 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -362,6 +362,7 @@ static Handle InstallFunction(Handle target, if (is_ecma_native) { function->shared()->set_instance_class_name(*symbol); } + function->shared()->set_native(true); return function; } diff --git a/src/flag-definitions.h b/src/flag-definitions.h index c3c06865e0..5c4384385b 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -186,6 +186,8 @@ DEFINE_bool(expose_gc, false, "expose gc extension") DEFINE_bool(expose_externalize_string, false, "expose externalize string extension") DEFINE_int(stack_trace_limit, 10, "number of stack frames to capture") +DEFINE_bool(builtins_in_stack_traces, false, + "show built-in functions in stack traces") DEFINE_bool(disable_native_files, false, "disable builtin natives files") // builtins-ia32.cc diff --git a/src/messages.js b/src/messages.js index be236a4a34..8fc54d69ab 100644 --- a/src/messages.js +++ b/src/messages.js @@ -1126,6 +1126,7 @@ function SetUpError() { return new f(m); } }); + %SetNativeFlag(f); } DefineError(function Error() { }); diff --git a/src/runtime.cc b/src/runtime.cc index c0d357ae2c..c5416ace81 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -2116,7 +2116,6 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetCode) { // Since we don't store the source for this we should never // optimize this. shared->code()->set_optimizable(false); - // Set the code, scope info, formal parameter count, // and the length of the target function. target->shared()->set_code(shared->code()); @@ -12924,34 +12923,32 @@ static bool ShowFrameInStackTrace(StackFrame* raw_frame, Object* caller, bool* seen_caller) { // Only display JS frames. - if (!raw_frame->is_java_script()) + if (!raw_frame->is_java_script()) { return false; + } JavaScriptFrame* frame = JavaScriptFrame::cast(raw_frame); Object* raw_fun = frame->function(); // Not sure when this can happen but skip it just in case. - if (!raw_fun->IsJSFunction()) + if (!raw_fun->IsJSFunction()) { return false; + } if ((raw_fun == caller) && !(*seen_caller)) { *seen_caller = true; return false; } // Skip all frames until we've seen the caller. if (!(*seen_caller)) return false; - // Also, skip the most obvious builtin calls. We recognize builtins - // as (1) functions called with the builtins object as the receiver and - // as (2) functions from native scripts called with undefined as the - // receiver (direct calls to helper functions in the builtins - // code). Some builtin calls (such as Number.ADD which is invoked - // using 'call') are very difficult to recognize so we're leaving - // them in for now. - if (frame->receiver()->IsJSBuiltinsObject()) { - return false; - } - JSFunction* fun = JSFunction::cast(raw_fun); - Object* raw_script = fun->shared()->script(); - if (frame->receiver()->IsUndefined() && raw_script->IsScript()) { - int script_type = Script::cast(raw_script)->type()->value(); - return script_type != Script::TYPE_NATIVE; + // Also, skip non-visible built-in functions and any call with the builtins + // object as receiver, so as to not reveal either the builtins object or + // an internal function. + // The --builtins-in-stack-traces command line flag allows including + // internal call sites in the stack trace for debugging purposes. + if (!FLAG_builtins_in_stack_traces) { + JSFunction* fun = JSFunction::cast(raw_fun); + if (frame->receiver()->IsJSBuiltinsObject() || + (fun->IsBuiltin() && !fun->shared()->native())) { + return false; + } } return true; } diff --git a/test/mjsunit/stack-traces-2.js b/test/mjsunit/stack-traces-2.js new file mode 100644 index 0000000000..165c4dfcec --- /dev/null +++ b/test/mjsunit/stack-traces-2.js @@ -0,0 +1,87 @@ +// Copyright 2011 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: --builtins-in-stack-traces + + +// Poisonous object that throws a reference error if attempted converted to +// a primitive values. +var thrower = { valueOf: function() { FAIL; }, + toString: function() { FAIL; } }; + +// Tests that a native constructor function is included in the +// stack trace. +function testTraceNativeConstructor(nativeFunc) { + var nativeFuncName = nativeFunc.name; + try { + new nativeFunc(thrower); + assertUnreachable(nativeFuncName); + } catch (e) { + assertTrue(e.stack.indexOf(nativeFuncName) >= 0, nativeFuncName); + } +} + +// Tests that a native conversion function is included in the +// stack trace. +function testTraceNativeConversion(nativeFunc) { + var nativeFuncName = nativeFunc.name; + try { + nativeFunc(thrower); + assertUnreachable(nativeFuncName); + } catch (e) { + assertTrue(e.stack.indexOf(nativeFuncName) >= 0, nativeFuncName); + } +} + + +function testNotOmittedBuiltin(throwing, included) { + try { + throwing(); + assertUnreachable(included); + } catch (e) { + assertTrue(e.stack.indexOf(included) >= 0, included); + } +} + + +testTraceNativeConversion(String); // Does ToString on argument. +testTraceNativeConversion(Number); // Does ToNumber on argument. +testTraceNativeConversion(RegExp); // Does ToString on argument. + +testTraceNativeConstructor(String); // Does ToString on argument. +testTraceNativeConstructor(Number); // Does ToNumber on argument. +testTraceNativeConstructor(RegExp); // Does ToString on argument. +testTraceNativeConstructor(Date); // Does ToNumber on argument. + +// QuickSort has builtins object as receiver, and is non-native +// builtin. Should not be omitted with the --builtins-in-stack-traces flag. +testNotOmittedBuiltin(function(){ [thrower, 2].sort(function (a,b) { + (b < a) - (a < b); }); + }, "QuickSort"); + +// Not omitted even though ADD from runtime.js is a non-native builtin. +testNotOmittedBuiltin(function(){ thrower + 2; }, "ADD"); \ No newline at end of file diff --git a/test/mjsunit/stack-traces.js b/test/mjsunit/stack-traces.js index 47a5cc594a..536e71bbb5 100644 --- a/test/mjsunit/stack-traces.js +++ b/test/mjsunit/stack-traces.js @@ -194,6 +194,46 @@ function testErrorsDuringFormatting() { } +// Poisonous object that throws a reference error if attempted converted to +// a primitive values. +var thrower = { valueOf: function() { FAIL; }, + toString: function() { FAIL; } }; + +// Tests that a native constructor function is included in the +// stack trace. +function testTraceNativeConstructor(nativeFunc) { + var nativeFuncName = nativeFunc.name; + try { + new nativeFunc(thrower); + assertUnreachable(nativeFuncName); + } catch (e) { + assertTrue(e.stack.indexOf(nativeFuncName) >= 0, nativeFuncName); + } +} + +// Tests that a native conversion function is included in the +// stack trace. +function testTraceNativeConversion(nativeFunc) { + var nativeFuncName = nativeFunc.name; + try { + nativeFunc(thrower); + assertUnreachable(nativeFuncName); + } catch (e) { + assertTrue(e.stack.indexOf(nativeFuncName) >= 0, nativeFuncName); + } +} + + +function testOmittedBuiltin(throwing, omitted) { + try { + throwing(); + assertUnreachable(omitted); + } catch (e) { + assertTrue(e.stack.indexOf(omitted) < 0, omitted); + } +} + + testTrace("testArrayNative", testArrayNative, ["Array.map (native)"]); testTrace("testNested", testNested, ["at one", "at two", "at three"]); testTrace("testMethodNameInference", testMethodNameInference, ["at Foo.bar"]); @@ -217,3 +257,21 @@ testTrace("testStrippedCustomError", testStrippedCustomError, ["hep-hey"], testCallerCensorship(); testUnintendedCallerCensorship(); testErrorsDuringFormatting(); + +testTraceNativeConversion(String); // Does ToString on argument. +testTraceNativeConversion(Number); // Does ToNumber on argument. +testTraceNativeConversion(RegExp); // Does ToString on argument. + +testTraceNativeConstructor(String); // Does ToString on argument. +testTraceNativeConstructor(Number); // Does ToNumber on argument. +testTraceNativeConstructor(RegExp); // Does ToString on argument. +testTraceNativeConstructor(Date); // Does ToNumber on argument. + +// Omitted because QuickSort has builtins object as receiver, and is non-native +// builtin. +testOmittedBuiltin(function(){ [thrower, 2].sort(function (a,b) { + (b < a) - (a < b); }); + }, "QuickSort"); + +// Omitted because ADD from runtime.js is non-native builtin. +testOmittedBuiltin(function(){ thrower + 2; }, "ADD"); \ No newline at end of file