From 7d3826ea480f8e15589d193cd17cbc7b072fe45c Mon Sep 17 00:00:00 2001
From: Ross McIlroy <rmcilroy@chromium.org>
Date: Wed, 5 Dec 2018 17:31:16 +0000
Subject: [PATCH] [Compile] Update JSFunction predicates to handle bytecode
 flushing.

Update is_compiled() and has_feedback_vector() to return false in the case
where the SFI's bytecode has been flushed, but the JSFunction hasn't yet been
reset to uncompiled. Also add code to reset the JSFunction when it is recompiled
lazily.

BUG=v8:8394

Change-Id: I7c5f79066603ac1ae097a0a62c625b1a8e39431c
Reviewed-on: https://chromium-review.googlesource.com/c/1363138
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58148}
---
 src/api.cc                                   |  4 +--
 src/compiler.cc                              | 11 +++++--
 src/compiler/pipeline.cc                     |  2 +-
 src/debug/liveedit.cc                        |  6 ++--
 src/heap/factory.cc                          |  4 +--
 src/objects-debug.cc                         |  4 +--
 src/objects.cc                               | 12 ++++---
 src/objects/js-objects-inl.h                 | 34 ++++++++++++++------
 src/objects/js-objects.h                     | 13 +++++---
 src/profiler/heap-snapshot-generator.cc      |  4 +--
 test/cctest/interpreter/interpreter-tester.h |  2 +-
 test/cctest/test-code-stub-assembler.cc      |  2 +-
 12 files changed, 64 insertions(+), 34 deletions(-)

