Preserve error message during finally block in try..finally.

R=ulan@chromium.org
BUG=129171
TEST=test-api/TryFinallyMessage

Review URL: https://chromiumcodereview.appspot.com/10537078

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

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index 2a5887a..e2af0e5 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -4508,6 +4508,52 @@ void FullCodeGenerator::ExitFinallyBlock() {
 }
 
 
+void FullCodeGenerator::SavePendingMessage() {
+  ASSERT(!result_register().is(r1));
+  // Store pending message while executing finally block upon exception.
+  ExternalReference pending_message_obj =
+      ExternalReference::address_of_pending_message_obj(isolate());
+  __ mov(ip, Operand(pending_message_obj));
+  __ ldr(r1, MemOperand(ip));
+  __ push(r1);
+
+  ExternalReference has_pending_message =
+      ExternalReference::address_of_has_pending_message(isolate());
+  __ mov(ip, Operand(has_pending_message));
+  __ ldr(r1, MemOperand(ip));
+  __ push(r1);
+
+  ExternalReference pending_message_script =
+      ExternalReference::address_of_pending_message_script(isolate());
+  __ mov(ip, Operand(pending_message_script));
+  __ ldr(r1, MemOperand(ip));
+  __ push(r1);
+}
+
+
+void FullCodeGenerator::RestorePendingMessage() {
+  ASSERT(!result_register().is(r1));
+  // Restore pending message.
+  __ pop(r1);
+  ExternalReference pending_message_script =
+      ExternalReference::address_of_pending_message_script(isolate());
+  __ mov(ip, Operand(pending_message_script));
+  __ str(r1, MemOperand(ip));
+
+  __ pop(r1);
+  ExternalReference has_pending_message =
+      ExternalReference::address_of_has_pending_message(isolate());
+  __ mov(ip, Operand(has_pending_message));
+  __ str(r1, MemOperand(ip));
+
+  __ pop(r1);
+  ExternalReference pending_message_obj =
+      ExternalReference::address_of_pending_message_obj(isolate());
+  __ mov(ip, Operand(pending_message_obj));
+  __ str(r1, MemOperand(ip));
+}
+
+
 #undef __
 
 #define __ ACCESS_MASM(masm())

diff --git a/src/assembler.cc b/src/assembler.cc
index 7871f4e..d4c49dd 100644 (file)
--- a/src/assembler.cc
+++ b/src/assembler.cc
@@ -955,6 +955,24 @@ ExternalReference ExternalReference::scheduled_exception_address(
 }
 
 
+ExternalReference ExternalReference::address_of_pending_message_obj(
+    Isolate* isolate) {
+  return ExternalReference(isolate->pending_message_obj_address());
+}
+
+
+ExternalReference ExternalReference::address_of_has_pending_message(
+    Isolate* isolate) {
+  return ExternalReference(isolate->has_pending_message_address());
+}
+
+
+ExternalReference ExternalReference::address_of_pending_message_script(
+    Isolate* isolate) {
+  return ExternalReference(isolate->pending_message_script_address());
+}
+
+
 ExternalReference ExternalReference::address_of_min_int() {
   return ExternalReference(reinterpret_cast<void*>(&double_constants.min_int));
 }

