[wasm] Reduce test-specific code

This reduces the amount of special paths for testing.
Setup the memory used for testing exactly the same way as in real world.
Also, always connect the interpreter to the instance being executed,
and to the existing WasmInstance struct. This keeps information
synchronized between interpreter and test runner.
These changes allow us to execute e.g. GrowMemory from cctests either
in the interpreter or in compiled code.

R=ahaas@chromium.org

Change-Id: Id4726d061f3cdba789275350f500d769d27d2d63
Reviewed-on: https://chromium-review.googlesource.com/488561
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44966}

src/wasm/wasm-debug.cc

@@ -105,9 +105,12 @@ class InterpreterHandle {
  public:
   // Initialize in the right order, using helper methods to make this possible.
   // WasmInterpreter has to be allocated in place, since it is not movable.
-  InterpreterHandle(Isolate* isolate, WasmDebugInfo* debug_info)
+  InterpreterHandle(Isolate* isolate, WasmDebugInfo* debug_info,
+                    WasmInstance* external_instance = nullptr)
       : instance_(debug_info->wasm_instance()->compiled_module()->module()),
-        interpreter_(isolate, GetBytesEnv(&instance_, debug_info)),
+        interpreter_(isolate, GetBytesEnv(external_instance ? external_instance
+                                                            : &instance_,
+                                          debug_info)),
         isolate_(isolate) {
     DisallowHeapAllocation no_gc;
 
@@ -401,8 +404,10 @@ class InterpreterHandle {
   }
 
   void UpdateMemory(JSArrayBuffer* new_memory) {
-    instance_.mem_start = reinterpret_cast<byte*>(new_memory->backing_store());
-    CHECK(new_memory->byte_length()->ToUint32(&instance_.mem_size));
+    byte* mem_start = reinterpret_cast<byte*>(new_memory->backing_store());
+    uint32_t mem_size;
+    CHECK(new_memory->byte_length()->ToUint32(&mem_size));
+    interpreter()->UpdateMemory(mem_start, mem_size);
   }
 
   Handle<JSArray> GetScopeDetails(Address frame_pointer, int frame_index,
@@ -570,12 +575,25 @@ void RedirectCallsitesInInstance(Isolate* isolate, WasmInstanceObject* instance,
 }  // namespace
 
 Handle<WasmDebugInfo> WasmDebugInfo::New(Handle<WasmInstanceObject> instance) {
-  Isolate* isolate = instance->GetIsolate();
-  Factory* factory = isolate->factory();
+  DCHECK(!instance->has_debug_info());
+  Factory* factory = instance->GetIsolate()->factory();
   Handle<FixedArray> arr = factory->NewFixedArray(kFieldCount, TENURED);
   arr->set(kWrapperTracerHeader, Smi::kZero);
   arr->set(kInstance, *instance);
-  return Handle<WasmDebugInfo>::cast(arr);
+  Handle<WasmDebugInfo> debug_info = Handle<WasmDebugInfo>::cast(arr);
+  instance->set_debug_info(*debug_info);
+  return debug_info;
+}
+
+WasmInterpreter* WasmDebugInfo::SetupForTesting(
+    Handle<WasmInstanceObject> instance_obj, WasmInstance* instance) {
+  Handle<WasmDebugInfo> debug_info = WasmDebugInfo::New(instance_obj);
+  Isolate* isolate = instance_obj->GetIsolate();
+  InterpreterHandle* cpp_handle =
+      new InterpreterHandle(isolate, *debug_info, instance);
+  Handle<Object> handle = Managed<InterpreterHandle>::New(isolate, cpp_handle);
+  debug_info->set(kInterpreterHandle, *handle);
+  return cpp_handle->interpreter();
 }
 
 bool WasmDebugInfo::IsDebugInfo(Object* object) {

src/wasm/wasm-interpreter.cc

@@ -601,9 +601,6 @@ inline int32_t ExecuteGrowMemory(uint32_t delta_pages,
   DCHECK_EQ(0, instance->mem_size % WasmModule::kPageSize);
   uint32_t old_pages = instance->mem_size / WasmModule::kPageSize;
 
-  // If an instance is set, execute GrowMemory on the instance. This will also
-  // update the WasmInstance struct used here.
-  if (!instance_obj.is_null()) {
   Isolate* isolate = instance_obj.ToHandleChecked()->GetIsolate();
   int32_t ret = WasmInstanceObject::GrowMemory(
       isolate, instance_obj.ToHandleChecked(), delta_pages);
@@ -611,46 +608,8 @@ inline int32_t ExecuteGrowMemory(uint32_t delta_pages,
   DCHECK_EQ(ret == -1 ? old_pages : old_pages + delta_pages,
             instance->mem_size / WasmModule::kPageSize);
   DCHECK(ret == -1 || static_cast<uint32_t>(ret) == old_pages);
+  USE(old_pages);
   return ret;
-  }
-
-  // TODO(ahaas): Move memory allocation to wasm-module.cc for better
-  // encapsulation.
-  if (delta_pages > FLAG_wasm_max_mem_pages ||
-      delta_pages > instance->module->max_mem_pages) {
-    return -1;
-  }
-
-  uint32_t new_pages = old_pages + delta_pages;
-  if (new_pages > FLAG_wasm_max_mem_pages ||
-      new_pages > instance->module->max_mem_pages) {
-    return -1;
-  }
-
-  byte* new_mem_start;
-  if (instance->mem_size == 0) {
-    // TODO(gdeepti): Fix bounds check to take into account size of memtype.
-    new_mem_start = static_cast<byte*>(
-        calloc(new_pages * WasmModule::kPageSize, sizeof(byte)));
-    if (!new_mem_start) return -1;
-  } else {
-    DCHECK_NOT_NULL(instance->mem_start);
-    if (EnableGuardRegions()) {
-      v8::base::OS::Unprotect(instance->mem_start,
-                              new_pages * WasmModule::kPageSize);
-      new_mem_start = instance->mem_start;
-    } else {
-      new_mem_start = static_cast<byte*>(
-          realloc(instance->mem_start, new_pages * WasmModule::kPageSize));
-      if (!new_mem_start) return -1;
-    }
-    // Zero initializing uninitialized memory from realloc
-    memset(new_mem_start + old_pages * WasmModule::kPageSize, 0,
-           delta_pages * WasmModule::kPageSize);
-  }
-  instance->mem_start = new_mem_start;
-  instance->mem_size = new_pages * WasmModule::kPageSize;
-  return static_cast<int32_t>(old_pages);
 }
 
 enum InternalOpcode {
@@ -2233,7 +2192,8 @@ class ThreadImpl {
   ExternalCallResult CallIndirectFunction(uint32_t table_index,
                                           uint32_t entry_index,
                                           uint32_t sig_index) {
-    if (!codemap()->has_instance()) {
+    if (!codemap()->has_instance() ||
+        !codemap()->instance()->compiled_module()->has_function_tables()) {
       // No instance. Rely on the information stored in the WasmModule.
       // TODO(wasm): This is only needed for testing. Refactor testing to use
       // the same paths as production.
@@ -2557,6 +2517,11 @@ void WasmInterpreter::WriteMemory(size_t offset, WasmVal val) {
   UNIMPLEMENTED();
 }
 
+void WasmInterpreter::UpdateMemory(byte* mem_start, uint32_t mem_size) {
+  internals_->instance_->mem_start = mem_start;
+  internals_->instance_->mem_size = mem_size;
+}
+
 void WasmInterpreter::AddFunctionForTesting(const WasmFunction* function) {
   internals_->codemap_.AddFunction(function, nullptr, nullptr);
 }

src/wasm/wasm-interpreter.h

@@ -256,6 +256,8 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
   size_t GetMemorySize();
   WasmVal ReadMemory(size_t offset);
   void WriteMemory(size_t offset, WasmVal val);
+  // Update the memory region, e.g. after external GrowMemory.
+  void UpdateMemory(byte* mem_start, uint32_t mem_size);
 
   //==========================================================================
   // Testing functionality.

src/wasm/wasm-objects.cc

@@ -533,7 +533,7 @@ Handle<WasmDebugInfo> WasmInstanceObject::GetOrCreateDebugInfo(
     Handle<WasmInstanceObject> instance) {
   if (instance->has_debug_info()) return handle(instance->debug_info());
   Handle<WasmDebugInfo> new_info = WasmDebugInfo::New(instance);
-  instance->set_debug_info(*new_info);
+  DCHECK(instance->has_debug_info());
   return new_info;
 }
 

src/wasm/wasm-objects.h

@@ -16,6 +16,8 @@ namespace internal {
 namespace wasm {
 class InterpretedFrame;
 struct WasmModule;
+struct WasmInstance;
+class WasmInterpreter;
 }
 
 class WasmCompiledModule;
@@ -549,6 +551,13 @@ class WasmDebugInfo : public FixedArray {
 
   static Handle<WasmDebugInfo> New(Handle<WasmInstanceObject>);
 
+  // Setup a WasmDebugInfo with an existing WasmInstance struct.
+  // Returns a pointer to the interpreter instantiated inside this
+  // WasmDebugInfo.
+  // Use for testing only.
+  static wasm::WasmInterpreter* SetupForTesting(Handle<WasmInstanceObject>,
+                                                wasm::WasmInstance*);
+
   static bool IsDebugInfo(Object*);
   static WasmDebugInfo* cast(Object*);
 
@@ -660,8 +669,11 @@ class WasmInstanceWrapper : public FixedArray {
   };
 };
 
+#undef DECLARE_CASTS
+#undef DECLARE_GETTER
 #undef DECLARE_ACCESSORS
 #undef DECLARE_OPTIONAL_ACCESSORS
+#undef DECLARE_OPTIONAL_GETTER
 
 }  // namespace internal
 }  // namespace v8

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

@@ -325,19 +325,11 @@ TEST(GrowMemoryPreservesData) {
 }
 
 TEST(GrowMemoryInvalidSize) {
-  {
-    // Grow memory by an invalid amount without initial memory.
-    WasmRunner<int32_t, uint32_t> r(kExecuteInterpreted);
-    BUILD(r, WASM_GROW_MEMORY(WASM_GET_LOCAL(0)));
-    CHECK_EQ(-1, r.Call(1048575));
-  }
-  {
   // Grow memory by an invalid amount without initial memory.
   WasmRunner<int32_t, uint32_t> r(kExecuteInterpreted);
   r.module().AddMemory(WasmModule::kPageSize);
   BUILD(r, WASM_GROW_MEMORY(WASM_GET_LOCAL(0)));
   CHECK_EQ(-1, r.Call(1048575));
-  }
 }
 
 TEST(TestPossibleNondeterminism) {
@@ -431,6 +423,7 @@ TEST(WasmInterpreterActivations) {
 
 TEST(InterpreterLoadWithoutMemory) {
   WasmRunner<int32_t, int32_t> r(kExecuteInterpreted);
+  r.module().AddMemory(0);
   BUILD(r, WASM_LOAD_MEM(MachineType::Int32(), WASM_GET_LOCAL(0)));
   CHECK_TRAP32(r.Call(0));
 }

test/cctest/wasm/wasm-run-utils.h

@@ -76,16 +76,10 @@ class TestingModule : public ModuleEnv {
  public:
   explicit TestingModule(Zone* zone, WasmExecutionMode mode = kExecuteCompiled)
       : ModuleEnv(&module_, &instance_),
-        execution_mode_(mode),
         instance_(&module_),
         isolate_(CcTest::InitIsolateOnce()),
         global_offset(0),
-        interpreter_(
-            mode == kExecuteInterpreted
-                ? new WasmInterpreter(
-                      isolate_, ModuleBytesEnv(&module_, &instance_,
-                                               Vector<const byte>::empty()))
-                : nullptr) {
+        interpreter_(nullptr) {
     WasmJs::Install(isolate_);
     instance->module = &module_;
     instance->globals_start = global_data;
@@ -94,23 +88,11 @@ class TestingModule : public ModuleEnv {
     instance->mem_size = 0;
     memset(global_data, 0, sizeof(global_data));
     instance_object_ = InitInstanceObject();
-  }
-
-  ~TestingModule() {
-    if (instance->mem_start) {
-      if (EnableGuardRegions() && module_.is_wasm()) {
-        // See the corresponding code in AddMemory. We use a different
-        // allocation path when guard regions are enabled, which means we have
-        // to free it differently too.
-        const size_t alloc_size =
-            RoundUp(kWasmMaxHeapOffset, v8::base::OS::CommitPageSize());
-        v8::base::OS::Free(instance->mem_start, alloc_size);
-      } else {
-        free(instance->mem_start);
+    if (mode == kExecuteInterpreted) {
+      interpreter_ =
+          WasmDebugInfo::SetupForTesting(instance_object_, &instance_);
     }
   }
-    if (interpreter_) delete interpreter_;
-  }
 
   void ChangeOriginToAsmjs() { module_.set_origin(kAsmJsOrigin); }
 
@@ -118,22 +100,20 @@ class TestingModule : public ModuleEnv {
     CHECK(!module_.has_memory);
     CHECK_NULL(instance->mem_start);
     CHECK_EQ(0, instance->mem_size);
+    DCHECK(!instance_object_->has_memory_buffer());
     module_.has_memory = true;
-    if (EnableGuardRegions() && module_.is_wasm()) {
-      const size_t alloc_size =
-          RoundUp(kWasmMaxHeapOffset, v8::base::OS::CommitPageSize());
-      instance->mem_start = reinterpret_cast<byte*>(
-          v8::base::OS::AllocateGuarded(alloc_size * 2));
-      instance->mem_start += alloc_size;
-      const size_t guard_size = RoundUp(size, v8::base::OS::CommitPageSize());
-      v8::base::OS::Unprotect(instance->mem_start, guard_size);
-    } else {
-      instance->mem_start = reinterpret_cast<byte*>(malloc(size));
-    }
+    bool enable_guard_regions = EnableGuardRegions() && module_.is_wasm();
+    uint32_t alloc_size =
+        enable_guard_regions ? RoundUp(size, OS::CommitPageSize()) : size;
+    Handle<JSArrayBuffer> new_buffer =
+        wasm::NewArrayBuffer(isolate_, alloc_size, enable_guard_regions);
+    CHECK(!new_buffer.is_null());
+    instance_object_->set_memory_buffer(*new_buffer);
+    instance->mem_start = reinterpret_cast<byte*>(new_buffer->backing_store());
     CHECK(size == 0 || instance->mem_start);
     memset(instance->mem_start, 0, size);
     instance->mem_size = size;
-    return raw_mem_start<byte>();
+    return instance->mem_start;
   }
 
   template <typename T>
@@ -289,7 +269,7 @@ class TestingModule : public ModuleEnv {
   }
 
   void PopulateIndirectFunctionTable() {
-    if (execution_mode_ == kExecuteInterpreted) return;
+    if (interpret()) return;
     // Initialize the fixed arrays in instance->function_tables.
     for (uint32_t i = 0; i < instance->function_tables.size(); i++) {
       WasmIndirectFunctionTable& table = module_.function_tables[i];
@@ -324,12 +304,11 @@ class TestingModule : public ModuleEnv {
   WasmFunction* GetFunctionAt(int index) { return &module_.functions[index]; }
 
   WasmInterpreter* interpreter() { return interpreter_; }
-  WasmExecutionMode execution_mode() { return execution_mode_; }
+  bool interpret() { return interpreter_ != nullptr; }
   Isolate* isolate() { return isolate_; }
   Handle<WasmInstanceObject> instance_object() { return instance_object_; }
 
  private:
-  WasmExecutionMode execution_mode_;
   WasmModule module_;
   WasmInstance instance_;
   Isolate* isolate_;
@@ -566,7 +545,6 @@ class WasmFunctionCompiler : private GraphAndBuilders {
     if (interpreter_) {
       // Add the code to the interpreter.
       interpreter_->SetFunctionCodeForTesting(function_, start, end);
-      return;
     }
 
     // Build the TurboFan graph.
@@ -714,7 +692,10 @@ class WasmRunnerBase : public HandleAndZoneScope {
 
   uint32_t function_index() { return functions_[0]->function_index(); }
   WasmFunction* function() { return functions_[0]->function_; }
-  WasmInterpreter* interpreter() { return functions_[0]->interpreter_; }
+  WasmInterpreter* interpreter() {
+    DCHECK(interpret());
+    return functions_[0]->interpreter_;
+  }
   bool possible_nondeterminism() { return possible_nondeterminism_; }
   TestingModule& module() { return module_; }
   Zone* zone() { return &zone_; }
@@ -729,6 +710,8 @@ class WasmRunnerBase : public HandleAndZoneScope {
     module_.instance->context = main_isolate()->native_context();
   }
 
+  bool interpret() { return module_.interpret(); }
+
  private:
   FunctionSig* CreateSig(MachineType return_type,
                          Vector<MachineType> param_types) {
@@ -768,8 +751,6 @@ class WasmRunnerBase : public HandleAndZoneScope {
   bool compiled_ = false;
   bool possible_nondeterminism_ = false;
 
-  bool interpret() { return module_.execution_mode() == kExecuteInterpreted; }
-
  public:
   // This field has to be static. Otherwise, gcc complains about the use in
   // the lambda context below.