[compiler] Fix serialization for Function#bind

It was not in sync with the optimization, which relies on
inspecting up the length and name fields even for bound
functions.

To make a now meaningful serializer test actually pass, I have
to to make some changes to the test setup.

I'm also moving the function name and length index constants
from JSFunction to JSFunctionOrBoundFunction for clarity.

TBR=marja@chromium.org

Bug: v8:7790
Change-Id: I36dd3c80996ccb53810c7ea9bfceb5c84ffd60ab
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2972919
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75299}

src/builtins/function.tq

@@ -16,10 +16,10 @@ extern transitioning builtin
 FunctionPrototypeBind(implicit context: Context)(
     JSFunction, JSAny, int32): JSAny;
 
-const kLengthDescriptorIndex:
-    constexpr int32 generates 'JSFunction::kLengthDescriptorIndex';
-const kNameDescriptorIndex:
-    constexpr int32 generates 'JSFunction::kNameDescriptorIndex';
+const kLengthDescriptorIndex: constexpr int32
+    generates 'JSFunctionOrBoundFunction::kLengthDescriptorIndex';
+const kNameDescriptorIndex: constexpr int32
+    generates 'JSFunctionOrBoundFunction::kNameDescriptorIndex';
 const kMinDescriptorsForFastBind:
     constexpr int31 generates 'JSFunction::kMinDescriptorsForFastBind';
 

src/compiler/js-call-reducer.cc

@@ -34,7 +34,7 @@
 #include "src/objects/feedback-vector-inl.h"
 #include "src/objects/js-array-buffer-inl.h"
 #include "src/objects/js-array-inl.h"
-#include "src/objects/js-objects.h"
+#include "src/objects/js-function.h"
 #include "src/objects/objects-inl.h"
 #include "src/objects/ordered-hash-table.h"
 
