Fix ARM compiler crash in short-circuited boolean expressions.

We did not handle the case where the left-hand-side expression was
fully compiled to control flow.  There were also some assertions for
unary and binary expressions that crashed debug builds when the
expression was fully compiled to control flow.

Regression test added.

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

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

diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc
index 0a56be8..5f8149e 100644 (file)
--- a/src/arm/codegen-arm.cc
+++ b/src/arm/codegen-arm.cc
@@ -3737,7 +3737,8 @@ void CodeGenerator::VisitUnaryOperation(UnaryOperation* node) {
     }
     frame_->EmitPush(r0);  // r0 has result
   }
-  ASSERT((has_cc() && frame_->height() == original_height) ||
+  ASSERT(!has_valid_frame() ||
+         (has_cc() && frame_->height() == original_height) ||
          (!has_cc() && frame_->height() == original_height + 1));
 }
 
@@ -3871,22 +3872,12 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) {
                           &is_true,
                           false_target(),
                           false);
-    if (has_cc()) {
-      Branch(false, false_target());
-
-      // Evaluate right side expression.
-      is_true.Bind();
-      LoadConditionAndSpill(node->right(),
-                            NOT_INSIDE_TYPEOF,
-                            true_target(),
-                            false_target(),
-                            false);
-
-    } else {
+    if (has_valid_frame() && !has_cc()) {
+      // The left-hand side result is on top of the virtual frame.
       JumpTarget pop_and_continue;
       JumpTarget exit;
 
-      __ ldr(r0, frame_->Top());  // dup the stack top
+      __ ldr(r0, frame_->Top());  // Duplicate the stack top.
       frame_->EmitPush(r0);
       // Avoid popping the result if it converts to 'false' using the
       // standard ToBoolean() conversion as described in ECMA-262,
@@ -3904,6 +3895,22 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) {
 
       // Exit (always with a materialized value).
       exit.Bind();
+    } else if (has_cc() || is_true.is_linked()) {
+      // The left-hand side is either (a) partially compiled to
+      // control flow with a final branch left to emit or (b) fully
+      // compiled to control flow and possibly true.
+      if (has_cc()) {
+        Branch(false, false_target());
+      }
+      is_true.Bind();
+      LoadConditionAndSpill(node->right(),
+                            NOT_INSIDE_TYPEOF,
+                            true_target(),
+                            false_target(),
+                            false);
+    } else {
+      // Nothing to do.
+      ASSERT(!has_valid_frame() && !has_cc() && !is_true.is_linked());
     }
 
   } else if (op == Token::OR) {
@@ -3913,18 +3920,8 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) {
                           true_target(),
                           &is_false,
                           false);
-    if (has_cc()) {
-      Branch(true, true_target());
-
-      // Evaluate right side expression.
-      is_false.Bind();
-      LoadConditionAndSpill(node->right(),
-                            NOT_INSIDE_TYPEOF,
-                            true_target(),
-                            false_target(),
-                            false);
-
-    } else {
+    if (has_valid_frame() && !has_cc()) {
+      // The left-hand side result is on top of the virtual frame.
       JumpTarget pop_and_continue;
       JumpTarget exit;
 
@@ -3946,6 +3943,22 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) {
 
       // Exit (always with a materialized value).
       exit.Bind();
+    } else if (has_cc() || is_false.is_linked()) {
+      // The left-hand side is either (a) partially compiled to
+      // control flow with a final branch left to emit or (b) fully
+      // compiled to control flow and possibly false.
+      if (has_cc()) {
+        Branch(true, true_target());
+      }
+      is_false.Bind();
+      LoadConditionAndSpill(node->right(),
+                            NOT_INSIDE_TYPEOF,
+                            true_target(),
+                            false_target(),
+                            false);
+    } else {
+      // Nothing to do.
+      ASSERT(!has_valid_frame() && !has_cc() && !is_false.is_linked());
     }
 
   } else {
@@ -3989,7 +4002,8 @@ void CodeGenerator::VisitBinaryOperation(BinaryOperation* node) {
     }
     frame_->EmitPush(r0);
   }
-  ASSERT((has_cc() && frame_->height() == original_height) ||
+  ASSERT(!has_valid_frame() ||
+         (has_cc() && frame_->height() == original_height) ||
          (!has_cc() && frame_->height() == original_height + 1));
 }
 

diff --git a/test/mjsunit/regress/regress-406.js b/test/mjsunit/regress/regress-406.js
new file mode 100644 (file)
index 0000000..f48a5de
--- /dev/null
+++ b/test/mjsunit/regress/regress-406.js
@@ -0,0 +1,69 @@
+// Copyright 2009 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 of constant folding of boolean-valued expressions.
+
+// See http://code.google.com/p/v8/issues/detail?id=406
+
+assertFalse(typeof(0) == "zero");
+assertTrue(typeof(0) != "zero");
+
+// The and and or truth tables with both operands constant.
+assertFalse(typeof(0) == "zero" && typeof(0) == "zero");
+assertFalse(typeof(0) == "zero" && typeof(0) != "zero");
+assertFalse(typeof(0) != "zero" && typeof(0) == "zero");
+assertTrue(typeof(0) != "zero" && typeof(0) != "zero");
+
+assertFalse(typeof(0) == "zero" || typeof(0) == "zero");
+assertTrue(typeof(0) == "zero" || typeof(0) != "zero");
+assertTrue(typeof(0) != "zero" || typeof(0) == "zero");
+assertTrue(typeof(0) != "zero" || typeof(0) != "zero");
+
+// Same with just the left operand constant.
+// Helper function to prevent simple constant folding.
+function one() { return 1; }
+
+assertFalse(typeof(0) == "zero" && one() < 0);
+assertFalse(typeof(0) == "zero" && one() > 0);
+assertFalse(typeof(0) != "zero" && one() < 0);
+assertTrue(typeof(0) != "zero" && one() > 0);
+
+assertFalse(typeof(0) == "zero" || one() < 0);
+assertTrue(typeof(0) == "zero" || one() > 0);
+assertTrue(typeof(0) != "zero" || one() < 0);
+assertTrue(typeof(0) != "zero" || one() > 0);
+
+// Same with just the right operand constant.
+assertFalse(one() < 0 && typeof(0) == "zero");
+assertFalse(one() < 0 && typeof(0) != "zero");
+assertFalse(one() > 0 && typeof(0) == "zero");
+assertTrue(one() > 0 && typeof(0) != "zero");
+
+assertFalse(one() < 0 || typeof(0) == "zero");
+assertTrue(one() < 0 || typeof(0) != "zero");
+assertTrue(one() > 0 || typeof(0) == "zero");
+assertTrue(one() > 0 || typeof(0) != "zero");