Revision: 10618
Author: [email protected]
Date: Tue Feb 7 01:31:06 2012
Log: When rethrowing an exception, print the stack trace of its
original site instead of rethrow site.
BUG=60240
Review URL: https://chromiumcodereview.appspot.com/9310122
http://code.google.com/p/v8/source/detail?r=10618
Modified:
/branches/bleeding_edge/src/heap.h
/branches/bleeding_edge/src/isolate.cc
/branches/bleeding_edge/src/isolate.h
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/runtime.h
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/heap.h Fri Feb 3 06:16:40 2012
+++ /branches/bleeding_edge/src/heap.h Tue Feb 7 01:31:06 2012
@@ -241,9 +241,10 @@
V(use_strict, "use strict") \
V(dot_symbol, ".") \
V(anonymous_function_symbol, "(anonymous function)") \
- V(compare_ic_symbol, ".compare_ic") \
+ V(compare_ic_symbol, ".compare_ic") \
V(infinity_symbol, "Infinity") \
- V(minus_infinity_symbol, "-Infinity")
+ V(minus_infinity_symbol, "-Infinity") \
+ V(hidden_stack_trace_symbol, "v8::hidden_stack_trace")
// Forward declarations.
class GCTracer;
=======================================
--- /branches/bleeding_edge/src/isolate.cc Mon Feb 6 00:59:43 2012
+++ /branches/bleeding_edge/src/isolate.cc Tue Feb 7 01:31:06 2012
@@ -540,6 +540,18 @@
return factory()->empty_symbol();
}
}
+
+
+void Isolate::CaptureAndSetCurrentStackTraceFor(Handle<JSObject>
error_object) {
+ if (capture_stack_trace_for_uncaught_exceptions_) {
+ // Capture stack trace for a detailed exception message.
+ Handle<String> key = factory()->hidden_stack_trace_symbol();
+ Handle<JSArray> stack_trace = CaptureCurrentStackTrace(
+ stack_trace_for_uncaught_exceptions_frame_limit_,
+ stack_trace_for_uncaught_exceptions_options_);
+ JSObject::SetHiddenProperty(error_object, key, stack_trace);
+ }
+}
Handle<JSArray> Isolate::CaptureCurrentStackTrace(
@@ -1037,22 +1049,39 @@
}
-void Isolate::DoThrow(MaybeObject* exception, MessageLocation* location) {
+bool Isolate::IsErrorObject(Handle<Object> obj) {
+ if (!obj->IsJSObject()) return false;
+
+ String* error_key = *(factory()->LookupAsciiSymbol("$Error"));
+ Object* error_constructor =
+ js_builtins_object()->GetPropertyNoExceptionThrown(error_key);
+
+ for (Object* prototype = *obj; !prototype->IsNull();
+ prototype = prototype->GetPrototype()) {
+ if (!prototype->IsJSObject()) return false;
+ if (JSObject::cast(prototype)->map()->constructor() ==
error_constructor) {
+ return true;
+ }
+ }
+ return false;
+}
+
+
+void Isolate::DoThrow(Object* exception, MessageLocation* location) {
ASSERT(!has_pending_exception());
HandleScope scope;
- Object* exception_object = Smi::FromInt(0);
- bool is_object = exception->ToObject(&exception_object);
- Handle<Object> exception_handle(exception_object);
+ Handle<Object> exception_handle(exception);
// Determine reporting and whether the exception is caught externally.
bool catchable_by_javascript = is_catchable_by_javascript(exception);
- // Only real objects can be caught by JS.
- ASSERT(!catchable_by_javascript || is_object);
bool can_be_caught_externally = false;
bool should_report_exception =
ShouldReportException(&can_be_caught_externally,
catchable_by_javascript);
bool report_exception = catchable_by_javascript &&
should_report_exception;
+ bool try_catch_needs_message =
+ can_be_caught_externally && try_catch_handler()->capture_message_;
+ bool bootstrapping = bootstrapper()->IsActive();
#ifdef ENABLE_DEBUGGER_SUPPORT
// Notify debugger of exception.
@@ -1061,34 +1090,52 @@
}
#endif
- // Generate the message.
- Handle<Object> message_obj;
- MessageLocation potential_computed_location;
- bool try_catch_needs_message =
- can_be_caught_externally &&
- try_catch_handler()->capture_message_;
+ // Generate the message if required.
if (report_exception || try_catch_needs_message) {
+ MessageLocation potential_computed_location;
if (location == NULL) {
- // If no location was specified we use a computed one instead
+ // If no location was specified we use a computed one instead.
ComputeLocation(&potential_computed_location);
location = &potential_computed_location;
}
- if (!bootstrapper()->IsActive()) {
- // It's not safe to try to make message objects or collect stack
- // traces while the bootstrapper is active since the infrastructure
- // may not have been properly initialized.
+ // It's not safe to try to make message objects or collect stack traces
+ // while the bootstrapper is active since the infrastructure may not
have
+ // been properly initialized.
+ if (!bootstrapping) {
Handle<String> stack_trace;
if (FLAG_trace_exception) stack_trace = StackTraceString();
Handle<JSArray> stack_trace_object;
- if (report_exception &&
capture_stack_trace_for_uncaught_exceptions_) {
+ if (capture_stack_trace_for_uncaught_exceptions_) {
+ if (IsErrorObject(exception_handle)) {
+ // We fetch the stack trace that corresponds to this error
object.
+ String* key = heap()->hidden_stack_trace_symbol();
+ Object* stack_property =
+ JSObject::cast(*exception_handle)->GetHiddenProperty(key);
+ // Property lookup may have failed. In this case it's probably
not
+ // a valid Error object.
+ if (stack_property->IsJSArray()) {
+ stack_trace_object =
Handle<JSArray>(JSArray::cast(stack_property));
+ }
+ }
+ if (stack_trace_object.is_null()) {
+ // Not an error object, we capture at throw site.
stack_trace_object = CaptureCurrentStackTrace(
stack_trace_for_uncaught_exceptions_frame_limit_,
stack_trace_for_uncaught_exceptions_options_);
- }
- ASSERT(is_object); // Can't use the handle unless there's a real
object.
- message_obj = MessageHandler::MakeMessageObject("uncaught_exception",
- location, HandleVector<Object>(&exception_handle, 1),
stack_trace,
+ }
+ }
+ Handle<Object> message_obj = MessageHandler::MakeMessageObject(
+ "uncaught_exception",
+ location,
+ HandleVector<Object>(&exception_handle, 1),
+ stack_trace,
stack_trace_object);
+ thread_local_top()->pending_message_obj_ = *message_obj;
+ if (location != NULL) {
+ thread_local_top()->pending_message_script_ = *location->script();
+ thread_local_top()->pending_message_start_pos_ =
location->start_pos();
+ thread_local_top()->pending_message_end_pos_ = location->end_pos();
+ }
} else if (location != NULL && !location->script().is_null()) {
// We are bootstrapping and caught an error where the location is set
// and we have a script for the location.
@@ -1104,30 +1151,13 @@
// Save the message for reporting if the the exception remains uncaught.
thread_local_top()->has_pending_message_ = report_exception;
- if (!message_obj.is_null()) {
- thread_local_top()->pending_message_obj_ = *message_obj;
- if (location != NULL) {
- thread_local_top()->pending_message_script_ = *location->script();
- thread_local_top()->pending_message_start_pos_ =
location->start_pos();
- thread_local_top()->pending_message_end_pos_ = location->end_pos();
- }
- }
// Do not forget to clean catcher_ if currently thrown exception cannot
// be caught. If necessary, ReThrow will update the catcher.
thread_local_top()->catcher_ = can_be_caught_externally ?
try_catch_handler() : NULL;
- // NOTE: Notifying the debugger or generating the message
- // may have caused new exceptions. For now, we just ignore
- // that and set the pending exception to the original one.
- if (is_object) {
- set_pending_exception(*exception_handle);
- } else {
- // Failures are not on the heap so they neither need nor work with
handles.
- ASSERT(exception_handle->IsFailure());
- set_pending_exception(exception);
- }
+ set_pending_exception(*exception_handle);
}
=======================================
--- /branches/bleeding_edge/src/isolate.h Mon Feb 6 00:59:43 2012
+++ /branches/bleeding_edge/src/isolate.h Tue Feb 7 01:31:06 2012
@@ -703,6 +703,8 @@
int frame_limit,
StackTrace::StackTraceOptions options);
+ void CaptureAndSetCurrentStackTraceFor(Handle<JSObject> error_object);
+
// Returns if the top context may access the given global object. If
// the result is false, the pending exception is guaranteed to be
// set.
@@ -729,7 +731,7 @@
// Promote a scheduled exception to pending. Asserts
has_scheduled_exception.
Failure* PromoteScheduledException();
- void DoThrow(MaybeObject* exception, MessageLocation* location);
+ void DoThrow(Object* exception, MessageLocation* location);
// Checks if exception should be reported and finds out if it's
// caught externally.
bool ShouldReportException(bool* can_be_caught_externally,
@@ -1141,6 +1143,10 @@
void InitializeDebugger();
+ // Traverse prototype chain to find out whether the object is derived
from
+ // the Error object.
+ bool IsErrorObject(Handle<Object> obj);
+
int stack_trace_nesting_level_;
StringStream* incomplete_message_;
// The preallocated memory thread singleton.
=======================================
--- /branches/bleeding_edge/src/messages.js Mon Dec 5 06:43:28 2011
+++ /branches/bleeding_edge/src/messages.js Tue Feb 7 01:31:06 2012
@@ -1078,9 +1078,9 @@
if (stackTraceLimit < 0 || stackTraceLimit > 10000) {
stackTraceLimit = 10000;
}
- var raw_stack = %CollectStackTrace(cons_opt
- ? cons_opt
- : captureStackTrace, stackTraceLimit);
+ var raw_stack = %CollectStackTrace(obj,
+ cons_opt ? cons_opt :
captureStackTrace,
+ stackTraceLimit);
DefineOneShotAccessor(obj, 'stack', function (obj) {
return FormatRawStackTrace(obj, raw_stack);
});
=======================================
--- /branches/bleeding_edge/src/objects.h Mon Feb 6 05:54:46 2012
+++ /branches/bleeding_edge/src/objects.h Tue Feb 7 01:31:06 2012
@@ -1638,7 +1638,7 @@
Handle<String> key,
Handle<Object> value);
// Returns a failure if a GC is required.
- MaybeObject* SetHiddenProperty(String* key, Object* value);
+ MUST_USE_RESULT MaybeObject* SetHiddenProperty(String* key, Object*
value);
// Gets the value of a hidden property with the given key. Returns
undefined
// if the property doesn't exist (or if called on a detached proxy),
// otherwise returns the value set for the key.
=======================================
--- /branches/bleeding_edge/src/runtime.cc Mon Feb 6 05:54:46 2012
+++ /branches/bleeding_edge/src/runtime.cc Tue Feb 7 01:31:06 2012
@@ -13263,9 +13263,10 @@
// element segments each containing a receiver, function, code and
// native code offset.
RUNTIME_FUNCTION(MaybeObject*, Runtime_CollectStackTrace) {
- ASSERT_EQ(args.length(), 2);
- Handle<Object> caller = args.at<Object>(0);
- CONVERT_NUMBER_CHECKED(int32_t, limit, Int32, args[1]);
+ ASSERT_EQ(args.length(), 3);
+ CONVERT_ARG_CHECKED(JSObject, error_object, 0);
+ Handle<Object> caller = args.at<Object>(1);
+ CONVERT_NUMBER_CHECKED(int32_t, limit, Int32, args[2]);
HandleScope scope(isolate);
Factory* factory = isolate->factory();
@@ -13315,6 +13316,8 @@
iter.Advance();
}
Handle<JSArray> result = factory->NewJSArrayWithElements(elements);
+ // Capture and attach a more detailed stack trace if necessary.
+ isolate->CaptureAndSetCurrentStackTraceFor(error_object);
result->set_length(Smi::FromInt(cursor));
return *result;
}
=======================================
--- /branches/bleeding_edge/src/runtime.h Tue Jan 17 06:29:17 2012
+++ /branches/bleeding_edge/src/runtime.h Tue Feb 7 01:31:06 2012
@@ -229,7 +229,7 @@
F(FunctionIsAPIFunction, 1, 1) \
F(FunctionIsBuiltin, 1, 1) \
F(GetScript, 1, 1) \
- F(CollectStackTrace, 2, 1) \
+ F(CollectStackTrace, 3, 1) \
F(GetV8Version, 0, 1) \
\
F(ClassOf, 1, 1) \
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Wed Feb 1 04:58:34 2012
+++ /branches/bleeding_edge/test/cctest/test-api.cc Tue Feb 7 01:31:06 2012
@@ -13533,6 +13533,137 @@
CompileRun("throw 'exception';");
v8::V8::SetCaptureStackTraceForUncaughtExceptions(false);
}
+
+
+static void RethrowStackTraceHandler(v8::Handle<v8::Message> message,
+ v8::Handle<v8::Value> data) {
+ // Use the frame where JavaScript is called from.
+ v8::Handle<v8::StackTrace> stack_trace = message->GetStackTrace();
+ CHECK(!stack_trace.IsEmpty());
+ int frame_count = stack_trace->GetFrameCount();
+ CHECK_EQ(3, frame_count);
+ int line_number[] = {1, 2, 5};
+ for (int i = 0; i < frame_count; i++) {
+ CHECK_EQ(line_number[i], stack_trace->GetFrame(i)->GetLineNumber());
+ }
+}
+
+
+// Test that we only return the stack trace at the site where the exception
+// is first thrown (not where it is rethrown).
+TEST(RethrowStackTrace) {
+ v8::HandleScope scope;
+ LocalContext env;
+ // We make sure that
+ // - the stack trace of the ReferenceError in g() is reported.
+ // - the stack trace is not overwritten when e1 is rethrown by t().
+ // - the stack trace of e2 does not overwrite that of e1.
+ const char* source =
+ "function g() { error; } \n"
+ "function f() { g(); } \n"
+ "function t(e) { throw e; } \n"
+ "try { \n"
+ " f(); \n"
+ "} catch (e1) { \n"
+ " try { \n"
+ " error; \n"
+ " } catch (e2) { \n"
+ " t(e1); \n"
+ " } \n"
+ "} \n";
+ v8::V8::AddMessageListener(RethrowStackTraceHandler);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(true);
+ CompileRun(source);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(false);
+ v8::V8::RemoveMessageListeners(RethrowStackTraceHandler);
+}
+
+
+static void RethrowPrimitiveStackTraceHandler(v8::Handle<v8::Message>
message,
+ v8::Handle<v8::Value> data) {
+ v8::Handle<v8::StackTrace> stack_trace = message->GetStackTrace();
+ CHECK(!stack_trace.IsEmpty());
+ int frame_count = stack_trace->GetFrameCount();
+ CHECK_EQ(2, frame_count);
+ int line_number[] = {3, 7};
+ for (int i = 0; i < frame_count; i++) {
+ CHECK_EQ(line_number[i], stack_trace->GetFrame(i)->GetLineNumber());
+ }
+}
+
+
+// Test that we do not recognize identity for primitive exceptions.
+TEST(RethrowPrimitiveStackTrace) {
+ v8::HandleScope scope;
+ LocalContext env;
+ // We do not capture stack trace for non Error objects on creation time.
+ // Instead, we capture the stack trace on last throw.
+ const char* source =
+ "function g() { throw 404; } \n"
+ "function f() { g(); } \n"
+ "function t(e) { throw e; } \n"
+ "try { \n"
+ " f(); \n"
+ "} catch (e1) { \n"
+ " t(e1) \n"
+ "} \n";
+ v8::V8::AddMessageListener(RethrowPrimitiveStackTraceHandler);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(true);
+ CompileRun(source);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(false);
+ v8::V8::RemoveMessageListeners(RethrowPrimitiveStackTraceHandler);
+}
+
+
+static void RethrowExistingStackTraceHandler(v8::Handle<v8::Message>
message,
+ v8::Handle<v8::Value> data) {
+ // Use the frame where JavaScript is called from.
+ v8::Handle<v8::StackTrace> stack_trace = message->GetStackTrace();
+ CHECK(!stack_trace.IsEmpty());
+ CHECK_EQ(1, stack_trace->GetFrameCount());
+ CHECK_EQ(1, stack_trace->GetFrame(0)->GetLineNumber());
+}
+
+
+// Test that the stack trace is captured when the error object is created
and
+// not where it is thrown.
+TEST(RethrowExistingStackTrace) {
+ v8::HandleScope scope;
+ LocalContext env;
+ const char* source =
+ "var e = new Error(); \n"
+ "throw e; \n";
+ v8::V8::AddMessageListener(RethrowExistingStackTraceHandler);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(true);
+ CompileRun(source);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(false);
+ v8::V8::RemoveMessageListeners(RethrowExistingStackTraceHandler);
+}
+
+
+static void RethrowBogusErrorStackTraceHandler(v8::Handle<v8::Message>
message,
+ v8::Handle<v8::Value> data)
{
+ // Use the frame where JavaScript is called from.
+ v8::Handle<v8::StackTrace> stack_trace = message->GetStackTrace();
+ CHECK(!stack_trace.IsEmpty());
+ CHECK_EQ(1, stack_trace->GetFrameCount());
+ CHECK_EQ(2, stack_trace->GetFrame(0)->GetLineNumber());
+}
+
+
+// Test that the stack trace is captured where the bogus Error object is
thrown.
+TEST(RethrowBogusErrorStackTrace) {
+ v8::HandleScope scope;
+ LocalContext env;
+ const char* source =
+ "var e = {__proto__: new Error()} \n"
+ "throw e; \n";
+ v8::V8::AddMessageListener(RethrowBogusErrorStackTraceHandler);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(true);
+ CompileRun(source);
+ v8::V8::SetCaptureStackTraceForUncaughtExceptions(false);
+ v8::V8::RemoveMessageListeners(RethrowBogusErrorStackTraceHandler);
+}
v8::Handle<Value> AnalyzeStackOfEvalWithSourceURL(const v8::Arguments&
args) {
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev