[wasm] Fix grow_memory implementation in interpreter

grow_memory was working from test cases, but not in combination with
compiled code. This CL makes the effect of grow_memory executed either
in the interpreter or compiled code always be reflected in both
execution environments.
It also adds a %RedirectToWasmInterpreter runtime function for testing
this interaction.

R=ahaas@chromium.org
CC=gdeepti@chromium.org
BUG=v8:5822

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

src/runtime/runtime-test.cc

@@ -981,6 +981,21 @@ RUNTIME_FUNCTION(Runtime_WasmNumInterpretedCalls) {
   return *isolate->factory()->NewNumberFromSize(static_cast<size_t>(num));
 }
 
+RUNTIME_FUNCTION(Runtime_RedirectToWasmInterpreter) {
+  DCHECK_EQ(2, args.length());
+  HandleScope scope(isolate);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, instance_obj, 0);
+  CONVERT_SMI_ARG_CHECKED(function_index, 1);
+  CHECK(WasmInstanceObject::IsWasmInstanceObject(*instance_obj));
+  Handle<WasmInstanceObject> instance =
+      Handle<WasmInstanceObject>::cast(instance_obj);
+  Handle<WasmDebugInfo> debug_info =
+      WasmInstanceObject::GetOrCreateDebugInfo(instance);
+  WasmDebugInfo::RedirectToInterpreter(debug_info,
+                                       Vector<int>(&function_index, 1));
+  return isolate->heap()->undefined_value();
+}
+
 RUNTIME_FUNCTION(Runtime_IncrementWaitCount) {
   isolate->IncrementWaitCountForTesting();
   return isolate->heap()->undefined_value();

src/runtime/runtime.h

@@ -611,7 +611,8 @@ namespace internal {
   F(SetWasmCompileControls, 2, 1)             \
   F(SetWasmInstantiateControls, 0, 1)         \
   F(Verify, 1, 1)                             \
-  F(WasmNumInterpretedCalls, 1, 1)
+  F(WasmNumInterpretedCalls, 1, 1)            \
+  F(RedirectToWasmInterpreter, 2, 1)
 
 #define FOR_EACH_INTRINSIC_TYPEDARRAY(F)     \
   F(ArrayBufferGetByteLength, 1, 1)          \

src/wasm/wasm-code-specialization.cc

@@ -137,28 +137,34 @@ bool CodeSpecialization::ApplyToWholeInstance(
     changed |= ApplyToWasmCode(wasm_function, icache_flush_mode);
   }
 
-  // Patch all exported functions.
-  for (auto exp : module->export_table) {
-    if (exp.kind != kExternalFunction) continue;
-    Code* export_wrapper = Code::cast(code_table->get(func_index));
-    DCHECK_EQ(Code::JS_TO_WASM_FUNCTION, export_wrapper->kind());
-    // There must be exactly one call to WASM_FUNCTION or WASM_TO_JS_FUNCTION.
-    for (RelocIterator it(export_wrapper,
-                          RelocInfo::ModeMask(RelocInfo::CODE_TARGET));
-         ; it.next()) {
-      DCHECK(!it.done());
-      // Ignore calls to other builtins like ToNumber.
-      if (!IsAtWasmDirectCallTarget(it)) continue;
-      Code* new_code = Code::cast(code_table->get(exp.index));
-      it.rinfo()->set_target_address(new_code->GetIsolate(),
-                                     new_code->instruction_start(),
-                                     UPDATE_WRITE_BARRIER, SKIP_ICACHE_FLUSH);
-      break;
+  // Patch all exported functions (if we shall relocate direct calls).
+  if (!relocate_direct_calls_instance.is_null()) {
+    // If we patch direct calls, the instance registered for that
+    // (relocate_direct_calls_instance) should match the instance we currently
+    // patch (instance).
+    DCHECK_EQ(instance, *relocate_direct_calls_instance);
+    for (auto exp : module->export_table) {
+      if (exp.kind != kExternalFunction) continue;
+      Code* export_wrapper = Code::cast(code_table->get(func_index));
+      DCHECK_EQ(Code::JS_TO_WASM_FUNCTION, export_wrapper->kind());
+      // There must be exactly one call to WASM_FUNCTION or WASM_TO_JS_FUNCTION.
+      for (RelocIterator it(export_wrapper,
+                            RelocInfo::ModeMask(RelocInfo::CODE_TARGET));
+           ; it.next()) {
+        DCHECK(!it.done());
+        // Ignore calls to other builtins like ToNumber.
+        if (!IsAtWasmDirectCallTarget(it)) continue;
+        Code* new_code = Code::cast(code_table->get(exp.index));
+        it.rinfo()->set_target_address(new_code->GetIsolate(),
+                                       new_code->instruction_start(),
+                                       UPDATE_WRITE_BARRIER, SKIP_ICACHE_FLUSH);
+        break;
+      }
+      changed = true;
+      func_index++;
     }
-    changed = true;
-    func_index++;
+    DCHECK_EQ(code_table->length(), func_index);
   }
-  DCHECK_EQ(code_table->length(), func_index);
   return changed;
 }
 

src/wasm/wasm-debug.cc

@@ -80,17 +80,12 @@ class InterpreterHandle {
     interpreter_.SetInstanceObject(instance);
 
     // Set memory start pointer and size.
-    // TODO(wasm): Update this on grow_memory in both directions (compiled ->
-    // interpreter, interpreter -> compiles).
+    instance_.mem_start = nullptr;
+    instance_.mem_size = 0;
     if (instance->has_memory_buffer()) {
-      JSArrayBuffer* mem_buffer = instance->memory_buffer();
-      instance_.mem_start =
-          reinterpret_cast<byte*>(mem_buffer->backing_store());
-      CHECK(mem_buffer->byte_length()->ToUint32(&instance_.mem_size));
+      UpdateMemory(instance->memory_buffer());
     } else {
       DCHECK_EQ(0, instance_.module->min_mem_pages);
-      instance_.mem_start = nullptr;
-      instance_.mem_size = 0;
     }
 
     // Set pointer to globals storage.
@@ -368,6 +363,11 @@ class InterpreterHandle {
     DCHECK_EQ(1, interpreter()->GetThreadCount());
     return interpreter()->GetThread(0)->NumInterpretedCalls();
   }
+
+  void UpdateMemory(JSArrayBuffer* new_memory) {
+    instance_.mem_start = reinterpret_cast<byte*>(new_memory->backing_store());
+    CHECK(new_memory->byte_length()->ToUint32(&instance_.mem_size));
+  }
 };
 
 InterpreterHandle* GetOrCreateInterpreterHandle(
@@ -548,3 +548,9 @@ uint64_t WasmDebugInfo::NumInterpretedCalls() {
   auto handle = GetInterpreterHandleOrNull(this);
   return handle ? handle->NumInterpretedCalls() : 0;
 }
+
+void WasmDebugInfo::UpdateMemory(JSArrayBuffer* new_memory) {
+  InterpreterHandle* interp_handle = GetInterpreterHandleOrNull(this);
+  if (!interp_handle) return;
+  interp_handle->UpdateMemory(new_memory);
+}

src/wasm/wasm-interpreter.cc

@@ -595,47 +595,62 @@ inline int64_t ExecuteI64ReinterpretF64(WasmVal a) {
   return a.to_unchecked<int64_t>();
 }
 
-inline int32_t ExecuteGrowMemory(uint32_t delta_pages, WasmInstance* instance) {
+inline int32_t ExecuteGrowMemory(uint32_t delta_pages,
+                                 MaybeHandle<WasmInstanceObject> instance_obj,
+                                 WasmInstance* instance) {
+  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);
+    // Some sanity checks.
+    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);
+    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 old_size = instance->mem_size;
-  uint32_t new_size;
+
+  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_size = delta_pages * wasm::WasmModule::kPageSize;
-    new_mem_start = static_cast<byte*>(calloc(new_size, sizeof(byte)));
-    if (!new_mem_start) {
-      return -1;
-    }
+    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);
-    new_size = old_size + delta_pages * wasm::WasmModule::kPageSize;
-    if (new_size / wasm::WasmModule::kPageSize > FLAG_wasm_max_mem_pages ||
-        new_size / wasm::WasmModule::kPageSize >
-            instance->module->max_mem_pages) {
-      return -1;
-    }
     if (EnableGuardRegions()) {
-      v8::base::OS::Unprotect(instance->mem_start, new_size);
+      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_size));
-      if (!new_mem_start) {
-        return -1;
-      }
+      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_size, 0, new_size - old_size);
+    memset(new_mem_start + old_pages * WasmModule::kPageSize, 0,
+           delta_pages * WasmModule::kPageSize);
   }
   instance->mem_start = new_mem_start;
