[wasm] Return MaybeHandle where allocation might fail

We sometimes allow allocation to fail and return a null Handle in that
case (e.g. for grow_memory). This refactors this code to return a
MaybeHandle instead, to document that allocation might fail and to force
the caller to handle this.

R=mstarzinger@chromium.org

Change-Id: Ia3ba65f840cfb1cf93e8dbd508a17375c19bae58
Reviewed-on: https://chromium-review.googlesource.com/995438
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52358}

src/wasm/module-compiler.cc

@@ -1652,16 +1652,14 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
   uint32_t globals_size = module_->globals_size;
   if (globals_size > 0) {
     constexpr bool enable_guard_regions = false;
-    Handle<JSArrayBuffer> global_buffer =
-        NewArrayBuffer(isolate_, globals_size, enable_guard_regions);
-    globals_ = global_buffer;
-    if (globals_.is_null()) {
+    if (!NewArrayBuffer(isolate_, globals_size, enable_guard_regions)
+             .ToHandle(&globals_)) {
       thrower_->RangeError("Out of memory: wasm globals");
       return {};
     }
     wasm_context->globals_start =
-        reinterpret_cast<byte*>(global_buffer->backing_store());
-    instance->set_globals_buffer(*global_buffer);
+        reinterpret_cast<byte*>(globals_->backing_store());
+    instance->set_globals_buffer(*globals_);
   }
 
   //--------------------------------------------------------------------------
@@ -2353,11 +2351,12 @@ Handle<JSArrayBuffer> InstanceBuilder::AllocateMemory(uint32_t num_pages) {
   const bool enable_guard_regions = use_trap_handler();
   const bool is_shared_memory =
       module_->has_shared_memory && i::FLAG_experimental_wasm_threads;
-  Handle<JSArrayBuffer> mem_buffer = NewArrayBuffer(
-      isolate_, num_pages * kWasmPageSize, enable_guard_regions,
-      is_shared_memory ? i::SharedFlag::kShared : i::SharedFlag::kNotShared);
-
-  if (mem_buffer.is_null()) {
+  i::SharedFlag shared_flag =
+      is_shared_memory ? i::SharedFlag::kShared : i::SharedFlag::kNotShared;
+  Handle<JSArrayBuffer> mem_buffer;
+  if (!NewArrayBuffer(isolate_, num_pages * kWasmPageSize, enable_guard_regions,
+                      shared_flag)
+           .ToHandle(&mem_buffer)) {
     thrower_->RangeError("Out of memory: wasm memory");
   }
   return mem_buffer;

src/wasm/wasm-js.cc

@@ -656,10 +656,12 @@ void WebAssemblyMemory(const v8::FunctionCallbackInfo<v8::Value>& args) {
                 static_cast<size_t>(initial);
   const bool enable_guard_regions =
       internal::trap_handler::IsTrapHandlerEnabled();
-  i::Handle<i::JSArrayBuffer> buffer = i::wasm::NewArrayBuffer(
-      i_isolate, size, enable_guard_regions,
-      is_shared_memory ? i::SharedFlag::kShared : i::SharedFlag::kNotShared);
-  if (buffer.is_null()) {
+  i::SharedFlag shared_flag =
+      is_shared_memory ? i::SharedFlag::kShared : i::SharedFlag::kNotShared;
+  i::Handle<i::JSArrayBuffer> buffer;
+  if (!i::wasm::NewArrayBuffer(i_isolate, size, enable_guard_regions,
+                               shared_flag)
+           .ToHandle(&buffer)) {
     thrower.RangeError("could not allocate memory");
     return;
   }

src/wasm/wasm-memory.cc

@@ -223,15 +223,15 @@ Handle<JSArrayBuffer> SetupArrayBuffer(Isolate* isolate, void* backing_store,
   return buffer;
 }
 
-Handle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
-                                     bool require_guard_regions,
-                                     SharedFlag shared) {
+MaybeHandle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
+                                          bool require_guard_regions,
+                                          SharedFlag shared) {
   // Check against kMaxInt, since the byte length is stored as int in the
   // JSArrayBuffer. Note that wasm_max_mem_pages can be raised from the command
   // line, and we don't want to fail a CHECK then.
   if (size > FLAG_wasm_max_mem_pages * kWasmPageSize || size > kMaxInt) {
     // TODO(titzer): lift restriction on maximum memory allocated here.
-    return Handle<JSArrayBuffer>::null();
+    return {};
   }
 
   WasmMemoryTracker* memory_tracker = isolate->wasm_engine()->memory_tracker();
@@ -248,9 +248,7 @@ Handle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
                                     require_guard_regions, &allocation_base,
                                     &allocation_length);
 
-  if (size > 0 && memory == nullptr) {
-    return Handle<JSArrayBuffer>::null();
-  }
+  if (size > 0 && memory == nullptr) return {};
 
 #if DEBUG
   // Double check the API allocator actually zero-initialized the memory.

src/wasm/wasm-memory.h

@@ -113,7 +113,7 @@ class WasmMemoryTracker {
   DISALLOW_COPY_AND_ASSIGN(WasmMemoryTracker);
 };
 
-Handle<JSArrayBuffer> NewArrayBuffer(
+MaybeHandle<JSArrayBuffer> NewArrayBuffer(
     Isolate*, size_t size, bool require_guard_regions,
     SharedFlag shared = SharedFlag::kNotShared);
 

src/wasm/wasm-objects.cc

@@ -339,11 +339,12 @@ void WasmTableObject::ClearDispatchTables(Handle<WasmTableObject> table,
 }
 
 namespace {
-Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
-                                       Handle<JSArrayBuffer> old_buffer,
-                                       uint32_t pages, uint32_t maximum_pages,
-                                       bool use_trap_handler) {
-  if (!old_buffer->is_growable()) return Handle<JSArrayBuffer>::null();
+MaybeHandle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
+                                            Handle<JSArrayBuffer> old_buffer,
+                                            uint32_t pages,
+                                            uint32_t maximum_pages,
+                                            bool use_trap_handler) {
+  if (!old_buffer->is_growable()) return {};
   Address old_mem_start = nullptr;
   uint32_t old_size = 0;
   if (!old_buffer.is_null()) {
@@ -354,14 +355,12 @@ Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
   uint32_t old_pages = old_size / wasm::kWasmPageSize;
   DCHECK_GE(std::numeric_limits<uint32_t>::max(),
             old_size + pages * wasm::kWasmPageSize);
-  if (old_pages > maximum_pages || pages > maximum_pages - old_pages) {
-    return Handle<JSArrayBuffer>::null();
-  }
+  if (old_pages > maximum_pages || pages > maximum_pages - old_pages) return {};
   size_t new_size =
       static_cast<size_t>(old_pages + pages) * wasm::kWasmPageSize;
   if (new_size > FLAG_wasm_max_mem_pages * wasm::kWasmPageSize ||
       new_size > kMaxInt) {
-    return Handle<JSArrayBuffer>::null();
+    return {};
   }
   // Reusing the backing store from externalized buffers causes problems with
   // Blink's array buffers. The connection between the two is lost, which can
@@ -377,7 +376,7 @@ Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
           old_mem_start));
       if (!i::SetPermissions(old_mem_start, new_size,
                              PageAllocator::kReadWrite)) {
-        return Handle<JSArrayBuffer>::null();
+        return {};
       }
       reinterpret_cast<v8::Isolate*>(isolate)
           ->AdjustAmountOfExternalAllocatedMemory(pages * wasm::kWasmPageSize);
@@ -395,8 +394,11 @@ Handle<JSArrayBuffer> GrowMemoryBuffer(Isolate* isolate,
     // We couldn't reuse the old backing store, so create a new one and copy the
     // old contents in.
     Handle<JSArrayBuffer> new_buffer;
-    new_buffer = wasm::NewArrayBuffer(isolate, new_size, use_trap_handler);
-    if (new_buffer.is_null() || old_size == 0) return new_buffer;
+    if (!wasm::NewArrayBuffer(isolate, new_size, use_trap_handler)
+             .ToHandle(&new_buffer)) {
+      return {};
+    }
+    if (old_size == 0) return new_buffer;
     Address new_mem_start = static_cast<Address>(new_buffer->backing_store());
     memcpy(new_mem_start, old_mem_start, old_size);
     DCHECK(old_buffer.is_null() || !old_buffer->is_shared());
@@ -499,9 +501,11 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
   }
   // TODO(kschimpf): We need to fix this by adding a field to WasmMemoryObject
   // that defines the style of memory being used.
-  new_buffer = GrowMemoryBuffer(isolate, old_buffer, pages, maximum_pages,
-                                trap_handler::IsTrapHandlerEnabled());
-  if (new_buffer.is_null()) return -1;
+  if (!GrowMemoryBuffer(isolate, old_buffer, pages, maximum_pages,
+                        trap_handler::IsTrapHandlerEnabled())
+           .ToHandle(&new_buffer)) {
+    return -1;
+  }
 
   if (memory_object->has_instances()) {
     Handle<FixedArrayOfWeakCells> instances(memory_object->instances(),

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

@@ -1084,8 +1084,10 @@ TEST(Run_WasmModule_Buffer_Externalized_GrowMemMemSize) {
 #else
     constexpr bool require_guard_regions = false;
 #endif
-    Handle<JSArrayBuffer> buffer = wasm::NewArrayBuffer(
-        isolate, 16 * kWasmPageSize, require_guard_regions);
+    Handle<JSArrayBuffer> buffer;
+    CHECK(
+        wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize, require_guard_regions)
+            .ToHandle(&buffer));
     Handle<WasmMemoryObject> mem_obj =
         WasmMemoryObject::New(isolate, buffer, 100);
     auto const contents = v8::Utils::ToLocal(buffer)->Externalize();
@@ -1111,8 +1113,10 @@ TEST(Run_WasmModule_Buffer_Externalized_Detach) {
 #else
     constexpr bool require_guard_regions = false;
 #endif
-    Handle<JSArrayBuffer> buffer = wasm::NewArrayBuffer(
-        isolate, 16 * kWasmPageSize, require_guard_regions);
+    Handle<JSArrayBuffer> buffer;
+    CHECK(
+        wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize, require_guard_regions)
+            .ToHandle(&buffer));
     auto const contents = v8::Utils::ToLocal(buffer)->Externalize();
     wasm::DetachMemoryBuffer(isolate, buffer, true);
     constexpr bool is_wasm_memory = true;
@@ -1133,8 +1137,9 @@ TEST(Run_WasmModule_Buffer_Externalized_Regression_UseAfterFree) {
 #else
   constexpr bool require_guard_regions = false;
 #endif
-  Handle<JSArrayBuffer> buffer =
-      wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize, require_guard_regions);
+  Handle<JSArrayBuffer> buffer;
+  CHECK(wasm::NewArrayBuffer(isolate, 16 * kWasmPageSize, require_guard_regions)
+            .ToHandle(&buffer));
   Handle<WasmMemoryObject> mem = WasmMemoryObject::New(isolate, buffer, 128);
   auto contents = v8::Utils::ToLocal(buffer)->Externalize();
   WasmMemoryObject::Grow(isolate, mem, 0);
@@ -1157,9 +1162,9 @@ TEST(Run_WasmModule_Reclaim_Memory) {
   for (int i = 0; i < 256; ++i) {
     HandleScope scope(isolate);
     constexpr bool require_guard_regions = true;
-    buffer = NewArrayBuffer(isolate, kWasmPageSize, require_guard_regions,
-                            SharedFlag::kNotShared);
-    CHECK(!buffer.is_null());
+    CHECK(NewArrayBuffer(isolate, kWasmPageSize, require_guard_regions,
+                         SharedFlag::kNotShared)
+              .ToHandle(&buffer));
   }
 }
 #endif

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

@@ -44,8 +44,9 @@ byte* TestingModuleBuilder::AddMemory(uint32_t size) {
       trap_handler::IsTrapHandlerEnabled() && test_module_.is_wasm();
   uint32_t alloc_size =
       enable_guard_regions ? RoundUp(size, CommitPageSize()) : size;
-  Handle<JSArrayBuffer> new_buffer =
-      wasm::NewArrayBuffer(isolate_, alloc_size, enable_guard_regions);
+  Handle<JSArrayBuffer> new_buffer;
+  CHECK(wasm::NewArrayBuffer(isolate_, alloc_size, enable_guard_regions)
+            .ToHandle(&new_buffer));
   CHECK(!new_buffer.is_null());
   mem_start_ = reinterpret_cast<byte*>(new_buffer->backing_store());
   mem_size_ = size;