Experimental: fix a bug in our handling of the short-circuit boolean...

src/codegen-ia32.cc

 // Copyright 2006-2008 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
@@ skipping 4603 matching lines @@
   // NOTE: If the left hand side produces a materialized value (not
   // control flow), we force the right hand side to do the same. This
   // is necessary because we assume that if we get control flow on the
   // last path out of an expression we got it on all paths.
   if (op == Token::AND) {
     JumpTarget is_true(this);
     ControlDestination dest(&is_true, destination()->false_target(), true);
     LoadCondition(node->left(), NOT_INSIDE_TYPEOF, &dest, false);
 
     if (dest.false_was_fall_through()) {
-      // We have just bound the false target.  There may be dangling
-      // jumps to is_true.
+      // The current false target was used as the fall-through.  If
+      // there are no dangling jumps to is_true then the left
+      // subexpression was unconditionally false.  Otherwise we have
+      // paths where we do have to evaluate the right subexpression.
       if (is_true.is_linked()) {
-        destination()->false_target()->Unuse();
-        destination()->false_target()->Jump();
+        // We need to compile the right subexpression.  If the jump to
+        // the current false target was a forward jump then we have a
+        // valid frame, we have just bound the false target, and we
+        // have to jump around the code for the right subexpression.
+        if (has_valid_frame()) {
+          destination()->false_target()->Unuse();
+          destination()->false_target()->Jump();
+        }
         is_true.Bind();
-        destination()->Goto(true);
+        // The left subexpression compiled to control flow, so the
+        // right one is free to do so as well.
+        LoadCondition(node->right(), NOT_INSIDE_TYPEOF, destination(), false);
+      } else {
+        // We have actually just jumped to or bound the current false
+        // target but the current control destination is not marked as
+        // used.
+        destination()->Use(false);
       }
+
     } else if (dest.is_used()) {
-      // The first subexpression compiled to control flow (is_true was
-      // just bound), so the second is free to do so as well.
+      // The left subexpression compiled to control flow (and is_true
+      // was just bound), so the right is free to do so as well.
       LoadCondition(node->right(), NOT_INSIDE_TYPEOF, destination(), false);
+
     } else {
       // We have a materialized value on the frame, so we exit with
       // one on all paths.  There are possibly also jumps to is_true
       // from nested subexpressions.
       JumpTarget pop_and_continue(this);
       JumpTarget exit(this);
 
       // Avoid popping the result if it converts to 'false' using the
       // standard ToBoolean() conversion as described in ECMA-262,
       // section 9.2, page 30.
       //
       // Duplicate the TOS value. The duplicate will be popped by
       // ToBoolean.
       frame_->Dup();
       ControlDestination dest(&pop_and_continue, &exit, true);
       ToBoolean(&dest);
 
       // Pop the result of evaluating the first part.
       frame_->Drop();
 
-      // Evaluate right side expression.
+      // Compile right side expression.
       is_true.Bind();
       Load(node->right());
 
       // Exit (always with a materialized value).
       exit.Bind();
     }
 
   } else if (op == Token::OR) {
     JumpTarget is_false(this);
     ControlDestination dest(destination()->true_target(), &is_false, false);
     LoadCondition(node->left(), NOT_INSIDE_TYPEOF, &dest, false);
 
     if (dest.true_was_fall_through()) {
-      // We have just bound the true target.  There may be dangling
-      // jumps to is_false.
+      // The current true target was used as the fall-through.  If
+      // there are no dangling jumps to is_false then the left
+      // subexpression was unconditionally true.  Otherwise we have
+      // paths where we do have to evaluate the right subexpression.
       if (is_false.is_linked()) {
-        destination()->true_target()->Unuse();
-        destination()->true_target()->Jump();
+        // We need to compile the right subexpression.  If the jump to
+        // the current true target was a forward jump then we have a
+        // valid frame, we have just bound the true target, and we
+        // have to jump around the code for the right subexpression.
+        if (has_valid_frame()) {
+          destination()->true_target()->Unuse();
+          destination()->true_target()->Jump();
+        }
         is_false.Bind();
-        destination()->Goto(false);
+        // The left subexpression compiled to control flow, so the
+        // right one is free to do so as well.
+        LoadCondition(node->right(), NOT_INSIDE_TYPEOF, destination(), false);
+      } else {
+        // We have just jumped to or bound the current true target but
+        // the current control destination is not marked as used.
+        destination()->Use(true);
       }
+
     } else if (dest.is_used()) {
-      // The first subexpression compiled to control flow (is_false
-      // was just bound), so the second is free to do so as well.
+      // The left subexpression compiled to control flow (and is_false
+      // was just bound), so the right is free to do so as well.
       LoadCondition(node->right(), NOT_INSIDE_TYPEOF, destination(), false);
+
     } else {
       // We have a materialized value on the frame, so we exit with
       // one on all paths.  There are possibly also jumps to is_false
       // from nested subexpressions.
       JumpTarget pop_and_continue(this);
       JumpTarget exit(this);
 
       // Avoid popping the result if it converts to 'true' using the
       // standard ToBoolean() conversion as described in ECMA-262,
       // section 9.2, page 30.
       //
       // Duplicate the TOS value. The duplicate will be popped by
       // ToBoolean.
       frame_->Dup();
       ControlDestination dest(&exit, &pop_and_continue, false);
       ToBoolean(&dest);
 
       // Pop the result of evaluating the first part.
       frame_->Drop();
 
-      // Evaluate right side expression.
+      // Compile right side expression.
       is_false.Bind();
       Load(node->right());
 
       // Exit (always with a materialized value).
       exit.Bind();
     }
 
   } else {
     // NOTE: The code below assumes that the slow cases (calls to runtime)
     // never return a constant/immutable object.
@@ skipping 2015 matching lines @@
 
   // Slow-case: Go through the JavaScript implementation.
   __ bind(&slow);
   __ InvokeBuiltin(Builtins::INSTANCE_OF, JUMP_FUNCTION);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal