[turbofan] Introduce Type for Class Constructors

This CL splits the TF type for JSFunction into CallableFunction and
ClassConstructor. This differentiation allows us to lower calls to the
CallFunction Builtin only for functions that we can actually call.
Class Constructors are special, as they are callable but should raise
an exception if called.
By not lowering class constructors to calls to CallFunction (but the
more generall Call) builtin, we can remove the checks for class
constructors from CallFunction (in a follow-up CL).

Bug: chromium:1262750
Change-Id: I399967eb03b2f20d2dcb67aef2243b32c9d3174e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3350457
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Patrick Thier <pthier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78445}

src/compiler/access-builder.cc

@@ -243,9 +243,12 @@ FieldAccess AccessBuilder::ForJSGeneratorObjectContext() {
 
 // static
 FieldAccess AccessBuilder::ForJSGeneratorObjectFunction() {
-  FieldAccess access = {kTaggedBase,         JSGeneratorObject::kFunctionOffset,
+  FieldAccess access = {kTaggedBase,
-                        Handle<Name>(),      MaybeHandle<Map>(),
+                        JSGeneratorObject::kFunctionOffset,
-                        Type::Function(),    MachineType::TaggedPointer(),
+                        Handle<Name>(),
+                        MaybeHandle<Map>(),
+                        Type::CallableFunction(),
+                        MachineType::TaggedPointer(),
                         kPointerWriteBarrier};
   return access;
 }

src/compiler/js-create-lowering.cc

@@ -939,6 +939,9 @@ Reduction JSCreateLowering::ReduceJSCreateClosure(Node* node) {
     return NoChange();
   }
 
+  // Don't inline anything for class constructors.
+  if (IsClassConstructor(shared.kind())) return NoChange();
+
   MapRef function_map =
       native_context().GetFunctionMapFromIndex(shared.function_map_index());
   DCHECK(!function_map.IsInobjectSlackTrackingInProgress());
@@ -958,7 +961,8 @@ Reduction JSCreateLowering::ReduceJSCreateClosure(Node* node) {
   // Emit code to allocate the JSFunction instance.
   STATIC_ASSERT(JSFunction::kSizeWithoutPrototype == 7 * kTaggedSize);
   AllocationBuilder a(jsgraph(), effect, control);
-  a.Allocate(function_map.instance_size(), allocation, Type::Function());
+  a.Allocate(function_map.instance_size(), allocation,
+             Type::CallableFunction());
   a.Store(AccessBuilder::ForMap(), function_map);
   a.Store(AccessBuilder::ForJSObjectPropertiesOrHashKnownPointer(),
           jsgraph()->EmptyFixedArrayConstant());

src/compiler/js-typed-lowering.cc

@@ -1657,15 +1657,11 @@ Reduction JSTypedLowering::ReduceJSCallForwardVarargs(Node* node) {
   Node* target = NodeProperties::GetValueInput(node, 0);
   Type target_type = NodeProperties::GetType(target);
 
-  // Check if {target} is a JSFunction.
+  // Check if {target} is a directly callable JSFunction.
-  if (target_type.Is(Type::Function())) {
+  if (target_type.Is(Type::CallableFunction())) {
     // Compute flags for the call.
     CallDescriptor::Flags flags = CallDescriptor::kNeedsFrameState;
     // Patch {node} to an indirect call via CallFunctionForwardVarargs.
-    // It is safe to call CallFunction instead of Call, as we already checked
-    // that the target is a function that is not a class constructor in
-    // JSCallReduer.
-    // TODO(pthier): We shouldn't blindly rely on checks made in another pass.
     Callable callable = CodeFactory::CallFunctionForwardVarargs(isolate());
     node->InsertInput(graph()->zone(), 0,
                       jsgraph()->HeapConstant(callable.code()));
@@ -1814,15 +1810,13 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
     return Changed(node);
   }
 
-  // Check if {target} is a JSFunction.
+  // Check if {target} is a directly callable JSFunction.
-  if (target_type.Is(Type::Function())) {
+  if (target_type.Is(Type::CallableFunction())) {
     // The node will change operators, remove the feedback vector.
     node->RemoveInput(n.FeedbackVectorIndex());
     // Compute flags for the call.
     CallDescriptor::Flags flags = CallDescriptor::kNeedsFrameState;
     // Patch {node} to an indirect call via the CallFunction builtin.
-    // It is safe to call CallFunction instead of Call, as we already checked
-    // that the target is a function that is not a class constructor.
     Callable callable = CodeFactory::CallFunction(isolate(), convert_mode);
     node->InsertInput(graph()->zone(), 0,
                       jsgraph()->HeapConstant(callable.code()));

src/compiler/typer.cc

@@ -1262,7 +1262,13 @@ Type Typer::Visitor::TypeJSCreateGeneratorObject(Node* node) {
 }
 
 Type Typer::Visitor::TypeJSCreateClosure(Node* node) {
-  return Type::Function();
+  SharedFunctionInfoRef shared =
+      JSCreateClosureNode{node}.Parameters().shared_info(typer_->broker());
+  if (IsClassConstructor(shared.kind())) {
+    return Type::ClassConstructor();
+  } else {
+    return Type::CallableFunction();
+  }
 }
 
 Type Typer::Visitor::TypeJSCreateIterResultObject(Node* node) {
@@ -2142,7 +2148,17 @@ Type Typer::Visitor::TypeCheckNotTaggedHole(Node* node) {
   return type;
 }
 
-Type Typer::Visitor::TypeCheckClosure(Node* node) { return Type::Function(); }
+Type Typer::Visitor::TypeCheckClosure(Node* node) {
+  FeedbackCellRef cell = MakeRef(typer_->broker(), FeedbackCellOf(node->op()));
+  base::Optional<SharedFunctionInfoRef> shared = cell.shared_function_info();
+  if (!shared.has_value()) return Type::Function();
+
+  if (IsClassConstructor(shared->kind())) {
+    return Type::ClassConstructor();
+  } else {
+    return Type::CallableFunction();
+  }
+}
 
 Type Typer::Visitor::TypeConvertReceiver(Node* node) {
   Type arg = Operand(node, 0);

src/compiler/types.cc

@@ -286,7 +286,6 @@ Type::bitset BitsetType::Lub(const MapRefLike& map) {
       DCHECK(!map.is_undetectable());
       return kBoundFunction;
     case JS_FUNCTION_TYPE:
-    case JS_CLASS_CONSTRUCTOR_TYPE:
     case JS_PROMISE_CONSTRUCTOR_TYPE:
     case JS_REG_EXP_CONSTRUCTOR_TYPE:
     case JS_ARRAY_CONSTRUCTOR_TYPE:
@@ -295,7 +294,9 @@ Type::bitset BitsetType::Lub(const MapRefLike& map) {
       TYPED_ARRAYS(TYPED_ARRAY_CONSTRUCTORS_SWITCH)
 #undef TYPED_ARRAY_CONSTRUCTORS_SWITCH
       DCHECK(!map.is_undetectable());
-      return kFunction;
+      return kCallableFunction;
+    case JS_CLASS_CONSTRUCTOR_TYPE:
+      return kClassConstructor;
     case JS_PROXY_TYPE:
       DCHECK(!map.is_undetectable());
       if (map.is_callable()) return kCallableProxy;

src/compiler/types.h

@@ -117,25 +117,26 @@ namespace compiler {
   V(OtherUndetectable,        uint64_t{1} << 17)  \
   V(CallableProxy,            uint64_t{1} << 18)  \
   V(OtherProxy,               uint64_t{1} << 19)  \
-  V(Function,                 uint64_t{1} << 20)  \
+  V(CallableFunction,         uint64_t{1} << 20)  \
-  V(BoundFunction,            uint64_t{1} << 21)  \
+  V(ClassConstructor,         uint64_t{1} << 21)  \
-  V(Hole,                     uint64_t{1} << 22)  \
+  V(BoundFunction,            uint64_t{1} << 22)  \
-  V(OtherInternal,            uint64_t{1} << 23)  \
+  V(Hole,                     uint64_t{1} << 23)  \
-  V(ExternalPointer,          uint64_t{1} << 24)  \
+  V(OtherInternal,            uint64_t{1} << 24)  \
-  V(Array,                    uint64_t{1} << 25)  \
+  V(ExternalPointer,          uint64_t{1} << 25)  \
-  V(UnsignedBigInt63,         uint64_t{1} << 26)  \
+  V(Array,                    uint64_t{1} << 26)  \
-  V(OtherUnsignedBigInt64,    uint64_t{1} << 27)  \
+  V(UnsignedBigInt63,         uint64_t{1} << 27)  \
-  V(NegativeBigInt63,         uint64_t{1} << 28)  \
+  V(OtherUnsignedBigInt64,    uint64_t{1} << 28)  \
-  V(OtherBigInt,              uint64_t{1} << 29)  \
+  V(NegativeBigInt63,         uint64_t{1} << 29)  \
-  /* TODO(v8:10391): Remove this type once all ExternalPointer usages are */ \
+  V(OtherBigInt,              uint64_t{1} << 30)  \
-  /* sandbox-ready. */                     \
+  V(WasmObject,               uint64_t{1} << 31)
-  V(SandboxedExternalPointer, uint64_t{1} << 30)  \
-  V(SandboxedPointer,         uint64_t{1} << 31)
 
 // We split the macro list into two parts because the Torque equivalent in
 // turbofan-types.tq uses two 32bit bitfield structs.
-#define PROPER_ATOMIC_BITSET_TYPE_HIGH_LIST(V)  \
+#define PROPER_ATOMIC_BITSET_TYPE_HIGH_LIST(V) \
-  V(WasmObject,               uint64_t{1} << 32)
+  /* TODO(v8:10391): Remove this type once all ExternalPointer usages are */ \
+  /* sandbox-ready. */                     \
+  V(SandboxedExternalPointer, uint64_t{1} << 32)  \
+  V(SandboxedPointer,         uint64_t{1} << 33)
 
 #define PROPER_BITSET_TYPE_LIST(V) \
   V(None,                     uint64_t{0}) \
@@ -190,6 +191,7 @@ namespace compiler {
   V(Proxy,                        kCallableProxy | kOtherProxy) \
   V(ArrayOrOtherObject,           kArray | kOtherObject) \
   V(ArrayOrProxy,                 kArray | kProxy) \
+  V(Function,                     kCallableFunction | kClassConstructor) \
   V(DetectableCallable,           kFunction | kBoundFunction | \
                                   kOtherCallable | kCallableProxy) \
   V(Callable,                     kDetectableCallable | kOtherUndetectable) \

src/objects/turbofan-types.tq

@@ -33,7 +33,8 @@ bitfield struct TurbofanTypeLowBits extends uint32 {
   other_undetectable: bool: 1 bit;
   callable_proxy: bool: 1 bit;
   other_proxy: bool: 1 bit;
-  function: bool: 1 bit;
+  callable_function: bool: 1 bit;
+  class_constructor: bool: 1 bit;
   bound_function: bool: 1 bit;
   hole: bool: 1 bit;
   other_internal: bool: 1 bit;
@@ -43,12 +44,12 @@ bitfield struct TurbofanTypeLowBits extends uint32 {
   other_unsigned_big_int_64: bool: 1 bit;
   negative_big_int_63: bool: 1 bit;
   other_big_int: bool: 1 bit;
-  sandboxed_external_pointer: bool: 1 bit;
+  wasm_object: bool: 1 bit;
-  sandboxed_pointer: bool: 1 bit;
 }
 
 bitfield struct TurbofanTypeHighBits extends uint32 {
-  wasm_object: bool: 1 bit;
+  sandboxed_external_pointer: bool: 1 bit;
+  sandboxed_pointer: bool: 1 bit;
 }
 
 @export
@@ -138,8 +139,12 @@ macro TestTurbofanBitsetType(
       return Is<Callable>(proxy) ? bitsetLow.callable_proxy :
                                    bitsetLow.other_proxy;
     }
-    case (JSFunction): {
+    case (fun: JSFunction): {
-      return bitsetLow.function;
+      if (fun.shared_function_info.flags.is_class_constructor) {
+        return bitsetLow.class_constructor;
+      } else {
+        return bitsetLow.callable_function;
+      }
     }
     case (JSBoundFunction): {
       return bitsetLow.bound_function;
@@ -167,7 +172,7 @@ macro TestTurbofanBitsetType(
     }
     @if(V8_ENABLE_WEBASSEMBLY)
     case (WasmObject): {
-      return bitsetHigh.wasm_object;
+      return bitsetLow.wasm_object;
     }
     case (Object): {
       return false;

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

@@ -0,0 +1,36 @@
+// Copyright 2021 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
+
+// Test calling a class constructor on a polymorphic object throws a TypeError.
+function f(o) {
+  o.get();
+}
+
+let obj = new Map();
+%PrepareFunctionForOptimization(f);
+f(obj);
+f(obj);
+
+obj.get = class C {};
+assertThrows(() => f(obj), TypeError);
+%OptimizeFunctionOnNextCall(f);
+assertThrows(() => f(obj), TypeError);
+
+// Test calling a closure of a class constructor throws a TypeError.
+function g(a) {
+  var f;
+  f = class {};
+  if (a == 1) {
+    f = function() {};
+  }
+  f();
+}
+
+%PrepareFunctionForOptimization(g);
+assertThrows(g, TypeError);
+assertThrows(g, TypeError);
+%OptimizeFunctionOnNextCall(g);
+assertThrows(g, TypeError);