[wasm] Fix interrupt of empty loop

This includes two fixes:
1. For dynamic tiering, the budget must always be reduced when jumping
   backwards, otherwise we might never trigger tier up, which makes the
   loop non-interruptible (because the tier-up check replaces the stack
   check).
2. The d8 worker implementation also needs to terminate the isolate via
   an interrupt, in addition to scheduling a task, because the worker
   might never return to the event queue.

This CL also fixes one of the failure modes of the inspector fuzzer
(see https://crbug.com/1180018).

R=jkummerow@chromium.org, marja@chromium.org

Bug: v8:12767, chromium:1180018

Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Change-Id: Ia01d1725fc14931d2ea54c4769c4ee93f866ed63
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3568470Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79773}

src/d8/d8.cc

@@ -3280,7 +3280,8 @@ Local<Context> Shell::CreateEvaluationContext(Isolate* isolate) {
   Local<ObjectTemplate> global_template = CreateGlobalTemplate(isolate);
   EscapableHandleScope handle_scope(isolate);
   Local<Context> context = Context::New(isolate, nullptr, global_template);
-  DCHECK(!context.IsEmpty());
+  DCHECK_IMPLIES(context.IsEmpty(), isolate->IsExecutionTerminating());
+  if (context.IsEmpty()) return {};
   if (i::FLAG_perf_prof_annotate_wasm || i::FLAG_vtune_prof_annotate_wasm) {
     isolate->SetWasmLoadSourceMapCallback(Shell::WasmLoadSourceMapCallback);
   }
@@ -4180,6 +4181,10 @@ void Worker::Terminate() {
   std::unique_ptr<v8::Task> task(
       new TerminateTask(task_manager_, shared_from_this()));
   task_runner_->PostTask(std::move(task));
+  // Also schedule an interrupt in case the worker is running code and never
+  // returning to the event queue. Since we checked the state before, and we are
+  // holding the {worker_mutex_}, it's safe to access the isolate.
+  isolate_->TerminateExecution();
 }
 
 void Worker::ProcessMessage(std::unique_ptr<SerializationData> data) {
@@ -4239,12 +4244,15 @@ void Worker::ExecuteInThread() {
 
   D8Console console(isolate_);
   Shell::Initialize(isolate_, &console, false);
-  {
+  // This is not really a loop, but the loop allows us to break out of this
+  // block easily.
+  for (bool execute = true; execute; execute = false) {
     Isolate::Scope iscope(isolate_);
     {
       HandleScope scope(isolate_);
       PerIsolateData data(isolate_);
       Local<Context> context = Shell::CreateEvaluationContext(isolate_);
+      if (context.IsEmpty()) break;
       context_.Reset(isolate_, context);
       {
         Context::Scope cscope(context);

src/d8/d8.h

@@ -259,8 +259,11 @@ class Worker : public std::enable_shared_from_this<Worker> {
   // need locking, but accessing the Worker's data member does.)
   base::Mutex worker_mutex_;
 
-  // Only accessed by the worker thread.
+  // The isolate should only be accessed by the worker itself, or when holding
+  // the worker_mutex_ and after checking the worker state.
   Isolate* isolate_ = nullptr;
+
+  // Only accessed by the worker thread.
   v8::Persistent<v8::Context> context_;
 };
 

src/wasm/baseline/liftoff-compiler.cc

@@ -728,6 +728,10 @@ class LiftoffCompiler {
 
   void TierupCheck(FullDecoder* decoder, WasmCodePosition position,
                    int budget_used, Register scratch_reg) {
+    // We should always decrement the budget, and we don't expect integer
+    // overflows in the budget calculation.
+    DCHECK_LE(1, budget_used);
+
     if (for_debugging_ != kNoDebugging) return;
     CODE_COMMENT("tierup check");
     // We never want to blow the entire budget at once.
@@ -2591,7 +2595,11 @@ class LiftoffCompiler {
       if (target->is_loop()) {
         DCHECK(target->label.get()->is_bound());
         int jump_distance = __ pc_offset() - target->label.get()->pos();
-        TierupCheck(decoder, decoder->position(), jump_distance, scratch_reg);
+        // For now we just add one as the cost for the tier up check. We might
+        // want to revisit this when tuning tiering budgets later.
+        const int kTierUpCheckCost = 1;
+        TierupCheck(decoder, decoder->position(),
+                    jump_distance + kTierUpCheckCost, scratch_reg);
       } else {
         // To estimate time spent in this function more accurately, we could
         // increment the tiering budget on forward jumps. However, we don't

test/mjsunit/wasm/worker-running-empty-loop-interruptible.js

+// Copyright 2022 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.
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+const builder = new WasmModuleBuilder();
+// void main() { for (;;) {} }
+builder.addFunction('main', kSig_v_v).addBody([
+  kExprLoop, kWasmVoid, kExprBr, 0, kExprEnd
+]).exportFunc();
+const module = builder.toModule();
+
+function workerCode() {
+  onmessage = function(module) {
+    print('[worker] Creating instance.');
+    let instance = new WebAssembly.Instance(module);
+    print('[worker] Reporting start.');
+    postMessage('start');
+    print('[worker] Running main.');
+    instance.exports.main();
+  };
+}
+
+print('[main] Starting worker.');
+const worker = new Worker(workerCode, {type: 'function'});
+print('[main] Sending module.');
+worker.postMessage(module);
+assertEquals('start', worker.getMessage());
+print('[main] Terminating worker and waiting for it to finish.');
+worker.terminateAndWait();
+print('[main] All done.');