Evict deoptimized code on feedback vector after creating the closure

When creating a new closure, we check feedback vector for any optimized
code and install it on the newly created closure. We evict the optimized
code from the feedback vector if it is marked for deoptimization. We
used to evict the code before creating the new closure. However,
creating a new closure could cause allocation failures and hence trigger
a GC. This could mark optimized code on feedback vector for
deoptimization if any weak objects held by optimized code are GC'ed.

This cl delays the eviction unitl after the closure was created.

Bug: v8:1163184
Change-Id: I217279e4a51f75b87bb7ae5a00fd1cf57805e3c8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2613034
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71999}

src/codegen/compiler.cc

@@ -3108,14 +3108,20 @@ void Compiler::PostInstantiation(Handle<JSFunction> function) {
   if (is_compiled_scope.is_compiled() && shared->HasBytecodeArray()) {
     JSFunction::InitializeFeedbackCell(function, &is_compiled_scope);
 
-    Code code = function->has_feedback_vector()
-                    ? function->feedback_vector().optimized_code()
-                    : Code();
-    if (!code.is_null()) {
-      // Caching of optimized code enabled and optimized code found.
-      DCHECK(!code.marked_for_deoptimization());
-      DCHECK(function->shared().is_compiled());
-      function->set_code(code);
+    if (function->has_feedback_vector()) {
+      // Evict any deoptimized code on feedback vector. We need to do this after
+      // creating the closure, since any heap allocations could trigger a GC and
+      // deoptimized the code on the feedback vector. So check for any
+      // deoptimized code just before installing it on the funciton.
+      function->feedback_vector().EvictOptimizedCodeMarkedForDeoptimization(
+          *shared, "new function from shared function info");
+      Code code = function->feedback_vector().optimized_code();
+      if (!code.is_null()) {
+        // Caching of optimized code enabled and optimized code found.
+        DCHECK(!code.marked_for_deoptimization());
+        DCHECK(function->shared().is_compiled());
+        function->set_code(code);
+      }
     }
 
     if (FLAG_always_opt && shared->allows_lazy_compilation() &&

src/heap/factory.cc

@@ -3547,15 +3547,8 @@ void Factory::JSFunctionBuilder::PrepareMap() {
 void Factory::JSFunctionBuilder::PrepareFeedbackCell() {
   Handle<FeedbackCell> feedback_cell;
   if (maybe_feedback_cell_.ToHandle(&feedback_cell)) {
-    // Track the newly-created closure, and check that the optimized code in
-    // the feedback cell wasn't marked for deoptimization while not pointed to
-    // by any live JSFunction.
+    // Track the newly-created closure.
     feedback_cell->IncrementClosureCount(isolate_);
-    if (feedback_cell->value().IsFeedbackVector()) {
-      FeedbackVector::cast(feedback_cell->value())
-          .EvictOptimizedCodeMarkedForDeoptimization(
-              *sfi_, "new function from shared function info");
-    }
   } else {
     // Fall back to the many_closures_cell.
     maybe_feedback_cell_ = isolate_->factory()->many_closures_cell();

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

@@ -0,0 +1,29 @@
+// 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: --allow-natives-syntax --stress-compact
+
+let arr = [20];
+// This forces arr.concat to create a new dictionary map which can be collected
+// on a GC.
+arr[Symbol.isConcatSpreadable] = true;
+
+for (let i = 0; i < 4; ++i) {
+  function tmp() {
+    // Creates a new map that is collected on a GC.
+    let c = arr.concat();
+    // Access something from c, so c's map is embedded in code object.
+    c.x;
+  };
+  %PrepareFunctionForOptimization(tmp);
+  tmp();
+  // Optimize on the second iteration, so the optimized code isn't function
+  // context specialized and installed on feedback vector.
+  if (i == 1) {
+    %OptimizeFunctionOnNextCall(tmp);
+    tmp();
+  }
+  // Simulate full Newspace, so on next closure creation we cause a GC.
+  if (i == 2) %SimulateNewspaceFull();
+}