[wasm][interpreter] Remove activations

Since the interpreter cannot call out to JS any more, there cannot be
more than one activation at a time. Hence remove the concept of
activations.

R=ahaas@chromium.org

Bug: v8:10389
Change-Id: Ifda5624e192464a1aed2943787bc6860d1917719
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2219942Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68118}

src/wasm/wasm-debug.cc

@@ -108,39 +108,6 @@ class InterpreterHandle {
   WasmInterpreter interpreter_;
   std::unordered_map<Address, uint32_t> activations_;
 
-  uint32_t StartActivation(Address frame_pointer) {
-    WasmInterpreter::Thread* thread = interpreter_.GetThread(0);
-    uint32_t activation_id = thread->StartActivation();
-    DCHECK_EQ(0, activations_.count(frame_pointer));
-    activations_.insert(std::make_pair(frame_pointer, activation_id));
-    return activation_id;
-  }
-
-  void FinishActivation(Address frame_pointer, uint32_t activation_id) {
-    WasmInterpreter::Thread* thread = interpreter_.GetThread(0);
-    thread->FinishActivation(activation_id);
-    DCHECK_EQ(1, activations_.count(frame_pointer));
-    activations_.erase(frame_pointer);
-  }
-
-  bool HasActivation(Address frame_pointer) {
-    return activations_.count(frame_pointer);
-  }
-
-  std::pair<uint32_t, uint32_t> GetActivationFrameRange(
-      WasmInterpreter::Thread* thread, Address frame_pointer) {
-    DCHECK_EQ(1, activations_.count(frame_pointer));
-    uint32_t activation_id = activations_.find(frame_pointer)->second;
-    uint32_t num_activations = static_cast<uint32_t>(activations_.size() - 1);
-    uint32_t frame_base = thread->ActivationFrameBase(activation_id);
-    uint32_t frame_limit = activation_id == num_activations
-                               ? thread->GetFrameCount()
-                               : thread->ActivationFrameBase(activation_id + 1);
-    DCHECK_LE(frame_base, frame_limit);
-    DCHECK_LE(frame_limit, thread->GetFrameCount());
-    return {frame_base, frame_limit};
-  }
-
   static ModuleWireBytes GetBytes(WasmDebugInfo debug_info) {
     // Return raw pointer into heap. The WasmInterpreter will make its own copy
     // of this data anyway, and there is no heap allocation in-between.
@@ -162,8 +129,7 @@ class InterpreterHandle {
   // Returns true if exited regularly, false if a trap/exception occurred and
   // was not handled inside this activation. In the latter case, a pending
   // exception will have been set on the isolate.
-  bool Execute(Handle<WasmInstanceObject> instance_object,
-               Address frame_pointer, uint32_t func_index,
+  bool Execute(Handle<WasmInstanceObject> instance_object, uint32_t func_index,
                Vector<WasmValue> argument_values,
                Vector<WasmValue> return_values) {
     DCHECK_GE(module()->functions.size(), func_index);
@@ -171,8 +137,6 @@ class InterpreterHandle {
     DCHECK_EQ(sig->parameter_count(), argument_values.size());
     DCHECK_EQ(sig->return_count(), return_values.size());
 
-    uint32_t activation_id = StartActivation(frame_pointer);
-
     WasmCodeRefScope code_ref_scope;
     WasmInterpreter::Thread* thread = interpreter_.GetThread(0);
     thread->InitFrame(&module()->functions[func_index],
@@ -204,12 +168,9 @@ class InterpreterHandle {
         }
         case WasmInterpreter::State::STOPPED:
           // An exception happened, and the current activation was unwound
-          // without hitting a local exception handler. All that remains to be
-          // done is finish the activation and let the exception propagate.
-          DCHECK_EQ(thread->ActivationFrameBase(activation_id),
-                    thread->GetFrameCount());
+          // without hitting a local exception handler. Let the exception
+          // propagate.
           DCHECK(isolate_->has_pending_exception());
-          FinishActivation(frame_pointer, activation_id);
           return false;
         // RUNNING should never occur here.
         case WasmInterpreter::State::RUNNING:
@@ -228,8 +189,6 @@ class InterpreterHandle {
       return_values[i] = thread->GetReturnValue(i);
     }
 
-    FinishActivation(frame_pointer, activation_id);
-
     return true;
   }
 

src/wasm/wasm-interpreter.cc

@@ -1104,12 +1104,6 @@ V8_INLINE bool has_nondeterminism<double>(double val) {
 
 // Responsible for executing code directly.
 class ThreadImpl {
-  struct Activation {
-    uint32_t fp;
-    sp_t sp;
-    Activation(uint32_t fp, sp_t sp) : fp(fp), sp(sp) {}
-  };
-
  public:
   // The {ReferenceStackScope} sets up the reference stack in the interpreter.
   // The handle to the reference stack has to be re-initialized everytime we
@@ -1146,8 +1140,7 @@ class ThreadImpl {
       : codemap_(codemap),
         isolate_(instance_object->GetIsolate()),
         instance_object_(instance_object),
-        frames_(zone),
-        activations_(zone) {}
+        frames_(zone) {}
 
   //==========================================================================
   // Implementation of public interface for WasmInterpreter::Thread.
@@ -1156,7 +1149,7 @@ class ThreadImpl {
   WasmInterpreter::State state() { return state_; }
 
   void InitFrame(const WasmFunction* function, WasmValue* args) {
-    DCHECK_EQ(current_activation().fp, frames_.size());
+    DCHECK(frames_.empty());
     InterpreterCode* code = codemap()->GetCode(function);
     size_t num_params = function->sig->parameter_count();
     EnsureStackSpace(num_params);
@@ -1177,9 +1170,8 @@ class ThreadImpl {
     }
     state_ = WasmInterpreter::RUNNING;
     Execute(frames_.back().code, frames_.back().pc, num_steps);
-    // If state_ is STOPPED, the current activation must be fully unwound.
-    DCHECK_IMPLIES(state_ == WasmInterpreter::STOPPED,
-                   current_activation().fp == frames_.size());
+    // If state_ is STOPPED, the stack must be fully unwound.
+    DCHECK_IMPLIES(state_ == WasmInterpreter::STOPPED, frames_.empty());
     return state_;
   }
 
@@ -1202,10 +1194,7 @@ class ThreadImpl {
   WasmValue GetReturnValue(uint32_t index) {
     if (state_ == WasmInterpreter::TRAPPED) return WasmValue(0xDEADBEEF);
     DCHECK_EQ(WasmInterpreter::FINISHED, state_);
-    Activation act = current_activation();
-    // Current activation must be finished.
-    DCHECK_EQ(act.fp, frames_.size());
-    return GetStackValue(act.sp + index);
+    return GetStackValue(index);
   }
 
   WasmValue GetStackValue(sp_t index) {
@@ -1226,39 +1215,6 @@ class ThreadImpl {
 
   Handle<Cell> reference_stack_cell() const { return reference_stack_cell_; }
 
-  uint32_t NumActivations() {
-    return static_cast<uint32_t>(activations_.size());
-  }
-
-  uint32_t StartActivation() {
-    TRACE("----- START ACTIVATION %zu -----\n", activations_.size());
-    // If you use activations, use them consistently:
-    DCHECK_IMPLIES(activations_.empty(), frames_.empty());
-    DCHECK_IMPLIES(activations_.empty(), StackHeight() == 0);
-    uint32_t activation_id = static_cast<uint32_t>(activations_.size());
-    activations_.emplace_back(static_cast<uint32_t>(frames_.size()),
-                              StackHeight());
-    state_ = WasmInterpreter::STOPPED;
-    return activation_id;
-  }
-
-  void FinishActivation(uint32_t id) {
-    TRACE("----- FINISH ACTIVATION %zu -----\n", activations_.size() - 1);
-    DCHECK_LT(0, activations_.size());
-    DCHECK_EQ(activations_.size() - 1, id);
-    // Stack height must match the start of this activation (otherwise unwind
-    // first).
-    DCHECK_EQ(activations_.back().fp, frames_.size());
-    DCHECK_LE(activations_.back().sp, StackHeight());
-    ResetStack(activations_.back().sp);
-    activations_.pop_back();
-  }
-
-  uint32_t ActivationFrameBase(uint32_t id) {
-    DCHECK_GT(activations_.size(), id);
-    return activations_[id].fp;
-  }
-
   WasmInterpreter::Thread::ExceptionHandlingResult RaiseException(
       Isolate* isolate, Handle<Object> exception) {
     DCHECK_EQ(WasmInterpreter::TRAPPED, state_);
@@ -1273,15 +1229,13 @@ class ThreadImpl {
 
  private:
   // Handle a thrown exception. Returns whether the exception was handled inside
-  // the current activation. Unwinds the interpreted stack accordingly.
+  // of wasm. Unwinds the interpreted stack accordingly.
   WasmInterpreter::Thread::ExceptionHandlingResult HandleException(
       Isolate* isolate) {
     DCHECK(isolate->has_pending_exception());
     bool catchable =
         isolate->is_catchable_by_wasm(isolate->pending_exception());
-    DCHECK_LT(0, activations_.size());
-    Activation& act = activations_.back();
-    while (frames_.size() > act.fp) {
+    while (!frames_.empty()) {
       Frame& frame = frames_.back();
       InterpreterCode* code = frame.code;
       if (catchable && code->side_table->HasEntryAt(frame.pc)) {
@@ -1299,8 +1253,8 @@ class ThreadImpl {
       frames_.pop_back();
     }
     TRACE("----- UNWIND -----\n");
-    DCHECK_EQ(act.fp, frames_.size());
-    DCHECK_EQ(act.sp, StackHeight());
+    DCHECK(frames_.empty());
+    DCHECK_EQ(sp_, stack_.get());
     state_ = WasmInterpreter::STOPPED;
     return WasmInterpreter::Thread::UNWOUND;
   }
@@ -1322,8 +1276,6 @@ class ThreadImpl {
   // kept in a separate on-heap reference stack to make the GC trace them.
   // TODO(wasm): Optimize simple stack operations (like "get_local",
   // "set_local", and "tee_local") so that they don't require a handle scope.
-  // TODO(wasm): Consider optimizing activations that use no reference
-  // values to avoid allocating the reference stack entirely.
   class StackValue {
    public:
     StackValue() = default;  // Only needed for resizing the stack.
@@ -1386,9 +1338,6 @@ class ThreadImpl {
   TrapReason trap_reason_ = kTrapCount;
   bool possible_nondeterminism_ = false;
   uint64_t num_interpreted_calls_ = 0;
-  // Store the stack height of each activation (for unwind and frame
-  // inspection).
-  ZoneVector<Activation> activations_;
 
   CodeMap* codemap() const { return codemap_; }
   const WasmModule* module() const { return codemap_->module(); }
@@ -1512,7 +1461,7 @@ class ThreadImpl {
     DCHECK_GT(frames_.size(), 0);
     spdiff_t sp_diff = static_cast<spdiff_t>(StackHeight() - frames_.back().sp);
     frames_.pop_back();
-    if (frames_.size() == current_activation().fp) {
+    if (frames_.empty()) {
       // A return from the last frame terminates the execution.
       state_ = WasmInterpreter::FINISHED;
       DoStackTransfer(sp_diff, arity);
@@ -1533,7 +1482,7 @@ class ThreadImpl {
   }
 
   // Returns true if the call was successful, false if the stack check failed
-  // and the current activation was fully unwound.
+  // and the stack was fully unwound.
   bool DoCall(Decoder* decoder, InterpreterCode* target, pc_t* pc,
               pc_t* limit) V8_WARN_UNUSED_RESULT {
     frames_.back().pc = *pc;
@@ -2802,10 +2751,9 @@ class ThreadImpl {
 
   // Check if our control stack (frames_) exceeds the limit. Trigger stack
   // overflow if it does, and unwinding the current frame.
-  // Returns true if execution can continue, false if the current activation was
-  // fully unwound.
-  // Do call this function immediately *after* pushing a new frame. The pc of
-  // the top frame will be reset to 0 if the stack check fails.
+  // Returns true if execution can continue, false if the stack was fully
+  // unwound. Do call this function immediately *after* pushing a new frame. The
+  // pc of the top frame will be reset to 0 if the stack check fails.
   bool DoStackCheck() V8_WARN_UNUSED_RESULT {
     // The goal of this stack check is not to prevent actual stack overflows,
     // but to simulate stack overflows during the execution of compiled code.
@@ -3863,10 +3811,6 @@ class ThreadImpl {
     DCHECK_EQ(WasmCode::kFunction, code->kind());
     return {CallResult::INTERNAL, codemap()->GetCode(code->index())};
   }
-
-  inline Activation current_activation() {
-    return activations_.empty() ? Activation(0, 0) : activations_.back();
-  }
 };
 
 namespace {
@@ -3936,24 +3880,6 @@ bool WasmInterpreter::Thread::PossibleNondeterminism() {
 uint64_t WasmInterpreter::Thread::NumInterpretedCalls() {
   return ToImpl(this)->NumInterpretedCalls();
 }
-uint32_t WasmInterpreter::Thread::NumActivations() {
-  return ToImpl(this)->NumActivations();
-}
-uint32_t WasmInterpreter::Thread::StartActivation() {
-  ThreadImpl* impl = ToImpl(this);
-  ThreadImpl::ReferenceStackScope stack_scope(impl);
-  return impl->StartActivation();
-}
-void WasmInterpreter::Thread::FinishActivation(uint32_t id) {
-  ThreadImpl* impl = ToImpl(this);
-  ThreadImpl::ReferenceStackScope stack_scope(impl);
-  impl->FinishActivation(id);
-}
-uint32_t WasmInterpreter::Thread::ActivationFrameBase(uint32_t id) {
-  ThreadImpl* impl = ToImpl(this);
-  ThreadImpl::ReferenceStackScope stack_scope(impl);
-  return impl->ActivationFrameBase(id);
-}
 
 //============================================================================
 // The implementation details of the interpreter.

src/wasm/wasm-interpreter.h

@@ -98,18 +98,6 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
 
     // Returns the number of calls / function frames executed on this thread.
     uint64_t NumInterpretedCalls();
-
-    // Each thread can have multiple activations, each represented by a portion
-    // of the stack frames of this thread. StartActivation returns the id
-    // (counting from 0 up) of the started activation.
-    // Activations must be properly stacked, i.e. if FinishActivation is called,
-    // the given id must the the latest activation on the stack.
-    uint32_t NumActivations();
-    uint32_t StartActivation();
-    void FinishActivation(uint32_t activation_id);
-    // Return the frame base of the given activation, i.e. the number of frames
-    // when this activation was started.
-    uint32_t ActivationFrameBase(uint32_t activation_id);
   };
 
   WasmInterpreter(Isolate* isolate, const WasmModule* module,

test/cctest/wasm/test-run-wasm-interpreter.cc

@@ -472,39 +472,6 @@ TEST(TestPossibleNondeterminism) {
   }
 }
 
-TEST(WasmInterpreterActivations) {
-  WasmRunner<void> r(ExecutionTier::kInterpreter);
-  Isolate* isolate = r.main_isolate();
-  BUILD(r, WASM_UNREACHABLE);
-
-  WasmInterpreter* interpreter = r.interpreter();
-  WasmInterpreter::Thread* thread = interpreter->GetThread(0);
-  CHECK_EQ(0, thread->NumActivations());
-  uint32_t act0 = thread->StartActivation();
-  CHECK_EQ(0, act0);
-  thread->InitFrame(r.function(), nullptr);
-  uint32_t act1 = thread->StartActivation();
-  CHECK_EQ(1, act1);
-  thread->InitFrame(r.function(), nullptr);
-  CHECK_EQ(2, thread->NumActivations());
-  CHECK_EQ(2, thread->GetFrameCount());
-  CHECK_EQ(WasmInterpreter::TRAPPED, thread->Run());
-  thread->RaiseException(isolate, handle(Smi::zero(), isolate));
-  CHECK_EQ(1, thread->GetFrameCount());
-  CHECK_EQ(2, thread->NumActivations());
-  thread->FinishActivation(act1);
-  isolate->clear_pending_exception();
-  CHECK_EQ(1, thread->GetFrameCount());
-  CHECK_EQ(1, thread->NumActivations());
-  CHECK_EQ(WasmInterpreter::TRAPPED, thread->Run());
-  thread->RaiseException(isolate, handle(Smi::zero(), isolate));
-  CHECK_EQ(0, thread->GetFrameCount());
-  CHECK_EQ(1, thread->NumActivations());
-  thread->FinishActivation(act0);
-  isolate->clear_pending_exception();
-  CHECK_EQ(0, thread->NumActivations());
-}
-
 TEST(InterpreterLoadWithoutMemory) {
   WasmRunner<int32_t, int32_t> r(ExecutionTier::kInterpreter);
   r.builder().AddMemory(0);

test/common/wasm/wasm-module-runner.cc

@@ -130,12 +130,6 @@ bool InterpretWasmModuleForTesting(Isolate* isolate,
   WasmInterpreter::Thread* thread = interpreter->GetThread(0);
   thread->Reset();
 
-  // Start an activation so that we can deal with stack overflows. We do not
-  // finish the activation. An activation is just part of the state of the
-  // interpreter, and we do not reuse the interpreter anyways. In addition,
-  // finishing the activation is not correct in all cases, e.g. when the
-  // execution of the interpreter did not finish after kMaxNumSteps.
-  thread->StartActivation();
   thread->InitFrame(&instance->module()->functions[function_index],
                     arguments.get());
   WasmInterpreter::State interpreter_result = thread->Run(kMaxNumSteps);
@@ -208,12 +202,6 @@ WasmInterpretationResult InterpretWasmModule(
   WasmInterpreter::Thread* thread = interpreter->GetThread(0);
   thread->Reset();
 
-  // Start an activation so that we can deal with stack overflows. We do not
-  // finish the activation. An activation is just part of the state of the
-  // interpreter, and we do not reuse the interpreter anyways. In addition,
-  // finishing the activation is not correct in all cases, e.g. when the
-  // execution of the interpreter did not finish after kMaxNumSteps.
-  thread->StartActivation();
   thread->InitFrame(&(instance->module()->functions[function_index]), args);
   WasmInterpreter::State interpreter_result = thread->Run(kMaxNumSteps);