Check if a function has feedback vector before OSRing.

With bytecode flushing and the current OSR triggering mechanism which
stores OSR nesting level on bytecode array it is possible to trigger
OSR on a closure that doesn't have feedback vector.

Bug: chromium:1031479
Change-Id: I4c62486f6b0eb6d6f9c96f98c1c1b275f3e6d6d5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1962850
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65431}

src/runtime/runtime-compiler.cc

@@ -188,6 +188,19 @@ static bool IsSuitableForOnStackReplacement(Isolate* isolate,
                                             Handle<JSFunction> function) {
   // Keep track of whether we've succeeded in optimizing.
   if (function->shared().optimization_disabled()) return false;
+  // TODO(chromium:1031479): When we flush the bytecode we also reset the
+  // feedback_cell that holds feedback vector / closure feedback cell array.
+  // Closure feedback cell array holds the feedback cells that should be used
+  // when creating closures from the given function. If we happen to execute the
+  // function again, we re-compile and re-intialize this array. So, the closures
+  // created by this function after the flush would have different feedback
+  // cells when compared to the corresponding closures created before the flush.
+  // Currently, OSR triggering mechanism is tied to the bytecode array. So, it
+  // is possible to mark one closure for OSR but optimize a different one.
+  // These two factors could cause us to OSR functions that don't have feedback
+  // vector. This is a temporary fix and we should fix one (or both) of the two
+  // causes.
+  if (!function->has_feedback_vector()) return false;
   // If we are trying to do OSR when there are already optimized
   // activations of the function, it means (a) the function is directly or
   // indirectly recursive and (b) an optimized invocation has been

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

+// Copyright 2015 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: --interrupt-budget=200 --stack-size=200 --budget-for-feedback-vector-allocation=100 --expose-gc --stress-flush-bytecode
+
+var i = 0;
+function main() {
+function v0() {
+   function v10(a) {
+        i++;
+        var cur_i = i;
+        try {
+            // This triggers the use of old closure that was installed in the
+            // earlier invocation of v10 and causes an infinite recursion. At
+            // some point we throw from here.
+            [].e = 1;
+
+            // Throw when the new closure is on the stack to trigger a OSR on
+            // the new closure
+            if (cur_i == 2) throw 1;
+        } catch(v22) {
+            // This loop triggers OSR.
+            for (const v24 in "c19rXGEC2E") {
+            }
+        }
+   }
+   const v25 = v10(1);
+   // We install v10's closure here. The bytecode for v10 gets flushed on gc()
+   const v21 = Object.defineProperty([].__proto__,"e",{set:v10});
+}
+const v26 = v0();
+// With --stress-flush-bytecode GC flushes the bytecode for v0 and v10
+gc();
+assertThrows(v0, TypeError);
+}
+main();