Deoptimization and multithreading.

When using Lockers and Unlockers it is possible to create a
scenario where multiple threads point to the same optimized
code object. When that happens, if one of the threads triggers
deoptimization, then the stack replacement needs to happen in
the stacks of all threads.
With this CL, the deoptimizer visits all threads to do so.
The CL also adds three tests where V8 used to crash due to this
issue.

Bug: v8:6563
Change-Id: I74e9af472d4833aa8d13e579df45133791f6a503
Reviewed-on: https://chromium-review.googlesource.com/670783Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Juliana Patricia Vicente Franco <jupvfranco@google.com>
Cr-Commit-Position: refs/heads/master@{#48060}

src/deoptimizer.cc

@@ -144,15 +144,59 @@ void Deoptimizer::GenerateDeoptimizationEntries(MacroAssembler* masm,
   generator.Generate();
 }
 
+namespace {
+class ActivationsFinder : public ThreadVisitor {
+ public:
+  explicit ActivationsFinder(std::set<Code*>* codes,
+                             Code* topmost_optimized_code,
+                             bool safe_to_deopt_topmost_optimized_code)
+      : codes_(codes) {
+#ifdef DEBUG
+    topmost_ = topmost_optimized_code;
+    safe_to_deopt_ = safe_to_deopt_topmost_optimized_code;
+#endif
+  }
+
+  // Find the frames with activations of codes marked for deoptimization, search
+  // for the trampoline to the deoptimizer call respective to each code, and use
+  // it to replace the current pc on the stack.
+  void VisitThread(Isolate* isolate, ThreadLocalTop* top) {
+    for (StackFrameIterator it(isolate, top); !it.done(); it.Advance()) {
+      if (it.frame()->type() == StackFrame::OPTIMIZED) {
+        Code* code = it.frame()->LookupCode();
+        if (code->kind() == Code::OPTIMIZED_FUNCTION &&
+            code->marked_for_deoptimization()) {
+          codes_->erase(code);
+          // Obtain the trampoline to the deoptimizer call.
+          SafepointEntry safepoint = code->GetSafepointEntry(it.frame()->pc());
+          int trampoline_pc = safepoint.trampoline_pc();
+          DCHECK_IMPLIES(code == topmost_, safe_to_deopt_);
+          // Replace the current pc on the stack with the trampoline.
+          it.frame()->set_pc(code->instruction_start() + trampoline_pc);
+        }
+      }
+    }
+  }
+
+ private:
+  std::set<Code*>* codes_;
+
+#ifdef DEBUG
+  Code* topmost_;
+  bool safe_to_deopt_;
+#endif
+};
+}  // namespace
+
 // Move marked code from the optimized code list to the deoptimized code list,
 // and replace pc on the stack for codes marked for deoptimization.
 void Deoptimizer::DeoptimizeMarkedCodeForContext(Context* context) {
   DisallowHeapAllocation no_allocation;
 
   Isolate* isolate = context->GetHeap()->isolate();
-#ifdef DEBUG
   Code* topmost_optimized_code = NULL;
   bool safe_to_deopt_topmost_optimized_code = false;
+#ifdef DEBUG
   // Make sure all activations of optimized code can deopt at their current PC.
   // The topmost optimized code has special handling because it cannot be
   // deoptimized due to weak object dependency.
@@ -227,28 +271,16 @@ void Deoptimizer::DeoptimizeMarkedCodeForContext(Context* context) {
     element = next;
   }
 
-  // Find the frames with activations of codes marked for deoptimization, search
-  // for the trampoline to the deoptimizer call respective to each code, and use
-  // it to replace the current pc on the stack.
-  for (StackFrameIterator it(isolate, isolate->thread_local_top()); !it.done();
-       it.Advance()) {
-    if (it.frame()->type() == StackFrame::OPTIMIZED) {
-      Code* code = it.frame()->LookupCode();
-      if (code->kind() == Code::OPTIMIZED_FUNCTION &&
-          code->marked_for_deoptimization()) {
-        codes.erase(code);
-        // Obtain the trampoline to the deoptimizer call.
-        SafepointEntry safepoint = code->GetSafepointEntry(it.frame()->pc());
-        int trampoline_pc = safepoint.trampoline_pc();
-        DCHECK_IMPLIES(code == topmost_optimized_code,
-                       safe_to_deopt_topmost_optimized_code);
-        // Replace the current pc on the stack with the trampoline.
-        it.frame()->set_pc(code->instruction_start() + trampoline_pc);
-      }
-    }
-  }
+  ActivationsFinder visitor(&codes, topmost_optimized_code,
+                            safe_to_deopt_topmost_optimized_code);
+  // Iterate over the stack of this thread.
+  visitor.VisitThread(isolate, isolate->thread_local_top());
+  // In addition to iterate over the stack of this thread, we also
+  // need to consider all the other threads as they may also use
+  // the code currently beings deoptimized.
+  isolate->thread_manager()->IterateArchivedThreads(&visitor);
 
-  // If there's no activation of a code in the stack then we can remove its
+  // If there's no activation of a code in any stack then we can remove its
   // deoptimization data. We do this to ensure that Code objects that will be
   // unlinked won't be kept alive.
   std::set<Code*>::iterator it;

src/runtime/runtime-compiler.cc

@@ -137,28 +137,6 @@ RUNTIME_FUNCTION(Runtime_NotifyStubFailure) {
   return isolate->heap()->undefined_value();
 }
 
-class ActivationsFinder : public ThreadVisitor {
- public:
-  Code* code_;
-  bool has_code_activations_;
-
-  explicit ActivationsFinder(Code* code)
-      : code_(code), has_code_activations_(false) {}
-
-  void VisitThread(Isolate* isolate, ThreadLocalTop* top) {
-    JavaScriptFrameIterator it(isolate, top);
-    VisitFrames(&it);
-  }
-
-  void VisitFrames(JavaScriptFrameIterator* it) {
-    for (; !it->done(); it->Advance()) {
-      JavaScriptFrame* frame = it->frame();
-      if (code_->contains(frame->pc())) has_code_activations_ = true;
-    }
-  }
-};
-
-
 RUNTIME_FUNCTION(Runtime_NotifyDeoptimized) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());

test/cctest/test-lockers.cc

@@ -54,6 +54,243 @@ using ::v8::String;
 using ::v8::Value;
 using ::v8::V8;
 
+namespace {
+
+class DeoptimizeCodeThread : public v8::base::Thread {
+ public:
+  DeoptimizeCodeThread(v8::Isolate* isolate, v8::Local<v8::Context> context,
+                       const char* trigger)
+      : Thread(Options("DeoptimizeCodeThread")),
+        isolate_(isolate),
+        context_(isolate, context),
+        source_(trigger) {}
+
+  void Run() {
+    v8::Locker locker(isolate_);
+    isolate_->Enter();
+    v8::HandleScope handle_scope(isolate_);
+    v8::Local<v8::Context> context =
+        v8::Local<v8::Context>::New(isolate_, context_);
+    v8::Context::Scope context_scope(context);
+    CHECK_EQ(isolate_, v8::Isolate::GetCurrent());
+    // This code triggers deoptimization of some function that will be
+    // used in a different thread.
+    CompileRun(source_);
+    isolate_->Exit();
+  }
+
+ private:
+  v8::Isolate* isolate_;
+  Persistent<v8::Context> context_;
+  // The code that triggers the deoptimization.
+  const char* source_;
+};
+
+void UnlockForDeoptimization(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  // Gets the pointer to the thread that will trigger the deoptimization of the
+  // code.
+  DeoptimizeCodeThread* deoptimizer =
+      reinterpret_cast<DeoptimizeCodeThread*>(isolate->GetData(0));
+  {
+    // Exits and unlocks the isolate.
+    isolate->Exit();
+    v8::Unlocker unlocker(isolate);
+    // Starts the deoptimizing thread.
+    deoptimizer->Start();
+    // Waits for deoptimization to finish.
+    deoptimizer->Join();
+  }
+  // The deoptimizing thread has finished its work, and the isolate
+  // will now be used by the current thread.
+  isolate->Enter();
+}
+
+void UnlockForDeoptimizationIfReady(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  bool* ready_to_deoptimize = reinterpret_cast<bool*>(isolate->GetData(1));
+  if (*ready_to_deoptimize) {
+    // The test should enter here only once, so put the flag back to false.
+    *ready_to_deoptimize = false;
+    // Gets the pointer to the thread that will trigger the deoptimization of
+    // the code.
+    DeoptimizeCodeThread* deoptimizer =
+        reinterpret_cast<DeoptimizeCodeThread*>(isolate->GetData(0));
+    {
+      // Exits and unlocks the thread.
+      isolate->Exit();
+      v8::Unlocker unlocker(isolate);
+      // Starts the thread that deoptimizes the function.
+      deoptimizer->Start();
+      // Waits for the deoptimizing thread to finish.
+      deoptimizer->Join();
+    }
+    // The deoptimizing thread has finished its work, and the isolate
+    // will now be used by the current thread.
+    isolate->Enter();
+  }
+}
+}  // namespace
+
+TEST(LazyDeoptimizationMultithread) {
+  i::FLAG_allow_natives_syntax = true;
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  v8::Isolate* isolate = v8::Isolate::New(create_params);
+  {
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolate_scope(isolate);
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> context = v8::Context::New(isolate);
+    const char* trigger_deopt = "obj = { y: 0, x: 1 };";
+
+    // We use the isolate to pass arguments to the UnlockForDeoptimization
+    // function. Namely, we pass a pointer to the deoptimizing thread.
+    DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
+    isolate->SetData(0, &deoptimize_thread);
+    v8::Context::Scope context_scope(context);
+
+    // Create the function templace for C++ code that is invoked from
+    // JavaScript code.
+    Local<v8::FunctionTemplate> fun_templ =
+        v8::FunctionTemplate::New(isolate, UnlockForDeoptimization);
+    Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
+    CHECK(context->Global()
+              ->Set(context, v8_str("unlock_for_deoptimization"), fun)
+              .FromJust());
+
+    // Optimizes a function f, which will be deoptimized in another
+    // thread.
+    CompileRun(
+        "var b = false; var obj = { x: 1 };"
+        "function f() { g(); return obj.x; }"
+        "function g() { if (b) { unlock_for_deoptimization(); } }"
+        "%NeverOptimizeFunction(g);"
+        "f(); f(); %OptimizeFunctionOnNextCall(f);"
+        "f();");
+
+    // Trigger the unlocking.
+    Local<Value> v = CompileRun("b = true; f();");
+
+    // Once the isolate has been unlocked, the thread will wait for the
+    // other thread to finish its task. Once this happens, this thread
+    // continues with its execution, that is, with the execution of the
+    // function g, which then returns to f. The function f should have
+    // also been deoptimized. If the replacement did not happen on this
+    // thread's stack, then the test will fail here.
+    CHECK(v->IsNumber());
+    CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
+  }
+  isolate->Dispose();
+}
+
+TEST(LazyDeoptimizationMultithreadWithNatives) {
+  i::FLAG_allow_natives_syntax = true;
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  v8::Isolate* isolate = v8::Isolate::New(create_params);
+  {
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolate_scope(isolate);
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> context = v8::Context::New(isolate);
+    const char* trigger_deopt = "%DeoptimizeFunction(f);";
+
+    // We use the isolate to pass arguments to the UnlockForDeoptimization
+    // function. Namely, we pass a pointer to the deoptimizing thread.
+    DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
+    isolate->SetData(0, &deoptimize_thread);
+    bool ready_to_deopt = false;
+    isolate->SetData(1, &ready_to_deopt);
+    v8::Context::Scope context_scope(context);
+
+    // Create the function templace for C++ code that is invoked from
+    // JavaScript code.
+    Local<v8::FunctionTemplate> fun_templ =
+        v8::FunctionTemplate::New(isolate, UnlockForDeoptimizationIfReady);
+    Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
+    CHECK(context->Global()
+              ->Set(context, v8_str("unlock_for_deoptimization"), fun)
+              .FromJust());
+
+    // Optimizes a function f, which will be deoptimized in another
+    // thread.
+    CompileRun(
+        "var obj = { x: 1 };"
+        "function f() { g(); return obj.x;}"
+        "function g() { "
+        "  unlock_for_deoptimization(); }"
+        "%NeverOptimizeFunction(g);"
+        "f(); f(); %OptimizeFunctionOnNextCall(f);");
+
+    // Trigger the unlocking.
+    ready_to_deopt = true;
+    isolate->SetData(1, &ready_to_deopt);
+    Local<Value> v = CompileRun("f();");
+
+    // Once the isolate has been unlocked, the thread will wait for the
+    // other thread to finish its task. Once this happens, this thread
+    // continues with its execution, that is, with the execution of the
+    // function g, which then returns to f. The function f should have
+    // also been deoptimized. Otherwise, the test will fail here.
+    CHECK(v->IsNumber());
+    CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
+  }
+  isolate->Dispose();
+}
+
+TEST(EagerDeoptimizationMultithread) {
+  i::FLAG_allow_natives_syntax = true;
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  v8::Isolate* isolate = v8::Isolate::New(create_params);
+  {
+    v8::Locker locker(isolate);
+    v8::Isolate::Scope isolate_scope(isolate);
+    v8::HandleScope scope(isolate);
+    v8::Local<v8::Context> context = v8::Context::New(isolate);
+    const char* trigger_deopt = "f({y: 0, x: 1});";
+
+    // We use the isolate to pass arguments to the UnlockForDeoptimization
+    // function. Namely, we pass a pointer to the deoptimizing thread.
+    DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
+    isolate->SetData(0, &deoptimize_thread);
+    bool ready_to_deopt = false;
+    isolate->SetData(1, &ready_to_deopt);
+    v8::Context::Scope context_scope(context);
+
+    // Create the function templace for C++ code that is invoked from
+    // JavaScript code.
+    Local<v8::FunctionTemplate> fun_templ =
+        v8::FunctionTemplate::New(isolate, UnlockForDeoptimizationIfReady);
+    Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
+    CHECK(context->Global()
+              ->Set(context, v8_str("unlock_for_deoptimization"), fun)
+              .FromJust());
+
+    // Optimizes a function f, which will be deoptimized by another thread.
+    CompileRun(
+        "function f(obj) { unlock_for_deoptimization(); return obj.x; }"
+        "f({x: 1}); f({x: 1});"
+        "%OptimizeFunctionOnNextCall(f);"
+        "f({x: 1});");
+
+    // Trigger the unlocking.
+    ready_to_deopt = true;
+    isolate->SetData(1, &ready_to_deopt);
+    Local<Value> v = CompileRun("f({x: 1});");
+
+    // Once the isolate has been unlocked, the thread will wait for the
+    // other thread to finish its task. Once this happens, this thread
+    // continues with its execution, that is, with the execution of the
+    // function g, which then returns to f. The function f should have
+    // also been deoptimized. Otherwise, the test will fail here.
+    CHECK(v->IsNumber());
+    CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
+  }
+  isolate->Dispose();
+}
 
 // Migrating an isolate
 class KangarooThread : public v8::base::Thread {