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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10618 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/heap.h

@@ -241,9 +241,10 @@ namespace internal {
   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;

src/isolate.cc

@@ -542,6 +542,18 @@ Handle<String> Isolate::StackTraceString() {
 }
 
 
+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(
     int frame_limit, StackTrace::StackTraceOptions options) {
   // Ensure no negative values.
@@ -1037,22 +1049,39 @@ bool Isolate::ShouldReportException(bool* can_be_caught_externally,
 }
 
 
-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 @@ void Isolate::DoThrow(MaybeObject* exception, MessageLocation* location) {
   }
 #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 @@ void Isolate::DoThrow(MaybeObject* exception, MessageLocation* location) {
 
   // 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);
 }
 
 

src/isolate.h

@@ -703,6 +703,8 @@ class Isolate {
       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 @@ class Isolate {
 
   // 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 @@ class Isolate {
 
   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.

src/messages.js

@@ -1078,9 +1078,9 @@ function captureStackTrace(obj, cons_opt) {
   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);
   });

src/objects.h

@@ -1638,7 +1638,7 @@ class JSObject: public JSReceiver {
                                           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.

src/runtime.cc

@@ -13263,9 +13263,10 @@ static bool ShowFrameInStackTrace(StackFrame* raw_frame,
 // 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 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CollectStackTrace) {
     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;
 }

src/runtime.h

@@ -229,7 +229,7 @@ namespace internal {
   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) \

test/cctest/test-api.cc

@@ -13535,6 +13535,137 @@ TEST(CaptureStackTraceForUncaughtExceptionAndSetters) {
 }
 
 
+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::HandleScope scope;
   v8::Handle<v8::StackTrace> stackTrace =