From 016fcd4affaca553ac3378edf4b7868bd79c9867 Mon Sep 17 00:00:00 2001
From: "kasperl@chromium.org"
 <kasperl@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Wed, 1 Oct 2008 07:43:00 +0000
Subject: [PATCH] Fix issue 86 by keeping track of the fact that finally blocks
 are evaluated with an extra element on the stack, which needs to be taken
 into account when breaking and continuing.

I'll clean up the code and add an abstraction for manipulating
the break stack height in a future CL -- I want to try to get
rid of the separate local variable we keep around for the "state"
when running in a finally block.
Review URL: http://codereview.chromium.org/5625

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@400 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/codegen-arm.cc                                     | 13 ++++++++++++-
 src/codegen-ia32.cc                                    | 11 ++++++++++-
 test/mjsunit/{bugs/bug-86.js => regress/regress-86.js} |  6 ++++--
 3 files changed, 26 insertions(+), 4 deletions(-)
 rename test/mjsunit/{bugs/bug-86.js => regress/regress-86.js} (94%)

diff --git a/src/codegen-arm.cc b/src/codegen-arm.cc
index dfe5745..5192bb2 100644
--- a/src/codegen-arm.cc
+++ b/src/codegen-arm.cc
@@ -3182,6 +3182,12 @@ void ArmCodeGenerator::VisitTryFinally(TryFinally* node) {
   // --- Finally block ---
   __ bind(&finally_block);
 
+  // We keep a single element on the stack - the (possibly faked)
+  // result - while evaluating the finally block. Record it, so that a
+  // break/continue crossing this statement can restore the stack.
+  const int kFinallyStackSize = 1 * kPointerSize;
+  break_stack_height_ += kFinallyStackSize;
+
   // Push the state on the stack. If necessary move the state to a
   // local variable to avoid having extra values on the stack while
   // evaluating the finally block.
@@ -3203,7 +3209,12 @@ void ArmCodeGenerator::VisitTryFinally(TryFinally* node) {
   }
   __ pop(r2);
 
-  __ pop(r0);  // Restore value or faked TOS.
+  // Restore return value or faked TOS.
+  __ pop(r0);
+
+  // Record the fact that the result has been removed from the stack.
+  break_stack_height_ -= kFinallyStackSize;
+
   // Generate code that jumps to the right destination for all used
   // shadow labels.
   for (int i = 0; i <= nof_escapes; i++) {
diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc
index 152d029..f3ae4cf 100644
--- a/src/codegen-ia32.cc
+++ b/src/codegen-ia32.cc
@@ -3492,7 +3492,7 @@ void Ia32CodeGenerator::VisitTryFinally(TryFinally* node) {
 
   __ PushTryHandler(IN_JAVASCRIPT, TRY_FINALLY_HANDLER);
   // TODO(1222589): remove the reliance of PushTryHandler on a cached TOS
-  __ push(eax);  //
+  __ push(eax);
 
   // Introduce shadow labels for all escapes from the try block,
   // including returns. We should probably try to unify the escaping
@@ -3558,6 +3558,12 @@ void Ia32CodeGenerator::VisitTryFinally(TryFinally* node) {
   // --- Finally block ---
   __ bind(&finally_block);
 
+  // We keep a single element on the stack - the (possibly faked)
+  // result - while evaluating the finally block. Record it, so that a
+  // break/continue crossing this statement can restore the stack.
+  const int kFinallyStackSize = 1 * kPointerSize;
+  break_stack_height_ += kFinallyStackSize;
+
   // Push the state on the stack. If necessary move the state to a
   // local variable to avoid having extra values on the stack while
   // evaluating the finally block.
@@ -3583,6 +3589,9 @@ void Ia32CodeGenerator::VisitTryFinally(TryFinally* node) {
   // Restore return value or faked TOS.
   __ pop(eax);
 
+  // Record the fact that the result has been removed from the stack.
+  break_stack_height_ -= kFinallyStackSize;
+
   // Generate code that jumps to the right destination for all used
   // shadow labels.
   for (int i = 0; i <= nof_escapes; i++) {
diff --git a/test/mjsunit/bugs/bug-86.js b/test/mjsunit/regress/regress-86.js
similarity index 94%
rename from test/mjsunit/bugs/bug-86.js
rename to test/mjsunit/regress/regress-86.js
index 9edf85f..a33b60b 100644
--- a/test/mjsunit/bugs/bug-86.js
+++ b/test/mjsunit/regress/regress-86.js
@@ -28,8 +28,9 @@
 var aList = [1, 2, 3];
 var loopCount = 0;
 var leftThroughFinally = false;
+var enteredFinally = false;
 for (x in aList) {
-  leftThroughFinally = false;
+  leftThroughFinally = true;
   try {
     throw "ex1";
   } catch(er1) {
@@ -40,5 +41,6 @@ for (x in aList) {
   }
   leftThroughFinally = false;
 }
-assertEquals(loopCount, 3);
+assertEquals(3, loopCount);
 assertTrue(enteredFinally);
+assertTrue(leftThroughFinally);
-- 
2.7.4