[wasm] Detach memory buffer only when GrowMemory is called from the JS API

BUG=chromium:699485

R=ahaas@chromium.org, bradnelson@chromium.org

Review-Url: https://codereview.chromium.org/2772973002
Cr-Commit-Position: refs/heads/master@{#44166}

src/wasm/wasm-js.cc

@@ -706,6 +706,7 @@ void WebAssemblyMemoryGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
     thrower.RangeError("Unable to grow instance memory.");
     return;
   }
+  i::wasm::DetachWebAssemblyMemoryBuffer(i_isolate, old_buffer);
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   return_value.Set(ret);
 }

src/wasm/wasm-module.cc

@@ -2358,7 +2358,8 @@ Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
   Handle<JSArrayBuffer> old_buffer;
   Address old_mem_start = nullptr;
   uint32_t old_size = 0;
-  if (buffer.ToHandle(&old_buffer) && old_buffer->backing_store() != nullptr) {
+  if (buffer.ToHandle(&old_buffer) && old_buffer->backing_store() != nullptr &&
+      old_buffer->byte_length()->IsNumber()) {
     old_mem_start = static_cast<Address>(old_buffer->backing_store());
     DCHECK_NOT_NULL(old_mem_start);
     old_size = old_buffer->byte_length()->Number();
@@ -2401,28 +2402,30 @@ void UncheckedUpdateInstanceMemory(Isolate* isolate,
   code_specialization.ApplyToWholeInstance(*instance);
 }
 
-void DetachArrayBuffer(Isolate* isolate, Handle<JSArrayBuffer> buffer) {
-  const bool has_guard_regions =
-      (!buffer.is_null() && buffer->has_guard_region());
+void wasm::DetachWebAssemblyMemoryBuffer(Isolate* isolate,
+                                         Handle<JSArrayBuffer> buffer) {
+  int64_t byte_length =
+      buffer->byte_length()->IsNumber()
+          ? static_cast<uint32_t>(buffer->byte_length()->Number())
+          : 0;
+  if (buffer.is_null() || byte_length == 0) return;
+  const bool has_guard_regions = buffer->has_guard_region();
   const bool is_external = buffer->is_external();
   void* backing_store = buffer->backing_store();
-  if (backing_store != nullptr) {
-    DCHECK(!buffer->is_neuterable());
-    int64_t byte_length = NumberToSize(buffer->byte_length());
-    buffer->set_is_neuterable(true);
-    if (!has_guard_regions && !is_external) {
-      buffer->set_is_external(true);
-      isolate->heap()->UnregisterArrayBuffer(*buffer);
-    }
-    buffer->Neuter();
-    if (has_guard_regions) {
-      base::OS::Free(backing_store, RoundUp(i::wasm::kWasmMaxHeapOffset,
-                                            base::OS::CommitPageSize()));
-      reinterpret_cast<v8::Isolate*>(isolate)
-          ->AdjustAmountOfExternalAllocatedMemory(-byte_length);
-    } else if (!has_guard_regions && !is_external) {
-      isolate->array_buffer_allocator()->Free(backing_store, byte_length);
-    }
+  DCHECK(!buffer->is_neuterable());
+  if (!has_guard_regions && !is_external) {
+    buffer->set_is_external(true);
+    isolate->heap()->UnregisterArrayBuffer(*buffer);
+  }
+  buffer->set_is_neuterable(true);
+  buffer->Neuter();
+  if (has_guard_regions) {
+    base::OS::Free(backing_store, RoundUp(i::wasm::kWasmMaxHeapOffset,
+                                          base::OS::CommitPageSize()));
+    reinterpret_cast<v8::Isolate*>(isolate)
+        ->AdjustAmountOfExternalAllocatedMemory(-byte_length);
+  } else if (!has_guard_regions && !is_external) {
+    isolate->array_buffer_allocator()->Free(backing_store, byte_length);
   }
 }
 
@@ -2436,8 +2439,10 @@ int32_t wasm::GrowWebAssemblyMemory(Isolate* isolate,
   Handle<JSArrayBuffer> old_buffer;
   uint32_t old_size = 0;
   Address old_mem_start = nullptr;
+  // Force byte_length to 0, if byte_length fails IsNumber() check.
   if (memory_buffer.ToHandle(&old_buffer) &&
-      old_buffer->backing_store() != nullptr) {
+      old_buffer->backing_store() != nullptr &&
+      old_buffer->byte_length()->IsNumber()) {
     old_size = old_buffer->byte_length()->Number();
     old_mem_start = static_cast<Address>(old_buffer->backing_store());
   }
@@ -2497,7 +2502,6 @@ int32_t wasm::GrowWebAssemblyMemory(Isolate* isolate,
     }
   }
   memory_object->set_buffer(*new_buffer);
-  DetachArrayBuffer(isolate, old_buffer);
   DCHECK(old_size % WasmModule::kPageSize == 0);
   return (old_size / WasmModule::kPageSize);
 }

src/wasm/wasm-module.h

@@ -437,6 +437,9 @@ int32_t GrowWebAssemblyMemory(Isolate* isolate,
 int32_t GrowMemory(Isolate* isolate, Handle<WasmInstanceObject> instance,
                    uint32_t pages);
 
+void DetachWebAssemblyMemoryBuffer(Isolate* isolate,
+                                   Handle<JSArrayBuffer> buffer);
+
 void UpdateDispatchTables(Isolate* isolate, Handle<FixedArray> dispatch_tables,
                           int index, Handle<JSFunction> js_function);
 

test/mjsunit/regress/wasm/regression-699485.js

@@ -0,0 +1,22 @@
+// Copyright 2016 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: --expose-wasm
+
+load("test/mjsunit/wasm/wasm-constants.js");
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+(function() {
+"use asm";
+var builder = new WasmModuleBuilder();
+builder.addMemory(0, 5, false);
+builder.addFunction("regression_699485", kSig_i_v)
+  .addBody([
+      kExprI32Const, 0x04,
+      kExprNop,
+      kExprGrowMemory, 0x00,
+      ]).exportFunc();
+let module = builder.instantiate();
+assertEquals(0, module.exports.regression_699485());
+})();