diff --git a/src/assembler.h b/src/assembler.h
index b935ab5..619c69c 100644 (file)
--- a/src/assembler.h
+++ b/src/assembler.h
@@ -640,6 +640,9 @@ class ExternalReference BASE_EMBEDDED {
   static ExternalReference handle_scope_level_address();
 
   static ExternalReference scheduled_exception_address(Isolate* isolate);
+  static ExternalReference address_of_pending_message_obj(Isolate* isolate);
+  static ExternalReference address_of_has_pending_message(Isolate* isolate);
+  static ExternalReference address_of_pending_message_script(Isolate* isolate);
 
   // Static variables containing common double constants.
   static ExternalReference address_of_min_int();

diff --git a/src/full-codegen.cc b/src/full-codegen.cc
index 9b1df4e..ebe877f 100644 (file)
--- a/src/full-codegen.cc
+++ b/src/full-codegen.cc
@@ -1287,7 +1287,11 @@ void FullCodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) {
   // is thrown.  The exception is in the result register, and must be
   // preserved by the finally block.  Call the finally block and then
   // rethrow the exception if it returns.
+  // Also preserve the pending message corresponding to the thrown error when
+  // executing the finally block.
+  SavePendingMessage();
   __ Call(&finally_entry);
+  RestorePendingMessage();
   __ push(result_register());
   __ CallRuntime(Runtime::kReThrow, 1);
 

diff --git a/src/full-codegen.h b/src/full-codegen.h
index 2a6b705..c1b4f00 100644 (file)
--- a/src/full-codegen.h
+++ b/src/full-codegen.h
@@ -527,6 +527,8 @@ class FullCodeGenerator: public AstVisitor {
   // Non-local control flow support.
   void EnterFinallyBlock();
   void ExitFinallyBlock();
+  void SavePendingMessage();
+  void RestorePendingMessage();
 
   // Loop nesting counter.
   int loop_depth() { return loop_depth_; }

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index 9727ea0..89b980d 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -4492,6 +4492,46 @@ void FullCodeGenerator::ExitFinallyBlock() {
 }
 
 
+void FullCodeGenerator::SavePendingMessage() {
+  ASSERT(!result_register().is(edx));
+  // Store pending message while executing finally block upon exception.
+  ExternalReference pending_message_obj =
+      ExternalReference::address_of_pending_message_obj(isolate());
+  __ mov(edx, Operand::StaticVariable(pending_message_obj));
+  __ push(edx);
+
+  ExternalReference has_pending_message =
+      ExternalReference::address_of_has_pending_message(isolate());
+  __ mov(edx, Operand::StaticVariable(has_pending_message));
+  __ push(edx);
+
+  ExternalReference pending_message_script =
+      ExternalReference::address_of_pending_message_script(isolate());
+  __ mov(edx, Operand::StaticVariable(pending_message_script));
+  __ push(edx);
+}
+
+
+void FullCodeGenerator::RestorePendingMessage() {
+  ASSERT(!result_register().is(edx));
+  // Restore pending message.
+  __ pop(edx);
+  ExternalReference pending_message_script =
+      ExternalReference::address_of_pending_message_script(isolate());
+  __ mov(Operand::StaticVariable(pending_message_script), edx);
+
+  __ pop(edx);
+  ExternalReference has_pending_message =
+      ExternalReference::address_of_has_pending_message(isolate());
+  __ mov(Operand::StaticVariable(has_pending_message), edx);
+
+  __ pop(edx);
+  ExternalReference pending_message_obj =
+      ExternalReference::address_of_pending_message_obj(isolate());
+  __ mov(Operand::StaticVariable(pending_message_obj), edx);
+}
+
+
 #undef __
 
 #define __ ACCESS_MASM(masm())

diff --git a/src/isolate.cc b/src/isolate.cc
index 6fcc926..8a186c9 100644 (file)
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -921,7 +921,7 @@ Failure* Isolate::Throw(Object* exception, MessageLocation* location) {
 }
 
 
-Failure* Isolate::ReThrow(MaybeObject* exception, MessageLocation* location) {
+Failure* Isolate::ReThrow(MaybeObject* exception) {
   bool can_be_caught_externally = false;
   bool catchable_by_javascript = is_catchable_by_javascript(exception);
   ShouldReportException(&can_be_caught_externally, catchable_by_javascript);

diff --git a/src/isolate.h b/src/isolate.h
index 9aa1242..c37cdc9 100644 (file)
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -578,6 +578,20 @@ class Isolate {
   MaybeObject** scheduled_exception_address() {
     return &thread_local_top_.scheduled_exception_;
   }
+
+  Address pending_message_obj_address() {
+    return reinterpret_cast<Address>(&thread_local_top_.pending_message_obj_);
+  }
+
+  Address has_pending_message_address() {
+    return reinterpret_cast<Address>(&thread_local_top_.has_pending_message_);
+  }
+
+  Address pending_message_script_address() {
+    return reinterpret_cast<Address>(
+        &thread_local_top_.pending_message_script_);
+  }
+
   MaybeObject* scheduled_exception() {
     ASSERT(has_scheduled_exception());
     return thread_local_top_.scheduled_exception_;
@@ -708,7 +722,7 @@ class Isolate {
   // Re-throw an exception.  This involves no error reporting since
   // error reporting was handled when the exception was thrown
   // originally.
-  Failure* ReThrow(MaybeObject* exception, MessageLocation* location = NULL);
+  Failure* ReThrow(MaybeObject* exception);
   void ScheduleThrow(Object* exception);
   void ReportPendingMessages();
   Failure* ThrowIllegalOperation();

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index 0db7424..a884484 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -4477,6 +4477,46 @@ void FullCodeGenerator::ExitFinallyBlock() {
 }
 
 
+void FullCodeGenerator::SavePendingMessage() {
+  ASSERT(!result_register().is(rdx));
+  // Store pending message while executing finally block upon exception.
+  ExternalReference pending_message_obj =
+      ExternalReference::address_of_pending_message_obj(isolate());
+  __ Load(rdx, pending_message_obj);
+  __ push(rdx);
+
+  ExternalReference has_pending_message =
+      ExternalReference::address_of_has_pending_message(isolate());
+  __ Load(rdx, has_pending_message);
+  __ push(rdx);
+
+  ExternalReference pending_message_script =
+      ExternalReference::address_of_pending_message_script(isolate());
+  __ Load(rdx, pending_message_script);
+  __ push(rdx);
+}
+
+
+void FullCodeGenerator::RestorePendingMessage() {
+  ASSERT(!result_register().is(rdx));
+  // Restore pending message.
+  __ pop(rdx);
+  ExternalReference pending_message_script =
+      ExternalReference::address_of_pending_message_script(isolate());
+  __ Store(pending_message_script, rdx);
+
+  __ pop(rdx);
+  ExternalReference has_pending_message =
+      ExternalReference::address_of_has_pending_message(isolate());
+  __ Store(has_pending_message, rdx);
+
+  __ pop(rdx);
+  ExternalReference pending_message_obj =
+      ExternalReference::address_of_pending_message_obj(isolate());
+  __ Store(pending_message_obj, rdx);
+}
+
+
 #undef __
 
 #define __ ACCESS_MASM(masm())

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index c5de671..19b0c47 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -16768,3 +16768,46 @@ THREADED_TEST(InstanceCheckOnPrototypeAccessor) {
   CHECK(templ->HasInstance(context->Global()->Get(v8_str("obj"))));
   CheckInstanceCheckedAccessors(true);
 }
+
+
+TEST(TryFinallyMessage) {
+  v8::HandleScope scope;
+  LocalContext context;
+  {
+    // Test that the original error message is not lost if there is a
+    // recursive call into Javascript is done in the finally block, e.g. to
+    // initialize an IC. (crbug.com/129171)
+    TryCatch try_catch;
+    const char* trigger_ic =
+        "try {                      \n"
+        "  throw new Error('test'); \n"
+        "} finally {                \n"
+        "  var x = 0;               \n"
+        "  x++;                     \n"  // Trigger an IC initialization here.
+        "}                          \n";
+    Local<Value> result = CompileRun(trigger_ic);
+    CHECK(try_catch.HasCaught());
+    Local<Message> message = try_catch.Message();
+    CHECK(!message.IsEmpty());
+    CHECK_EQ(2, message->GetLineNumber());
+  }
+
+  {
+    // Test that the original exception message is indeed overwritten if
+    // a new error is thrown in the finally block.
+    TryCatch try_catch;
+    const char* throw_again =
+        "try {                       \n"
+        "  throw new Error('test');  \n"
+        "} finally {                 \n"
+        "  var x = 0;                \n"
+        "  x++;                      \n"
+        "  throw new Error('again'); \n"  // This is the new uncaught error.
+        "}                           \n";
+    Local<Value> result = CompileRun(throw_again);
+    CHECK(try_catch.HasCaught());
+    Local<Message> message = try_catch.Message();
+    CHECK(!message.IsEmpty());
+    CHECK_EQ(6, message->GetLineNumber());
+  }
+}