From 09179b314aeed73343ef1c99d49629a361b14dd8 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 11 Jun 2012 13:18:05 +0000 Subject: [PATCH] Reland r11753: Fix try..finally. R=ulan@chromium.org BUG=129171 TEST=test-api/TryFinallyMessage, mjsunit/try-finally-continue.js Review URL: https://chromiumcodereview.appspot.com/10540095 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11762 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 41 ++++++++++++++++++++ src/assembler.cc | 18 +++++++++ src/assembler.h | 3 ++ src/full-codegen.h | 2 +- src/ia32/full-codegen-ia32.cc | 35 ++++++++++++++++++ src/isolate.cc | 2 +- src/isolate.h | 16 +++++++- src/x64/full-codegen-x64.cc | 35 ++++++++++++++++++ test/cctest/test-api.cc | 43 +++++++++++++++++++++ test/mjsunit/try-finally-continue.js | 72 ++++++++++++++++++++++++++++++++++++ 10 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 test/mjsunit/try-finally-continue.js diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 99c3c22..6ee5d15 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -4493,14 +4493,55 @@ void FullCodeGenerator::EnterFinallyBlock() { ASSERT_EQ(1, kSmiTagSize + kSmiShiftSize); STATIC_ASSERT(kSmiTag == 0); __ add(r1, r1, Operand(r1)); // Convert to smi. + + // Store result register while executing finally block. + __ push(r1); + + // Store pending message while executing finally block. + 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::ExitFinallyBlock() { ASSERT(!result_register().is(r1)); + // Restore pending message from stack. + __ 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)); + // Restore result register from stack. __ pop(r1); + // Uncook return address and return. __ pop(result_register()); ASSERT_EQ(1, kSmiTagSize + kSmiShiftSize); diff --git a/src/assembler.cc b/src/assembler.cc index d4a2c84..827a4ab 100644 --- a/src/assembler.cc +++ b/src/assembler.cc @@ -956,6 +956,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(&double_constants.min_int)); } diff --git a/src/assembler.h b/src/assembler.h index b9763e3..c0fb14b 100644 --- a/src/assembler.h +++ b/src/assembler.h @@ -643,6 +643,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.h b/src/full-codegen.h index e55d7cc..928de47 100644 --- a/src/full-codegen.h +++ b/src/full-codegen.h @@ -240,7 +240,7 @@ class FullCodeGenerator: public AstVisitor { // The finally block of a try/finally statement. class Finally : public NestedStatement { public: - static const int kElementCount = 2; + static const int kElementCount = 5; explicit Finally(FullCodeGenerator* codegen) : NestedStatement(codegen) { } virtual ~Finally() {} diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 2987777..5a513fd 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -4476,14 +4476,49 @@ void FullCodeGenerator::EnterFinallyBlock() { STATIC_ASSERT(kSmiTag == 0); __ SmiTag(edx); __ push(edx); + // Store result register while executing finally block. __ push(result_register()); + + // Store pending message while executing finally block. + 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::ExitFinallyBlock() { ASSERT(!result_register().is(edx)); + // Restore pending message from stack. + __ 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); + + // Restore result register from stack. __ pop(result_register()); + // Uncook return address. __ pop(edx); __ SmiUntag(edx); diff --git a/src/isolate.cc b/src/isolate.cc index 7df14c4..8fcb370 100644 --- 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 83cdb0d..5ca2b87 100644 --- 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
(&thread_local_top_.pending_message_obj_); + } + + Address has_pending_message_address() { + return reinterpret_cast
(&thread_local_top_.has_pending_message_); + } + + Address pending_message_script_address() { + return reinterpret_cast
( + &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 e35fb79..a3e42eb 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -4460,15 +4460,50 @@ void FullCodeGenerator::EnterFinallyBlock() { __ subq(rdx, rcx); __ Integer32ToSmi(rdx, rdx); __ push(rdx); + // Store result register while executing finally block. __ push(result_register()); + + // Store pending message while executing finally block. + 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::ExitFinallyBlock() { ASSERT(!result_register().is(rdx)); ASSERT(!result_register().is(rcx)); + // Restore pending message from stack. + __ 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); + + // Restore result register from stack. __ pop(result_register()); + // Uncook return address. __ pop(rdx); __ SmiToInteger32(rdx, rdx); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index c5de671..19b0c47 100644 --- 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 result = CompileRun(trigger_ic); + CHECK(try_catch.HasCaught()); + Local 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 result = CompileRun(throw_again); + CHECK(try_catch.HasCaught()); + Local message = try_catch.Message(); + CHECK(!message.IsEmpty()); + CHECK_EQ(6, message->GetLineNumber()); + } +} diff --git a/test/mjsunit/try-finally-continue.js b/test/mjsunit/try-finally-continue.js new file mode 100644 index 0000000..b55e7ac --- /dev/null +++ b/test/mjsunit/try-finally-continue.js @@ -0,0 +1,72 @@ +// 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. + +// Test that we correctly restore the stack when continuing from a +// finally block inside a for-in. + +var f = 0; +var a = [1, 2, 3]; + +for (x in a) { + try{ + throw 'error'; + } finally { + f++; + continue; + } +} +assertEquals(3, f); + +f = 0; +for (x in a) { + try { + f++; + } finally { + f++; + continue; + } +} +assertEquals(6, f); + +f = 0; +for (x in a) { + try { + f++; + } finally { + try { + throw 'error' + } finally { + try { + f++; + } finally { + f++; + continue; + } + } + } +} +assertEquals(9, f); \ No newline at end of file -- 2.7.4