[runtime] Fix reentrancy bug in JSFunction::EnsureHasInitialMap

Foozie came up with a mind-boggling example hitting a similarly
mind-boggling bug: object construction (JSObject::New) wants to create
the constructor's function initial map (JSFunction::GetDerivedMap ->
JSFunction::EnsureHasInitialMap). To do so, it calls
JSFunction::CalculateExpectedNofProperties. This harmless sounding
function triggers compilation of the function. Since we're running with
--always-opt, this is an optimizing compilation. Turbofan ends up
depending on the function's "prototype" property, for which it wants to
create the initial map so that it can install the code dependency. That
is, EnsureHasInitialMap is reentered. At this point there is no further
compilation attempt because the bytecode now exists. The initial map is
created and installed on the function, and TF records the code
dependency on that map. When CalculateExpectedNofProperties returns
control to the outer EnsureHasInitialMap, yet another initial map is
created and set on the function, forgetting the previous one and thus
the code dependency.

I'm not sure if this bug can only be observed with --always-opt. The fix
is general.

Bug: chromium:1092011
Change-Id: I8b972748e49b9eb8f06fa17ea9ca037de2bd7532
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2238570Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68292}

src/objects/js-objects.cc

@@ -5161,8 +5161,16 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
   if (function->has_initial_map()) return;
   Isolate* isolate = function->GetIsolate();
 
-  // First create a new map with the size and number of in-object properties
-  // suggested by the function.
+  int expected_nof_properties =
+      CalculateExpectedNofProperties(isolate, function);
+
+  // {CalculateExpectedNofProperties} can have had the side effect of creating
+  // the initial map (e.g. it could have triggered an optimized compilation
+  // whose dependency installation reentered {EnsureHasInitialMap}).
+  if (function->has_initial_map()) return;
+
+  // Create a new map with the size and number of in-object properties suggested
+  // by the function.
   InstanceType instance_type;
   if (IsResumableFunction(function->shared().kind())) {
     instance_type = IsAsyncGeneratorFunction(function->shared().kind())
@@ -5174,8 +5182,6 @@ void JSFunction::EnsureHasInitialMap(Handle<JSFunction> function) {
 
   int instance_size;
   int inobject_properties;
-  int expected_nof_properties =
-      CalculateExpectedNofProperties(isolate, function);
   CalculateInstanceSizeHelper(instance_type, false, 0, expected_nof_properties,
                               &instance_size, &inobject_properties);
 

src/objects/js-objects.h

@@ -1136,6 +1136,7 @@ class JSFunction : public JSFunctionOrBoundFunction {
   DECL_CAST(JSFunction)
 
   // Calculate the instance size and in-object properties count.
+  // {CalculateExpectedNofProperties} can trigger compilation.
   static V8_WARN_UNUSED_RESULT int CalculateExpectedNofProperties(
       Isolate* isolate, Handle<JSFunction> function);
   static void CalculateInstanceSizeHelper(InstanceType instance_type,

test/mjsunit/compiler/regress-1092011.js

+// Copyright 2020 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: --always-opt
+
+var __caught = 0;
+
+(function main() {
+  function foo(f) {
+    const pr = new Promise(function executor() {
+      f(function resolvefun() {
+        try {
+          throw 42;
+        } catch (e) {
+          __caught++;
+        }
+      }, function rejectfun() {});
+    });
+    pr.__proto__ = foo.prototype;
+    return pr;
+  }
+  foo.__proto__ = Promise;
+  foo.prototype.then = function thenfun() {};
+  new foo();
+  foo.prototype = undefined;
+  foo.all([foo.resolve()]);
+})();
+
+assertEquals(2, __caught);