-  instance->mem_size = new_size;
-  return static_cast<int32_t>(old_size / WasmModule::kPageSize);
+  instance->mem_size = new_pages * WasmModule::kPageSize;
+  return static_cast<int32_t>(old_pages);
 }
 
 enum InternalOpcode {
@@ -923,6 +938,9 @@ class CodeMap {
     DCHECK(has_instance());
     return instance_;
   }
+  MaybeHandle<WasmInstanceObject> maybe_instance() const {
+    return has_instance() ? instance_ : MaybeHandle<WasmInstanceObject>();
+  }
 
   void SetInstanceObject(WasmInstanceObject* instance) {
     // Only set the instance once (otherwise we have to destroy the global
@@ -1867,7 +1885,8 @@ class ThreadImpl {
         case kExprGrowMemory: {
           MemoryIndexOperand operand(&decoder, code->at(pc));
           uint32_t delta_pages = Pop().to<uint32_t>();
-          Push(pc, WasmVal(ExecuteGrowMemory(delta_pages, instance())));
+          Push(pc, WasmVal(ExecuteGrowMemory(
+                       delta_pages, codemap_->maybe_instance(), instance())));
           len = 1 + operand.length;
           break;
         }

src/wasm/wasm-objects.cc

@@ -376,6 +376,9 @@ void SetInstanceMemory(Isolate* isolate, Handle<WasmInstanceObject> instance,
   instance->set_memory_buffer(*buffer);
   WasmCompiledModule::SetSpecializationMemInfoFrom(
       isolate->factory(), handle(instance->compiled_module()), buffer);
+  if (instance->has_debug_info()) {
+    instance->debug_info()->UpdateMemory(*buffer);
+  }
 }
 
 void UncheckedUpdateInstanceMemory(Isolate* isolate,

src/wasm/wasm-objects.h

@@ -573,6 +573,10 @@ class WasmDebugInfo : public FixedArray {
   uint64_t NumInterpretedCalls();
 
   DECLARE_GETTER(wasm_instance, WasmInstanceObject);
+
+  // Update the memory view of the interpreter after executing GrowMemory in
+  // compiled code.
+  void UpdateMemory(JSArrayBuffer* new_memory);
 };
 
 class WasmInstanceWrapper : public FixedArray {

test/mjsunit/wasm/interpreter-mixed.js

+// Copyright 2017 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.
+
+// Flags: --allow-natives-syntax
+
+load('test/mjsunit/wasm/wasm-constants.js');
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+// =============================================================================
+// Tests in this file test the interaction between the wasm interpreter and
+// compiled code.
+// =============================================================================
+
+(function testGrowMemoryBetweenInterpretedAndCompiled() {
+  // grow_memory can be called from interpreted or compiled code, and changes
+  // should be reflected in either execution.
+  var builder = new WasmModuleBuilder();
+  var grow_body = [kExprGetLocal, 0, kExprGrowMemory, kMemoryZero];
+  var load_body = [kExprGetLocal, 0, kExprI32LoadMem, 0, 0];
+  var store_body = [kExprGetLocal, 0, kExprGetLocal, 1, kExprI32StoreMem, 0, 0];
+  builder.addFunction('grow_memory', kSig_i_i).addBody(grow_body).exportFunc();
+  builder.addFunction('load', kSig_i_i).addBody(load_body).exportFunc();
+  builder.addFunction('store', kSig_v_ii).addBody(store_body).exportFunc();
+  var grow_interp_function =
+      builder.addFunction('grow_memory_interpreted', kSig_i_i)
+          .addBody(grow_body)
+          .exportFunc();
+  var load_interp_function = builder.addFunction('load_interpreted', kSig_i_i)
+                                 .addBody(load_body)
+                                 .exportFunc();
+  var kNumPages = 2;
+  var kMaxPages = 10;
+  builder.addMemory(kNumPages, kMaxPages, false);
+  var instance = builder.instantiate();
+  var exp = instance.exports;
+  %RedirectToWasmInterpreter(instance, grow_interp_function.index);
+  %RedirectToWasmInterpreter(instance, load_interp_function.index);
+
+  // Initially, we can load from offset 12, but not OOB.
+  var oob_index = kNumPages * kPageSize;
+  var initial_interpreted = % WasmNumInterpretedCalls(instance);
+  assertEquals(0, exp.load(12));
+  assertEquals(0, exp.load_interpreted(12));
+  assertTraps(kTrapMemOutOfBounds, () => exp.load(oob_index));
+  assertTraps(
+      kTrapMemOutOfBounds, () => exp.load_interpreted(oob_index));
+  // Grow by 2 pages from compiled code, and ensure that this is reflected in
+  // the interpreter.
+  assertEquals(kNumPages, exp.grow_memory(2));
+  kNumPages += 2;
+  assertEquals(kNumPages, exp.grow_memory_interpreted(0));
+  assertEquals(kNumPages, exp.grow_memory(0));
+  // Now we can load from the previous OOB index.
+  assertEquals(0, exp.load(oob_index));
+  assertEquals(0, exp.load_interpreted(oob_index));
+  // Set new OOB index and ensure that it traps.
+  oob_index = kNumPages * kPageSize;
+  assertTraps(kTrapMemOutOfBounds, () => exp.load(oob_index));
+  assertTraps(
+      kTrapMemOutOfBounds, () => exp.load_interpreted(oob_index));
+  // Grow by another page in the interpreter, and ensure that this is reflected
+  // in compiled code.
+  assertEquals(kNumPages, exp.grow_memory_interpreted(1));
+  kNumPages += 1;
+  assertEquals(kNumPages, exp.grow_memory_interpreted(0));
+  assertEquals(kNumPages, exp.grow_memory(0));
+  // Now we can store to the previous OOB index and read it back in both
+  // environments.
+  exp.store(oob_index, 47);
+  assertEquals(47, exp.load(oob_index));
+  assertEquals(47, exp.load_interpreted(oob_index));
+  // We cannot grow beyong kMaxPages.
+  assertEquals(-1, exp.grow_memory(kMaxPages - kNumPages + 1));
+  assertEquals(-1, exp.grow_memory_interpreted(kMaxPages - kNumPages + 1));
+  // Overall, we executed 9 functions in the interpreter.
+  assertEquals(initial_interpreted + 9, % WasmNumInterpretedCalls(instance));
+})();

test/mjsunit/wasm/wasm-module-builder.js

@@ -101,9 +101,9 @@ class WasmFunctionBuilder {
       if (typeof b != 'number')
         throw new Error('invalid body (entries have to be numbers): ' + body);
     }
-    this.body = body;
+    this.body = body.slice();
     // Automatically add the end for the function block to the body.
-    body.push(kExprEnd);
+    this.body.push(kExprEnd);
     return this;
   }