Fix a bug in abrupt exit from with or catch inside finally.
authorkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 7 Sep 2011 09:21:44 +0000 (09:21 +0000)
committerkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 7 Sep 2011 09:21:44 +0000 (09:21 +0000)
When with or catch is nested inside finally, we were not properly restoring
the context in the stack for the finally code.  Also, as a small
optimization, restore it from the handler block instead of iteratively
unwinding contexts.

R=fschneider@chromium.org
BUG=
TEST=

Review URL: http://codereview.chromium.org/7837023

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

src/arm/full-codegen-arm.cc
src/full-codegen.cc
src/ia32/full-codegen-ia32.cc
src/mips/full-codegen-mips.cc
src/x64/full-codegen-x64.cc
test/mjsunit/regress/regress-95485.js [new file with mode: 0644]

index f6d9906..2b9d846 100644 (file)
@@ -4292,6 +4292,34 @@ void FullCodeGenerator::ExitFinallyBlock() {
 
 #undef __
 
+#define __ ACCESS_MASM(masm())
+
+FullCodeGenerator::NestedStatement* FullCodeGenerator::TryFinally::Exit(
+    int* stack_depth,
+    int* context_length) {
+  // The macros used here must preserve the result register.
+
+  // Because the handler block contains the context of the finally
+  // code, we can restore it directly from there for the finally code
+  // rather than iteratively unwinding contexts via their previous
+  // links.
+  __ Drop(*stack_depth);  // Down to the handler block.
+  if (*context_length > 0) {
+    // Restore the context to its dedicated register and the stack.
+    __ ldr(cp, MemOperand(sp, StackHandlerConstants::kContextOffset));
+    __ str(cp, MemOperand(fp, StandardFrameConstants::kContextOffset));
+  }
+  __ PopTryHandler();
+  __ bl(finally_entry_);
+
+  *stack_depth = 0;
+  *context_length = 0;
+  return previous_;
+}
+
+
+#undef __
+
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM
index 4f1abbe..d187129 100644 (file)
@@ -1354,25 +1354,6 @@ void FullCodeGenerator::VisitThrow(Throw* expr) {
 }
 
 
-FullCodeGenerator::NestedStatement* FullCodeGenerator::TryFinally::Exit(
-    int* stack_depth,
-    int* context_length) {
-  // The macros used here must preserve the result register.
-  __ Drop(*stack_depth);
-  __ PopTryHandler();
-  *stack_depth = 0;
-
-  Register context = FullCodeGenerator::context_register();
-  while (*context_length > 0) {
-    codegen_->LoadContextField(context, Context::PREVIOUS_INDEX);
-    --(*context_length);
-  }
-
-  __ Call(finally_entry_);
-  return previous_;
-}
-
-
 FullCodeGenerator::NestedStatement* FullCodeGenerator::TryCatch::Exit(
     int* stack_depth,
     int* context_length) {
index ca4e824..f4dca00 100644 (file)
@@ -4348,6 +4348,34 @@ void FullCodeGenerator::ExitFinallyBlock() {
 
 #undef __
 
+#define __ ACCESS_MASM(masm())
+
+FullCodeGenerator::NestedStatement* FullCodeGenerator::TryFinally::Exit(
+    int* stack_depth,
+    int* context_length) {
+  // The macros used here must preserve the result register.
+
+  // Because the handler block contains the context of the finally
+  // code, we can restore it directly from there for the finally code
+  // rather than iteratively unwinding contexts via their previous
+  // links.
+  __ Drop(*stack_depth);  // Down to the handler block.
+  if (*context_length > 0) {
+    // Restore the context to its dedicated register and the stack.
+    __ mov(esi, Operand(esp, StackHandlerConstants::kContextOffset));
+    __ mov(Operand(ebp, StandardFrameConstants::kContextOffset), esi);
+  }
+  __ PopTryHandler();
+  __ call(finally_entry_);
+
+  *stack_depth = 0;
+  *context_length = 0;
+  return previous_;
+}
+
+
+#undef __
+
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32
index ec2a248..4525247 100644 (file)
@@ -4293,6 +4293,34 @@ void FullCodeGenerator::ExitFinallyBlock() {
 
 #undef __
 
+#define __ ACCESS_MASM(masm())
+
+FullCodeGenerator::NestedStatement* FullCodeGenerator::TryFinally::Exit(
+    int* stack_depth,
+    int* context_length) {
+  // The macros used here must preserve the result register.
+
+  // Because the handler block contains the context of the finally
+  // code, we can restore it directly from there for the finally code
+  // rather than iteratively unwinding contexts via their previous
+  // links.
+  __ Drop(*stack_depth);  // Down to the handler block.
+  if (*context_length > 0) {
+    // Restore the context to its dedicated register and the stack.
+    __ lw(cp, MemOperand(sp, StackHandlerConstants::kContextOffset));
+    __ sw(cp, MemOperand(fp, StandardFrameConstants::kContextOffset));
+  }
+  __ PopTryHandler();
+  __ Call(finally_entry_);
+
+  *stack_depth = 0;
+  *context_length = 0;
+  return previous_;
+}
+
+
+#undef __
+
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_MIPS
index 3f51057..d543de7 100644 (file)
@@ -4228,6 +4228,33 @@ void FullCodeGenerator::ExitFinallyBlock() {
 
 #undef __
 
+#define __ ACCESS_MASM(masm())
+
+FullCodeGenerator::NestedStatement* FullCodeGenerator::TryFinally::Exit(
+    int* stack_depth,
+    int* context_length) {
+  // The macros used here must preserve the result register.
+
+  // Because the handler block contains the context of the finally
+  // code, we can restore it directly from there for the finally code
+  // rather than iteratively unwinding contexts via their previous
+  // links.
+  __ Drop(*stack_depth);  // Down to the handler block.
+  if (*context_length > 0) {
+    // Restore the context to its dedicated register and the stack.
+    __ movq(rsi, Operand(rsp, StackHandlerConstants::kContextOffset));
+    __ movq(Operand(rbp, StandardFrameConstants::kContextOffset), rsi);
+  }
+  __ PopTryHandler();
+  __ call(finally_entry_);
+
+  *stack_depth = 0;
+  *context_length = 0;
+  return previous_;
+}
+
+
+#undef __
 
 } }  // namespace v8::internal
 
diff --git a/test/mjsunit/regress/regress-95485.js b/test/mjsunit/regress/regress-95485.js
new file mode 100644 (file)
index 0000000..2510072
--- /dev/null
@@ -0,0 +1,42 @@
+// Copyright 2011 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.
+
+function Test() {
+  var left  = 'XXX';
+  var right = 'YYY';
+  for (var i = 0; i < 3; i++) {
+    var cons = left + right;
+    var substring = cons.substring(2, 4);
+    try {
+      with ({Test: i})
+          continue;
+    } finally { }
+  }
+  return substring;
+}
+
+assertEquals('XY', Test());