@@ -2633,14 +2633,17 @@ Reduction JSCallReducer::ReduceFunctionPrototypeBind(Node* node) {
     // recomputed even if the actual value of the object changes.
     // This mirrors the checks done in builtins-function-gen.cc at
     // runtime otherwise.
-    int minimum_nof_descriptors = std::max({JSFunction::kLengthDescriptorIndex,
-                                            JSFunction::kNameDescriptorIndex}) +
+    int minimum_nof_descriptors =
+        std::max({JSFunctionOrBoundFunction::kLengthDescriptorIndex,
+                  JSFunctionOrBoundFunction::kNameDescriptorIndex}) +
         1;
     if (receiver_map.NumberOfOwnDescriptors() < minimum_nof_descriptors) {
       return inference.NoChange();
     }
-    const InternalIndex kLengthIndex(JSFunction::kLengthDescriptorIndex);
-    const InternalIndex kNameIndex(JSFunction::kNameDescriptorIndex);
+    const InternalIndex kLengthIndex(
+        JSFunctionOrBoundFunction::kLengthDescriptorIndex);
+    const InternalIndex kNameIndex(
+        JSFunctionOrBoundFunction::kNameDescriptorIndex);
     if (!receiver_map.serialized_own_descriptor(kLengthIndex) ||
         !receiver_map.serialized_own_descriptor(kNameIndex)) {
       TRACE_BROKER_MISSING(broker(),

src/compiler/serializer-for-background-compilation.cc

@@ -2716,8 +2716,9 @@ void ProcessMapForFunctionBind(MapRef map) {
                             1;
   if (map.NumberOfOwnDescriptors() >= min_nof_descriptors) {
     map.SerializeOwnDescriptor(
-        InternalIndex(JSFunction::kLengthDescriptorIndex));
-    map.SerializeOwnDescriptor(InternalIndex(JSFunction::kNameDescriptorIndex));
+        InternalIndex(JSFunctionOrBoundFunction::kLengthDescriptorIndex));
+    map.SerializeOwnDescriptor(
+        InternalIndex(JSFunctionOrBoundFunction::kNameDescriptorIndex));
   }
 }
 }  // namespace
@@ -2725,17 +2726,22 @@ void ProcessMapForFunctionBind(MapRef map) {
 void SerializerForBackgroundCompilation::ProcessHintsForFunctionBind(
     Hints const& receiver_hints) {
   for (auto constant : receiver_hints.constants()) {
-    if (!constant->IsJSFunction()) continue;
+    if (constant->IsJSFunction()) {
       JSFunctionRef function =
           MakeRef(broker(), Handle<JSFunction>::cast(constant));
       function.Serialize();
       ProcessMapForFunctionBind(function.map());
+    } else if (constant->IsJSBoundFunction()) {
+      JSBoundFunctionRef function =
+          MakeRef(broker(), Handle<JSBoundFunction>::cast(constant));
+      function.Serialize();
+      ProcessMapForFunctionBind(function.map());
+    }
   }
 
   for (auto map : receiver_hints.maps()) {
-    if (!map->IsJSFunctionMap()) continue;
-    MapRef map_ref = MakeRef(broker(), map);
-    ProcessMapForFunctionBind(map_ref);
+    if (!map->IsJSFunctionMap() && !map->IsJSBoundFunctionMap()) continue;
+    ProcessMapForFunctionBind(MakeRef(broker(), map));
   }
 }
 

src/heap/factory.cc

@@ -3572,14 +3572,14 @@ Handle<Map> Factory::CreateSloppyFunctionMap(
       static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY);
 
   int field_index = 0;
-  STATIC_ASSERT(JSFunction::kLengthDescriptorIndex == 0);
+  STATIC_ASSERT(JSFunctionOrBoundFunction::kLengthDescriptorIndex == 0);
   {  // Add length accessor.
     Descriptor d = Descriptor::AccessorConstant(
         length_string(), function_length_accessor(), roc_attribs);
     map->AppendDescriptor(isolate(), &d);
   }
 
-  STATIC_ASSERT(JSFunction::kNameDescriptorIndex == 1);
+  STATIC_ASSERT(JSFunctionOrBoundFunction::kNameDescriptorIndex == 1);
   if (IsFunctionModeWithName(function_mode)) {
     // Add name field.
     Handle<Name> name = isolate()->factory()->name_string();

src/init/bootstrapper.cc

@@ -3871,6 +3871,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     Map::EnsureDescriptorSlack(isolate_, map, 2);
 
     {  // length
+      STATIC_ASSERT(JSFunctionOrBoundFunction::kLengthDescriptorIndex == 0);
       Descriptor d = Descriptor::AccessorConstant(
           factory->length_string(), factory->bound_function_length_accessor(),
           roc_attribs);
@@ -3878,6 +3879,7 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     }
 
     {  // name
+      STATIC_ASSERT(JSFunctionOrBoundFunction::kNameDescriptorIndex == 1);
       Descriptor d = Descriptor::AccessorConstant(
           factory->name_string(), factory->bound_function_name_accessor(),
           roc_attribs);

src/objects/js-function.h

@@ -24,6 +24,9 @@ class JSFunctionOrBoundFunction
     : public TorqueGeneratedJSFunctionOrBoundFunction<JSFunctionOrBoundFunction,
                                                       JSObject> {
  public:
+  static const int kLengthDescriptorIndex = 0;
+  static const int kNameDescriptorIndex = 1;
+
   STATIC_ASSERT(kHeaderSize == JSObject::kHeaderSize);
   TQ_OBJECT_CONSTRUCTORS(JSFunctionOrBoundFunction)
 };
@@ -59,9 +62,6 @@ class JSFunction : public JSFunctionOrBoundFunction {
   // can be shared by instances.
   DECL_ACCESSORS(shared, SharedFunctionInfo)
 
-  static const int kLengthDescriptorIndex = 0;
-  static const int kNameDescriptorIndex = 1;
-
   // Fast binding requires length and name accessors.
   static const int kMinDescriptorsForFastBind = 2;
 

test/cctest/cctest.cc

@@ -272,6 +272,7 @@ i::Handle<i::JSFunction> Optimize(
   i::OptimizedCompilationInfo info(zone, isolate, shared, function,
                                    i::CodeKind::TURBOFAN);
 
+  if (flags & ~i::OptimizedCompilationInfo::kInlining) UNIMPLEMENTED();
   if (flags & i::OptimizedCompilationInfo::kInlining) {
     info.set_inlining();
   }

test/cctest/compiler/serializer-tester.cc

@@ -18,7 +18,8 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-SerializerTester::SerializerTester(const char* source)
+SerializerTester::SerializerTester(const char* global_source,
+                                   const char* local_source)
     : canonical_(main_isolate()) {
   // The tests only make sense in the context of concurrent compilation.
   FLAG_concurrent_inlining = true;
@@ -33,18 +34,14 @@ SerializerTester::SerializerTester(const char* source)
   FLAG_allow_natives_syntax = true;
   FlagList::EnforceFlagImplications();
 
+  CompileRun(global_source);
+
   std::string function_string = "(function() { ";
-  function_string += source;
+  function_string += local_source;
   function_string += " })();";
   Handle<JSFunction> function = Handle<JSFunction>::cast(v8::Utils::OpenHandle(
       *v8::Local<v8::Function>::Cast(CompileRun(function_string.c_str()))));
-  uint32_t flags = i::OptimizedCompilationInfo::kInlining |
-                   i::OptimizedCompilationInfo::kFunctionContextSpecializing |
-                   i::OptimizedCompilationInfo::kLoopPeeling |
-                   i::OptimizedCompilationInfo::kBailoutOnUninitialized |
-                   i::OptimizedCompilationInfo::kAllocationFolding |
-                   i::OptimizedCompilationInfo::kSplitting |
-                   i::OptimizedCompilationInfo::kAnalyzeEnvironmentLiveness;
+  uint32_t flags = i::OptimizedCompilationInfo::kInlining;
   Optimize(function, main_zone(), main_isolate(), flags, &broker_);
   // Update handle to the corresponding serialized Handle in the broker.
   function =
@@ -54,7 +51,7 @@ SerializerTester::SerializerTester(const char* source)
 
 TEST(SerializeEmptyFunction) {
   SerializerTester tester(
-      "function f() {}; %EnsureFeedbackVectorForFunction(f); return f;");
+      "", "function f() {}; %EnsureFeedbackVectorForFunction(f); return f;");
   JSFunctionRef function = tester.function();
   CHECK(tester.broker()->IsSerializedForCompilation(
       function.shared(), function.feedback_vector()));
@@ -63,9 +60,10 @@ TEST(SerializeEmptyFunction) {
 // This helper function allows for testing whether an inlinee candidate
 // was properly serialized. It expects that the top-level function (that is
 // run through the SerializerTester) will return its inlinee candidate.
-void CheckForSerializedInlinee(const char* source, int argc = 0,
+void CheckForSerializedInlinee(const char* global_source,
+                               const char* local_source, int argc = 0,
                                Handle<Object> argv[] = {}) {
-  SerializerTester tester(source);
+  SerializerTester tester(global_source, local_source);
   JSFunctionRef f = tester.function();
   CHECK(tester.broker()->IsSerializedForCompilation(f.shared(),
                                                     f.feedback_vector()));
@@ -75,7 +73,6 @@ void CheckForSerializedInlinee(const char* source, int argc = 0,
       tester.isolate()->factory()->undefined_value(), argc, argv);
   Handle<Object> g;
   CHECK(g_obj.ToHandle(&g));
-
   CHECK_WITH_MSG(
       g->IsJSFunction(),
       "The return value of the outer function must be a function too");
@@ -95,7 +92,7 @@ void CheckForSerializedInlinee(const char* source, int argc = 0,
 }
 
 TEST(SerializeInlinedClosure) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function f() {"
                             "  function g(){ return g; }"
                             "  %EnsureFeedbackVectorForFunction(g);"
@@ -106,7 +103,7 @@ TEST(SerializeInlinedClosure) {
 }
 
 TEST(SerializeInlinedFunction) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g() {};"
                             "%EnsureFeedbackVectorForFunction(g);"
                             "function f() {"
@@ -117,7 +114,7 @@ TEST(SerializeInlinedFunction) {
 }
 
 TEST(SerializeCallUndefinedReceiver) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g(a,b,c) {};"
                             "%EnsureFeedbackVectorForFunction(g);"
                             "function f() {"
@@ -128,7 +125,7 @@ TEST(SerializeCallUndefinedReceiver) {
 }
 
 TEST(SerializeCallUndefinedReceiver2) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g(a,b) {};"
                             "%EnsureFeedbackVectorForFunction(g);"
                             "function f() {"
@@ -139,7 +136,7 @@ TEST(SerializeCallUndefinedReceiver2) {
 }
 
 TEST(SerializeCallProperty) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "let obj = {"
                             "  g: function g(a,b,c) {}"
                             "};"
@@ -152,7 +149,7 @@ TEST(SerializeCallProperty) {
 }
 
 TEST(SerializeCallProperty2) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "let obj = {"
                             "  g: function g(a,b) {}"
                             "};"
@@ -165,7 +162,7 @@ TEST(SerializeCallProperty2) {
 }
 
 TEST(SerializeCallAnyReceiver) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "let obj = {"
                             "  g: function g() {}"
                             "};"
@@ -180,7 +177,7 @@ TEST(SerializeCallAnyReceiver) {
 }
 
 TEST(SerializeCallWithSpread) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g(args) {};"
                             "%EnsureFeedbackVectorForFunction(g);"
                             "const arr = [1,2,3];"
@@ -195,7 +192,7 @@ TEST(SerializeCallWithSpread) {
 // thus allowing us to test if we forward arguments hints (`callee` in this
 // example) and correctly serialize the inlining candidate `j`.
 TEST(SerializeCallArguments) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g(callee) { callee(); };"
                             "function h() {};"
                             "function i() {};"
@@ -213,7 +210,7 @@ TEST(SerializeCallArguments) {
 }
 
 TEST(SerializeConstruct) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g() {};"
                             "%EnsureFeedbackVectorForFunction(g);"
                             "function f() {"
@@ -224,7 +221,7 @@ TEST(SerializeConstruct) {
 }
 
 TEST(SerializeConstructWithSpread) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g(a, b, c) {};"
                             "%EnsureFeedbackVectorForFunction(g);"
                             "const arr = [1, 2];"
@@ -236,7 +233,7 @@ TEST(SerializeConstructWithSpread) {
 }
 
 TEST(SerializeConstructSuper) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "class A {};"
                             "class B extends A { constructor() { super(); } };"
                             "%EnsureFeedbackVectorForFunction(A);"
@@ -249,7 +246,7 @@ TEST(SerializeConstructSuper) {
 }
 
 TEST(SerializeConditionalJump) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g(callee) { callee(); };"
                             "function h() {};"
                             "function i() {};"
@@ -268,7 +265,7 @@ TEST(SerializeConditionalJump) {
 }
 
 TEST(SerializeUnconditionalJump) {
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function g(callee) { callee(); };"
                             "function h() {};"
                             "function i() {};"
@@ -292,6 +289,7 @@ TEST(SerializeUnconditionalJump) {
 
 TEST(MergeJumpTargetEnvironment) {
   CheckForSerializedInlinee(
+      "",
       "function f() {"
       "  let g;"
       "  while (true) {"
@@ -305,27 +303,29 @@ TEST(MergeJumpTargetEnvironment) {
 }
 
 TEST(BoundFunctionTarget) {
+  const char* global = "function apply1(foo, arg) { return foo(arg); };";
   CheckForSerializedInlinee(
-      "function apply(foo, arg) { return foo(arg); };"
-      "%EnsureFeedbackVectorForFunction(apply);"
+      global,
+      "%EnsureFeedbackVectorForFunction(apply1);"
       "function test() {"
       "  const lambda = (a) => a;"
       "  %EnsureFeedbackVectorForFunction(lambda);"
-      "  let bound = apply.bind(null, lambda).bind(null, 42);"
-      "  %TurbofanStaticAssert(bound() == 42); return apply;"
+      "  let bound = apply1.bind(null, lambda).bind(null, 42);"
+      "  %TurbofanStaticAssert(bound() == 42); return apply1;"
       "};"
       "%EnsureFeedbackVectorForFunction(test);"
       "test(); return test;");
 }
 
 TEST(BoundFunctionArguments) {
+  const char* global = "function apply2(foo, arg) { return foo(arg); };";
   CheckForSerializedInlinee(
-      "function apply(foo, arg) { return foo(arg); };"
-      "%EnsureFeedbackVectorForFunction(apply);"
+      global,
+      "%EnsureFeedbackVectorForFunction(apply2);"
       "function test() {"
       "  const lambda = (a) => a;"
       "  %EnsureFeedbackVectorForFunction(lambda);"
-      "  let bound = apply.bind(null, lambda).bind(null, 42);"
+      "  let bound = apply2.bind(null, lambda).bind(null, 42);"
       "  %TurbofanStaticAssert(bound() == 42); return lambda;"
       "};"
       "%EnsureFeedbackVectorForFunction(test);"
@@ -335,7 +335,7 @@ TEST(BoundFunctionArguments) {
 TEST(ArrowFunctionInlined) {
   // The loop is to ensure there is a feedback vector for the arrow function
   // {b}.
-  CheckForSerializedInlinee(
+  CheckForSerializedInlinee("",
                             "function foo() {"
                             "  let b = x => x * x;"
                             "  let a = [1, 2, 3].map(b);"
@@ -348,6 +348,7 @@ TEST(ArrowFunctionInlined) {
 
 TEST(BoundFunctionResult) {
   CheckForSerializedInlinee(
+      "",
       "function id(x) { return x }"
       "function foo() { id.bind(undefined, 42)(); return id; }"
       "%PrepareFunctionForOptimization(foo);"
@@ -360,6 +361,7 @@ TEST(BoundFunctionResult) {
 
 TEST(MultipleFunctionCalls) {
   CheckForSerializedInlinee(
+      "",
       "function inc(x) { return ++x; }"
       "function dec(x) { return --x; }"
       "function apply(f, x) { return f(x); }"

test/cctest/compiler/serializer-tester.h

@@ -18,15 +18,16 @@ class ZoneStats;
 
 // The purpose of this class is to provide testing facility for the
 // SerializerForBackgroundCompilation class. On a high-level, it executes the
-// following steps:
-// 1. Wraps the provided source in an IIFE
-// 2. Generates bytecode for the given source
-// 3. Runs the bytecode which *must* return a function
-// 4. Takes the returned function and optimizes it
-// 5. The optimized function is accessible through `function()`
+// code given by {global_source} at global scope and then performs the following
+// steps for {local_source}:
+// 1. Wraps it in an IIFE.
+// 2. Generates bytecode and runs it, which *must* return a function.
+// 3. Takes the returned function and optimizes it.
+// 4. The optimized function is accessible through `function()`.
 class SerializerTester : public HandleAndZoneScope {
  public:
-  explicit SerializerTester(const char* source);
+  explicit SerializerTester(const char* global_source,
+                            const char* local_source);
 
   JSFunctionRef function() const { return function_.value(); }
   JSHeapBroker* broker() const { return broker_.get(); }