[turbofan] Fix missing callability check on Array callbacks

This fixes the second-order Array.prototype function {forEach} and {map}
to now perform a callability check of the given callback function. For
empty arrays it is observable whether such a check outside the loop has
been elided or not.

R=mvstanton@chromium.org
TEST=mjsunit/regress/regress-crbug-747062
BUG=chromium:747062

Change-Id: I1bbe7f44b3b3d18e9b41ad0436975434adf84321
Reviewed-on: https://chromium-review.googlesource.com/588893Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46942}

src/compiler/effect-control-linearizer.cc

@@ -709,6 +709,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
     case IrOpcode::kCheckedTruncateTaggedToWord32:
       result = LowerCheckedTruncateTaggedToWord32(node, frame_state);
       break;
+    case IrOpcode::kObjectIsCallable:
+      result = LowerObjectIsCallable(node);
+      break;
     case IrOpcode::kObjectIsDetectableCallable:
       result = LowerObjectIsDetectableCallable(node);
       break;
@@ -1839,6 +1842,30 @@ Node* EffectControlLinearizer::LowerCheckedTruncateTaggedToWord32(
   return done.PhiAt(0);
 }
 
+Node* EffectControlLinearizer::LowerObjectIsCallable(Node* node) {
+  Node* value = node->InputAt(0);
+
+  auto if_smi = __ MakeDeferredLabel<1>();
+  auto done = __ MakeLabel<2>(MachineRepresentation::kBit);
+
+  Node* check = ObjectIsSmi(value);
+  __ GotoIf(check, &if_smi);
+
+  Node* value_map = __ LoadField(AccessBuilder::ForMap(), value);
+  Node* value_bit_field =
+      __ LoadField(AccessBuilder::ForMapBitField(), value_map);
+  Node* vfalse = __ Word32Equal(
+      __ Int32Constant(1 << Map::kIsCallable),
+      __ Word32And(value_bit_field, __ Int32Constant(1 << Map::kIsCallable)));
+  __ Goto(&done, vfalse);
+
+  __ Bind(&if_smi);
+  __ Goto(&done, __ Int32Constant(0));
+
+  __ Bind(&done);
+  return done.PhiAt(0);
+}
+
 Node* EffectControlLinearizer::LowerObjectIsDetectableCallable(Node* node) {
   Node* value = node->InputAt(0);
 

src/compiler/effect-control-linearizer.h

@@ -83,6 +83,7 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
   Node* LowerTruncateTaggedToFloat64(Node* node);
   Node* LowerTruncateTaggedToWord32(Node* node);
   Node* LowerCheckedTruncateTaggedToWord32(Node* node, Node* frame_state);
+  Node* LowerObjectIsCallable(Node* node);
   Node* LowerObjectIsDetectableCallable(Node* node);
   Node* LowerObjectIsNaN(Node* node);
   Node* LowerObjectIsNonCallable(Node* node);

src/compiler/escape-analysis.cc

@@ -837,6 +837,7 @@ bool EscapeStatusAnalysis::CheckUsesForEscape(Node* uses, Node* rep,
       case IrOpcode::kStringIndexOf:
       case IrOpcode::kStringToLowerCaseIntl:
       case IrOpcode::kStringToUpperCaseIntl:
+      case IrOpcode::kObjectIsCallable:
       case IrOpcode::kObjectIsDetectableCallable:
       case IrOpcode::kObjectIsNaN:
       case IrOpcode::kObjectIsNonCallable:

src/compiler/js-call-reducer.cc

@@ -534,11 +534,32 @@ Reduction JSCallReducer::ReduceArrayForEach(Handle<JSFunction> function,
       simplified()->LoadField(AccessBuilder::ForJSArrayLength(PACKED_ELEMENTS)),
       receiver, effect, control);
 
+  std::vector<Node*> checkpoint_params(
+      {receiver, fncallback, this_arg, k, original_length});
+  const int stack_parameters = static_cast<int>(checkpoint_params.size());
+
+  // Check whether the given callback function is callable. Note that this has
+  // to happen outside the loop to make sure we also throw on empty arrays.
+  Node* check = graph()->NewNode(simplified()->ObjectIsCallable(), fncallback);
+  Node* check_branch =
+      graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
+  Node* check_fail = graph()->NewNode(common()->IfFalse(), check_branch);
+  Node* check_frame_state = CreateJavaScriptBuiltinContinuationFrameState(
+      jsgraph(), function, Builtins::kArrayForEachLoopLazyDeoptContinuation,
+      node->InputAt(0), context, &checkpoint_params[0], stack_parameters,
+      outer_frame_state, ContinuationFrameStateMode::LAZY);
+  Node* check_throw = check_fail = graph()->NewNode(
+      javascript()->CallRuntime(Runtime::kThrowCalledNonCallable), fncallback,
+      context, check_frame_state, effect, check_fail);
+  control = graph()->NewNode(common()->IfTrue(), check_branch);
+
+  // Start the loop.
   Node* loop = control = graph()->NewNode(common()->Loop(2), control, control);
   Node* eloop = effect =
       graph()->NewNode(common()->EffectPhi(2), effect, effect, loop);
   Node* vloop = k = graph()->NewNode(
       common()->Phi(MachineRepresentation::kTagged, 2), k, k, loop);
+  checkpoint_params[3] = k;
 
   control = loop;
   effect = eloop;
@@ -552,10 +573,6 @@ Reduction JSCallReducer::ReduceArrayForEach(Handle<JSFunction> function,
   Node* if_false = graph()->NewNode(common()->IfFalse(), continue_branch);
   control = if_true;
 
-  std::vector<Node*> checkpoint_params(
-      {receiver, fncallback, this_arg, k, original_length});
-  const int stack_parameters = static_cast<int>(checkpoint_params.size());
-
   Node* frame_state = CreateJavaScriptBuiltinContinuationFrameState(
       jsgraph(), function, Builtins::kArrayForEachLoopEagerDeoptContinuation,
       node->InputAt(0), context, &checkpoint_params[0], stack_parameters,
@@ -622,14 +639,26 @@ Reduction JSCallReducer::ReduceArrayForEach(Handle<JSFunction> function,
       javascript()->Call(5, p.frequency()), fncallback, this_arg, element, k,
       receiver, context, frame_state, effect, control);
 
-  // Update potential {IfException} uses of {node} to point to the above
-  // JavaScript call node within the loop instead.
+  // Rewire potential exception edges.
   Node* on_exception = nullptr;
   if (NodeProperties::IsExceptionalCall(node, &on_exception)) {
-    NodeProperties::ReplaceControlInput(on_exception, control);
-    NodeProperties::ReplaceEffectInput(on_exception, effect);
+    // Create appropriate {IfException} and {IfSuccess} nodes.
+    Node* if_exception0 =
+        graph()->NewNode(common()->IfException(), check_throw, check_fail);
+    check_fail = graph()->NewNode(common()->IfSuccess(), check_fail);
+    Node* if_exception1 =
+        graph()->NewNode(common()->IfException(), effect, control);
     control = graph()->NewNode(common()->IfSuccess(), control);
-    Revisit(on_exception);
+
+    // Join the exception edges.
+    Node* merge =
+        graph()->NewNode(common()->Merge(2), if_exception0, if_exception1);
+    Node* ephi = graph()->NewNode(common()->EffectPhi(2), if_exception0,
+                                  if_exception1, merge);
+    Node* phi =
+        graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
+                         if_exception0, if_exception1, merge);
+    ReplaceWithValue(on_exception, phi, ephi, merge);
   }
 
   if (IsHoleyElementsKind(kind)) {
@@ -652,6 +681,13 @@ Reduction JSCallReducer::ReduceArrayForEach(Handle<JSFunction> function,
   control = if_false;
   effect = eloop;
 
+  // The above %ThrowCalledNonCallable runtime call is an unconditional
+  // throw, making it impossible to return a successful completion in this
+  // case. We simply connect the successful completion to the graph end.
+  Node* terminate =
+      graph()->NewNode(common()->Throw(), check_throw, check_fail);
+  NodeProperties::MergeControlToEnd(graph(), common(), terminate);
+
   ReplaceWithValue(node, jsgraph()->UndefinedConstant(), effect, control);
   return Replace(jsgraph()->UndefinedConstant());
 }
@@ -731,11 +767,32 @@ Reduction JSCallReducer::ReduceArrayMap(Handle<JSFunction> function,
       array_constructor, array_constructor, original_length, context,
       outer_frame_state, effect, control);
 
+  std::vector<Node*> checkpoint_params(
+      {receiver, fncallback, this_arg, a, k, original_length});
+  const int stack_parameters = static_cast<int>(checkpoint_params.size());
+
+  // Check whether the given callback function is callable. Note that this has
+  // to happen outside the loop to make sure we also throw on empty arrays.
+  Node* check = graph()->NewNode(simplified()->ObjectIsCallable(), fncallback);
+  Node* check_branch =
+      graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
+  Node* check_fail = graph()->NewNode(common()->IfFalse(), check_branch);
+  Node* check_frame_state = CreateJavaScriptBuiltinContinuationFrameState(
+      jsgraph(), function, Builtins::kArrayMapLoopLazyDeoptContinuation,
+      node->InputAt(0), context, &checkpoint_params[0], stack_parameters,
+      outer_frame_state, ContinuationFrameStateMode::LAZY);
+  Node* check_throw = check_fail = graph()->NewNode(
+      javascript()->CallRuntime(Runtime::kThrowCalledNonCallable), fncallback,
+      context, check_frame_state, effect, check_fail);
+  control = graph()->NewNode(common()->IfTrue(), check_branch);
+
+  // Start the loop.
   Node* loop = control = graph()->NewNode(common()->Loop(2), control, control);
   Node* eloop = effect =
       graph()->NewNode(common()->EffectPhi(2), effect, effect, loop);
   Node* vloop = k = graph()->NewNode(
       common()->Phi(MachineRepresentation::kTagged, 2), k, k, loop);
+  checkpoint_params[4] = k;
 
   control = loop;
   effect = eloop;
@@ -749,10 +806,6 @@ Reduction JSCallReducer::ReduceArrayMap(Handle<JSFunction> function,
   Node* if_false = graph()->NewNode(common()->IfFalse(), continue_branch);
   control = if_true;
 
-  std::vector<Node*> checkpoint_params(
-      {receiver, fncallback, this_arg, a, k, original_length});
-  const int stack_parameters = static_cast<int>(checkpoint_params.size());
-
   Node* frame_state = CreateJavaScriptBuiltinContinuationFrameState(
       jsgraph(), function, Builtins::kArrayMapLoopEagerDeoptContinuation,
       node->InputAt(0), context, &checkpoint_params[0], stack_parameters,
@@ -803,14 +856,26 @@ Reduction JSCallReducer::ReduceArrayMap(Handle<JSFunction> function,
       javascript()->Call(5, p.frequency()), fncallback, this_arg, element, k,
       receiver, context, frame_state, effect, control);
 
-  // Update potential {IfException} uses of {node} to point to the above
-  // JavaScript call node within the loop instead.
+  // Rewire potential exception edges.
   Node* on_exception = nullptr;
   if (NodeProperties::IsExceptionalCall(node, &on_exception)) {
-    NodeProperties::ReplaceControlInput(on_exception, control);
-    NodeProperties::ReplaceEffectInput(on_exception, effect);
+    // Create appropriate {IfException} and {IfSuccess} nodes.
+    Node* if_exception0 =
+        graph()->NewNode(common()->IfException(), check_throw, check_fail);
+    check_fail = graph()->NewNode(common()->IfSuccess(), check_fail);
+    Node* if_exception1 =
+        graph()->NewNode(common()->IfException(), effect, control);
     control = graph()->NewNode(common()->IfSuccess(), control);
-    Revisit(on_exception);
+
+    // Join the exception edges.
+    Node* merge =
+        graph()->NewNode(common()->Merge(2), if_exception0, if_exception1);
+    Node* ephi = graph()->NewNode(common()->EffectPhi(2), if_exception0,
+                                  if_exception1, merge);
+    Node* phi =
+        graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
+                         if_exception0, if_exception1, merge);
+    ReplaceWithValue(on_exception, phi, ephi, merge);
   }
 
   Handle<Map> double_map(Map::cast(
@@ -830,6 +895,13 @@ Reduction JSCallReducer::ReduceArrayMap(Handle<JSFunction> function,
   control = if_false;
   effect = eloop;
 
+  // The above %ThrowCalledNonCallable runtime call is an unconditional
+  // throw, making it impossible to return a successful completion in this
+  // case. We simply connect the successful completion to the graph end.
+  Node* terminate =
+      graph()->NewNode(common()->Throw(), check_throw, check_fail);
+  NodeProperties::MergeControlToEnd(graph(), common(), terminate);
+
   ReplaceWithValue(node, a, effect, control);
   return Replace(a);
 }

src/compiler/opcodes.h

@@ -345,6 +345,7 @@
   V(StoreElement)                   \
   V(StoreTypedElement)              \
   V(TransitionAndStoreElement)      \
+  V(ObjectIsCallable)               \
   V(ObjectIsDetectableCallable)     \
   V(ObjectIsNaN)                    \
   V(ObjectIsNonCallable)            \

src/compiler/simplified-lowering.cc

@@ -2682,6 +2682,10 @@ class RepresentationSelector {
         if (lower()) DeferReplacement(node, node->InputAt(0));
         return;
       }
+      case IrOpcode::kObjectIsCallable: {
+        VisitObjectIs(node, Type::Callable(), lowering);
+        return;
+      }
       case IrOpcode::kObjectIsDetectableCallable: {
         VisitObjectIs(node, Type::DetectableCallable(), lowering);
         return;

src/compiler/simplified-operator.cc

@@ -546,6 +546,7 @@ UnicodeEncoding UnicodeEncodingOf(const Operator* op) {
   V(TruncateTaggedPointerToBit, Operator::kNoProperties, 1, 0)   \
   V(TruncateTaggedToWord32, Operator::kNoProperties, 1, 0)       \
   V(TruncateTaggedToFloat64, Operator::kNoProperties, 1, 0)      \
+  V(ObjectIsCallable, Operator::kNoProperties, 1, 0)             \
   V(ObjectIsDetectableCallable, Operator::kNoProperties, 1, 0)   \
   V(ObjectIsNaN, Operator::kNoProperties, 1, 0)                  \
   V(ObjectIsNonCallable, Operator::kNoProperties, 1, 0)          \

src/compiler/simplified-operator.h

@@ -450,6 +450,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* CheckNotTaggedHole();
   const Operator* ConvertTaggedHoleToUndefined();
 
+  const Operator* ObjectIsCallable();
   const Operator* ObjectIsDetectableCallable();
   const Operator* ObjectIsNaN();
   const Operator* ObjectIsNonCallable();

src/compiler/typer.cc

@@ -287,6 +287,7 @@ class Typer::Visitor : public Reducer {
   SIMPLIFIED_SPECULATIVE_NUMBER_BINOP_LIST(DECLARE_METHOD)
 #undef DECLARE_METHOD
 
+  static Type* ObjectIsCallable(Type*, Typer*);
   static Type* ObjectIsDetectableCallable(Type*, Typer*);
   static Type* ObjectIsNaN(Type*, Typer*);
   static Type* ObjectIsNonCallable(Type*, Typer*);
@@ -508,6 +509,12 @@ Type* Typer::Visitor::ToString(Type* type, Typer* t) {
 
 // Type checks.
 
+Type* Typer::Visitor::ObjectIsCallable(Type* type, Typer* t) {
+  if (type->Is(Type::Callable())) return t->singleton_true_;
+  if (!type->Maybe(Type::Callable())) return t->singleton_false_;
+  return Type::Boolean();
+}
+
 Type* Typer::Visitor::ObjectIsDetectableCallable(Type* type, Typer* t) {
   if (type->Is(Type::DetectableCallable())) return t->singleton_true_;
   if (!type->Maybe(Type::DetectableCallable())) return t->singleton_false_;
@@ -1951,6 +1958,10 @@ Type* Typer::Visitor::TypeStoreTypedElement(Node* node) {
   UNREACHABLE();
 }
 
+Type* Typer::Visitor::TypeObjectIsCallable(Node* node) {
+  return TypeUnaryOp(node, ObjectIsCallable);
+}
+
 Type* Typer::Visitor::TypeObjectIsDetectableCallable(Node* node) {
   return TypeUnaryOp(node, ObjectIsDetectableCallable);
 }

src/compiler/verifier.cc

@@ -994,6 +994,7 @@ void Verifier::Visitor::Check(Node* node) {
       CheckTypeIs(node, Type::Boolean());
       break;
 
+    case IrOpcode::kObjectIsCallable:
     case IrOpcode::kObjectIsDetectableCallable:
     case IrOpcode::kObjectIsNaN:
     case IrOpcode::kObjectIsNonCallable:

test/mjsunit/regress/regress-crbug-747062.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+(function TestNonCallableForEach() {
+  function foo() { [].forEach(undefined) }
+  assertThrows(foo, TypeError);
+  assertThrows(foo, TypeError);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo, TypeError);
+})();
+
+(function TestNonCallableForEachCaught() {
+  function foo() { try { [].forEach(undefined) } catch(e) { return e } }
+  assertInstanceof(foo(), TypeError);
+  assertInstanceof(foo(), TypeError);
+  %OptimizeFunctionOnNextCall(foo);
+  assertInstanceof(foo(), TypeError);
+})();
+
+(function TestNonCallableMap() {
+  function foo() { [].map(undefined); }
+  assertThrows(foo, TypeError);
+  assertThrows(foo, TypeError);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo, TypeError);
+})();
+
+(function TestNonCallableMapCaught() {
+  function foo() { try { [].map(undefined) } catch(e) { return e } }
+  assertInstanceof(foo(), TypeError);
+  assertInstanceof(foo(), TypeError);
+  %OptimizeFunctionOnNextCall(foo);
+  assertInstanceof(foo(), TypeError);
+})();