diff --git a/src/api.cc b/src/api.cc
index 8d6f9c167a..06d912974b 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -800,8 +800,8 @@ StartupData SnapshotCreator::CreateBlob(
       fun->CompleteInobjectSlackTrackingIfActive();
 
       // Also, clear out feedback vectors, or any optimized code.
-      if (fun->has_feedback_vector()) {
-        fun->feedback_cell()->set_value(
+      if (!fun->raw_feedback_cell()->value()->IsUndefined()) {
+        fun->raw_feedback_cell()->set_value(
             i::ReadOnlyRoots(isolate).undefined_value());
         fun->set_code(isolate->builtins()->builtin(i::Builtins::kCompileLazy));
       }
diff --git a/src/compiler.cc b/src/compiler.cc
index 62fb393895..70d18fed09 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -607,7 +607,7 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Code> GetCodeFromOptimizedCodeCache(
   Handle<SharedFunctionInfo> shared(function->shared(), function->GetIsolate());
   DisallowHeapAllocation no_gc;
   if (osr_offset.IsNone()) {
-    if (function->feedback_cell()->value()->IsFeedbackVector()) {
+    if (function->has_feedback_vector()) {
       FeedbackVector feedback_vector = function->feedback_vector();
       feedback_vector->EvictOptimizedCodeMarkedForDeoptimization(
           function->shared(), "GetCodeFromOptimizedCodeCache");
@@ -1196,6 +1196,10 @@ bool Compiler::Compile(Handle<JSFunction> function, ClearExceptionFlag flag,
   DCHECK(!function->HasOptimizationMarker());
   DCHECK(!function->HasOptimizedCode());
 
+  // Reset the JSFunction if we are recompiling due to the bytecode having been
+  // flushed.
+  function->ResetIfBytecodeFlushed();
+
   Isolate* isolate = function->GetIsolate();
   Handle<SharedFunctionInfo> shared_info = handle(function->shared(), isolate);
 
@@ -1406,7 +1410,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
       JSFunction::EnsureFeedbackVector(result);
       if (allow_eval_cache) {
         // Make sure to cache this result.
-        Handle<FeedbackCell> new_feedback_cell(result->feedback_cell(),
+        Handle<FeedbackCell> new_feedback_cell(result->raw_feedback_cell(),
                                                isolate);
         compilation_cache->PutEval(source, outer_info, context, shared_info,
                                    new_feedback_cell, eval_scope_position);
@@ -1419,7 +1423,8 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     if (allow_eval_cache) {
       // Add the SharedFunctionInfo and the LiteralsArray to the eval cache if
       // we didn't retrieve from there.
-      Handle<FeedbackCell> new_feedback_cell(result->feedback_cell(), isolate);
+      Handle<FeedbackCell> new_feedback_cell(result->raw_feedback_cell(),
+                                             isolate);
       compilation_cache->PutEval(source, outer_info, context, shared_info,
                                  new_feedback_cell, eval_scope_position);
     }
diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
index f2cf7350e8..f563cb82fd 100644
--- a/src/compiler/pipeline.cc
+++ b/src/compiler/pipeline.cc
@@ -913,7 +913,7 @@ PipelineCompilationJob::Status PipelineCompilationJob::PrepareJobImpl(
     compilation_info()->MarkAsAllocationFoldingEnabled();
   }
 
-  if (compilation_info()->closure()->feedback_cell()->map() ==
+  if (compilation_info()->closure()->raw_feedback_cell()->map() ==
       ReadOnlyRoots(isolate).one_closure_cell_map()) {
     compilation_info()->MarkAsFunctionContextSpecializing();
   }
diff --git a/src/debug/liveedit.cc b/src/debug/liveedit.cc
index 866bdd8796..466c176842 100644
--- a/src/debug/liveedit.cc
+++ b/src/debug/liveedit.cc
@@ -1133,7 +1133,8 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
     }
 
     for (auto& js_function : data->js_functions) {
-      js_function->set_feedback_cell(*isolate->factory()->many_closures_cell());
+      js_function->set_raw_feedback_cell(
+          *isolate->factory()->many_closures_cell());
       if (!js_function->is_compiled()) continue;
       JSFunction::EnsureFeedbackVector(js_function);
     }
@@ -1173,7 +1174,8 @@ void LiveEdit::PatchScript(Isolate* isolate, Handle<Script> script,
       js_function->set_shared(*new_sfi);
       js_function->set_code(js_function->shared()->GetCode());
 
-      js_function->set_feedback_cell(*isolate->factory()->many_closures_cell());
+      js_function->set_raw_feedback_cell(
+          *isolate->factory()->many_closures_cell());
       if (!js_function->is_compiled()) continue;
       JSFunction::EnsureFeedbackVector(js_function);
     }
diff --git a/src/heap/factory.cc b/src/heap/factory.cc
index f2f8606acc..ab70869e51 100644
--- a/src/heap/factory.cc
+++ b/src/heap/factory.cc
@@ -2410,7 +2410,7 @@ Handle<JSFunction> Factory::NewFunction(Handle<Map> map,
   function->set_shared(*info);
   function->set_code(info->GetCode());
   function->set_context(*context);
-  function->set_feedback_cell(*many_closures_cell());
+  function->set_raw_feedback_cell(*many_closures_cell());
   int header_size;
   if (map->has_prototype_slot()) {
     header_size = JSFunction::kSizeWithPrototype;
@@ -2604,7 +2604,7 @@ Handle<JSFunction> Factory::NewFunctionFromSharedFunctionInfo(
         ->EvictOptimizedCodeMarkedForDeoptimization(
             *info, "new function from shared function info");
   }
-  result->set_feedback_cell(*feedback_cell);
+  result->set_raw_feedback_cell(*feedback_cell);
 
   // Give compiler a chance to pre-initialize.
   Compiler::PostInstantiation(result, pretenure);
diff --git a/src/objects-debug.cc b/src/objects-debug.cc
index 2b22315ca4..a6414b7dd9 100644
--- a/src/objects-debug.cc
+++ b/src/objects-debug.cc
@@ -1046,8 +1046,8 @@ void JSBoundFunction::JSBoundFunctionVerify(Isolate* isolate) {
 void JSFunction::JSFunctionVerify(Isolate* isolate) {
   CHECK(IsJSFunction());
   JSObjectVerify(isolate);
-  VerifyHeapPointer(isolate, feedback_cell());
-  CHECK(feedback_cell()->IsFeedbackCell());
+  VerifyHeapPointer(isolate, raw_feedback_cell());
+  CHECK(raw_feedback_cell()->IsFeedbackCell());
   CHECK(code()->IsCode());
   CHECK(map()->is_callable());
   Handle<JSFunction> function(*this, isolate);
diff --git a/src/objects.cc b/src/objects.cc
index bd79b0651f..0fd7d98ea1 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -12685,18 +12685,19 @@ void JSFunction::MarkForOptimization(ConcurrencyMode mode) {
 void JSFunction::EnsureFeedbackVector(Handle<JSFunction> function) {
   Isolate* const isolate = function->GetIsolate();
   DCHECK(function->shared()->is_compiled());
-  if (function->feedback_cell()->value()->IsUndefined(isolate)) {
+  if (!function->has_feedback_vector()) {
     Handle<SharedFunctionInfo> shared(function->shared(), isolate);
     if (!shared->HasAsmWasmData()) {
       DCHECK(function->shared()->HasBytecodeArray());
       Handle<FeedbackVector> feedback_vector =
           FeedbackVector::New(isolate, shared);
-      if (function->feedback_cell() == isolate->heap()->many_closures_cell()) {
+      if (function->raw_feedback_cell() ==
+          isolate->heap()->many_closures_cell()) {
         Handle<FeedbackCell> feedback_cell =
             isolate->factory()->NewOneClosureCell(feedback_vector);
-        function->set_feedback_cell(*feedback_cell);
+        function->set_raw_feedback_cell(*feedback_cell);
       } else {
-        function->feedback_cell()->set_value(*feedback_vector);
+        function->raw_feedback_cell()->set_value(*feedback_vector);
       }
     }
   }
@@ -14675,7 +14676,8 @@ int AbstractCode::SourceStatementPosition(int offset) {
 }
 
 void JSFunction::ClearTypeFeedbackInfo() {
-  if (feedback_cell()->value()->IsFeedbackVector()) {
+  ResetIfBytecodeFlushed();
+  if (has_feedback_vector()) {
     FeedbackVector vector = feedback_vector();
     Isolate* isolate = GetIsolate();
     if (vector->ClearSlots(isolate)) {
diff --git a/src/objects/js-objects-inl.h b/src/objects/js-objects-inl.h
index 01d7bf290f..f17694b2ef 100644
--- a/src/objects/js-objects-inl.h
+++ b/src/objects/js-objects-inl.h
@@ -441,7 +441,7 @@ ACCESSORS(JSBoundFunction, bound_this, Object, kBoundThisOffset)
 ACCESSORS2(JSBoundFunction, bound_arguments, FixedArray, kBoundArgumentsOffset)
 
 ACCESSORS2(JSFunction, shared, SharedFunctionInfo, kSharedFunctionInfoOffset)
-ACCESSORS(JSFunction, feedback_cell, FeedbackCell, kFeedbackCellOffset)
+ACCESSORS(JSFunction, raw_feedback_cell, FeedbackCell, kFeedbackCellOffset)
 
 ACCESSORS2(JSGlobalObject, native_context, Context, kNativeContextOffset)
 ACCESSORS2(JSGlobalObject, global_proxy, JSObject, kGlobalProxyOffset)
@@ -450,7 +450,7 @@ ACCESSORS(JSGlobalProxy, native_context, Object, kNativeContextOffset)
 
 FeedbackVector JSFunction::feedback_vector() const {
   DCHECK(has_feedback_vector());
-  return FeedbackVector::cast(feedback_cell()->value());
+  return FeedbackVector::cast(raw_feedback_cell()->value());
 }
 
 // Code objects that are marked for deoptimization are not considered to be
@@ -460,7 +460,7 @@ FeedbackVector JSFunction::feedback_vector() const {
 // TODO(jupvfranco): rename this function. Maybe RunOptimizedCode,
 // or IsValidOptimizedCode.
 bool JSFunction::IsOptimized() {
-  return code()->kind() == Code::OPTIMIZED_FUNCTION &&
+  return is_compiled() && code()->kind() == Code::OPTIMIZED_FUNCTION &&
          !code()->marked_for_deoptimization();
 }
 
@@ -482,9 +482,9 @@ void JSFunction::ClearOptimizationMarker() {
 // Optimized code marked for deoptimization will tier back down to running
 // interpreted on its next activation, and already doesn't count as IsOptimized.
 bool JSFunction::IsInterpreted() {
-  return code()->is_interpreter_trampoline_builtin() ||
-         (code()->kind() == Code::OPTIMIZED_FUNCTION &&
-          code()->marked_for_deoptimization());
+  return is_compiled() && (code()->is_interpreter_trampoline_builtin() ||
+                           (code()->kind() == Code::OPTIMIZED_FUNCTION &&
+                            code()->marked_for_deoptimization()));
 }
 
 bool JSFunction::ChecksOptimizationMarker() {
@@ -558,7 +558,8 @@ void JSFunction::SetOptimizationMarker(OptimizationMarker marker) {
 }
 
 bool JSFunction::has_feedback_vector() const {
-  return !feedback_cell()->value()->IsUndefined();
+  return shared()->is_compiled() &&
+         !raw_feedback_cell()->value()->IsUndefined();
 }
 
 Context JSFunction::context() {
@@ -634,8 +635,23 @@ Object* JSFunction::prototype() {
   return instance_prototype();
 }
 
-bool JSFunction::is_compiled() {
-  return code()->builtin_index() != Builtins::kCompileLazy;
+bool JSFunction::is_compiled() const {
+  return code()->builtin_index() != Builtins::kCompileLazy &&
+         shared()->is_compiled();
+}
+
+void JSFunction::ResetIfBytecodeFlushed() {
+  if (!shared()->is_compiled()) {
+    // Bytecode was flushed and function is now uncompiled, reset JSFunction
+    // by setting code to CompileLazy and clearing the feedback vector.
+    if (code()->builtin_index() != Builtins::kCompileLazy) {
+      set_code(GetIsolate()->builtins()->builtin(i::Builtins::kCompileLazy));
+    }
+    if (raw_feedback_cell()->value()->IsFeedbackVector()) {
+      raw_feedback_cell()->set_value(
+          ReadOnlyRoots(GetIsolate()).undefined_value());
+    }
+  }
 }
 
 ACCESSORS(JSValue, value, Object, kValueOffset)
diff --git a/src/objects/js-objects.h b/src/objects/js-objects.h
index 635ed825c0..c3eeebf92b 100644
--- a/src/objects/js-objects.h
+++ b/src/objects/js-objects.h
@@ -1022,9 +1022,11 @@ class JSFunction : public JSObject {
   // Completes inobject slack tracking on initial map if it is active.
   inline void CompleteInobjectSlackTrackingIfActive();
 
-  // [feedback_cell]: The FeedbackCell used to hold the FeedbackVector
-  // eventually.
-  DECL_ACCESSORS(feedback_cell, FeedbackCell)
+  // [raw_feedback_cell]: Gives raw access to the FeedbackCell used to hold the
+  /// FeedbackVector eventually. Generally this shouldn't be used to get the
+  // feedback_vector, instead use feedback_vector() which correctly deals with
+  // the JSFunction's bytecode being flushed.
+  DECL_ACCESSORS(raw_feedback_cell, FeedbackCell)
 
   // feedback_vector() can be used once the function is compiled.
   inline FeedbackVector feedback_vector() const;
@@ -1034,6 +1036,9 @@ class JSFunction : public JSObject {
   // Unconditionally clear the type feedback vector.
   void ClearTypeFeedbackInfo();
 
+  // Resets function to clear compiled data after bytecode has been flushed.
+  inline void ResetIfBytecodeFlushed();
+
   inline bool has_prototype_slot() const;
 
   // The initial map for an object created by this constructor.
@@ -1063,7 +1068,7 @@ class JSFunction : public JSObject {
   static void SetPrototype(Handle<JSFunction> function, Handle<Object> value);
 
   // Returns if this function has been compiled to native code yet.
-  inline bool is_compiled();
+  inline bool is_compiled() const;
 
   static int GetHeaderSize(bool function_has_prototype_slot) {
     return function_has_prototype_slot ? JSFunction::kSizeWithPrototype
diff --git a/src/profiler/heap-snapshot-generator.cc b/src/profiler/heap-snapshot-generator.cc
index ee5c71ebf9..a32e510bff 100644
--- a/src/profiler/heap-snapshot-generator.cc
+++ b/src/profiler/heap-snapshot-generator.cc
@@ -836,8 +836,8 @@ void V8HeapExplorer::ExtractJSObjectReferences(HeapEntry* entry,
       }
     }
     SharedFunctionInfo shared_info = js_fun->shared();
-    TagObject(js_fun->feedback_cell(), "(function feedback cell)");
-    SetInternalReference(entry, "feedback_cell", js_fun->feedback_cell(),
+    TagObject(js_fun->raw_feedback_cell(), "(function feedback cell)");
+    SetInternalReference(entry, "feedback_cell", js_fun->raw_feedback_cell(),
                          JSFunction::kFeedbackCellOffset);
     TagObject(shared_info, "(shared function info)");
     SetInternalReference(entry, "shared", shared_info,
diff --git a/test/cctest/interpreter/interpreter-tester.h b/test/cctest/interpreter/interpreter-tester.h
index 909489b79e..ad0f9b9413 100644
--- a/test/cctest/interpreter/interpreter-tester.h
+++ b/test/cctest/interpreter/interpreter-tester.h
@@ -122,7 +122,7 @@ class InterpreterTester {
       function->shared()->set_function_data(*bytecode_.ToHandleChecked());
     }
     if (!feedback_metadata_.is_null()) {
-      function->set_feedback_cell(isolate_->heap()->many_closures_cell());
+      function->set_raw_feedback_cell(isolate_->heap()->many_closures_cell());
       // Set the raw feedback metadata to circumvent checks that we are not
       // overwriting existing metadata.
       function->shared()->set_raw_outer_scope_info_or_feedback_metadata(
diff --git a/test/cctest/test-code-stub-assembler.cc b/test/cctest/test-code-stub-assembler.cc
index e501ae1e6f..61d85c931c 100644
--- a/test/cctest/test-code-stub-assembler.cc
+++ b/test/cctest/test-code-stub-assembler.cc
@@ -2483,7 +2483,7 @@ TEST(AllocateFunctionWithMapAndContext) {
   CHECK_EQ(ReadOnlyRoots(isolate).empty_property_array(),
            fun->property_array());
   CHECK_EQ(ReadOnlyRoots(isolate).empty_fixed_array(), fun->elements());
-  CHECK_EQ(isolate->heap()->many_closures_cell(), fun->feedback_cell());
+  CHECK_EQ(isolate->heap()->many_closures_cell(), fun->raw_feedback_cell());
   CHECK(!fun->has_prototype_slot());
   CHECK_EQ(*isolate->promise_capability_default_resolve_shared_fun(),
            fun->shared());