Backed out changeset 969534e11452 (bug 1843296) for webgpu crash

dom/webgpu/Buffer.cpp

@@ -16,7 +16,6 @@
 #include "nsContentUtils.h"
 #include "nsWrapperCache.h"
 #include "Device.h"
-#include "mozilla/webgpu/ffi/wgpu.h"
 
 namespace mozilla::webgpu {
 
@@ -71,52 +70,35 @@ already_AddRefed<Buffer> Buffer::Create(Device* aDevice, RawId aDeviceId,
 
   bool hasMapFlags = aDesc.mUsage & (dom::GPUBufferUsage_Binding::MAP_WRITE |
                                      dom::GPUBufferUsage_Binding::MAP_READ);
-
-  bool allocSucceeded = false;
   if (hasMapFlags || aDesc.mMappedAtCreation) {
-    // If shmem allocation fails, we continue and provide the parent side with
-    // an empty shmem which it will interpret as an OOM situtation.
     const auto checked = CheckedInt<size_t>(aDesc.mSize);
-    const size_t maxSize = WGPUMAX_BUFFER_SIZE;
-    if (checked.isValid() && checked.value() > 0 && checked.value() < maxSize) {
-      size_t size = checked.value();
+    if (!checked.isValid()) {
+      aRv.ThrowRangeError("Mappable size is too large");
+      return nullptr;
+    }
+    size_t size = checked.value();
 
-      auto maybeShmem = ipc::UnsafeSharedMemoryHandle::CreateAndMap(size);
+    auto maybeShmem = ipc::UnsafeSharedMemoryHandle::CreateAndMap(size);
 
-      if (maybeShmem.isSome()) {
-        allocSucceeded = true;
-        handle = std::move(maybeShmem.ref().first);
-        mapping = std::move(maybeShmem.ref().second);
-
-        MOZ_RELEASE_ASSERT(mapping.Size() >= size);
-
-        // zero out memory
-        memset(mapping.Bytes().data(), 0, size);
-      }
+    if (maybeShmem.isNothing()) {
+      aRv.ThrowRangeError(
+          nsPrintfCString("Unable to allocate shmem of size %" PRIuPTR, size));
+      return nullptr;
     }
 
-    if (checked.value() == 0) {
-      // Zero-sized buffers is a special case. We don't create a shmem since
-      // allocating the memory would not make sense, however mappable null
-      // buffers are allowed by the spec so we just pass the null handle which
-      // in practice deserializes into a null handle on the parent side and
-      // behaves like a zero-sized allocation.
-      allocSucceeded = true;
-    }
-  }
+    handle = std::move(maybeShmem.ref().first);
+    mapping = std::move(maybeShmem.ref().second);
 
-  // If mapped at creation and the shmem allocation failed, immediately throw
-  // a range error and don't attempt to create the buffer.
-  if (aDesc.mMappedAtCreation && !allocSucceeded) {
-    aRv.ThrowRangeError("Allocation failed");
-    return nullptr;
+    MOZ_RELEASE_ASSERT(mapping.Size() >= size);
+
+    // zero out memory
+    memset(mapping.Bytes().data(), 0, size);
   }
 
   RawId id = actor->DeviceCreateBuffer(aDeviceId, aDesc, std::move(handle));
 
   RefPtr<Buffer> buffer =
       new Buffer(aDevice, id, aDesc.mSize, aDesc.mUsage, std::move(mapping));
-
   if (aDesc.mMappedAtCreation) {
     // Mapped at creation's raison d'être is write access, since the buffer is
     // being created and there isn't anything interesting to read in it yet.

dom/webgpu/ipc/WebGPUParent.cpp

@@ -385,31 +385,22 @@ ipc::IPCResult WebGPUParent::RecvCreateBuffer(
 
   bool hasMapFlags = aDesc.mUsage & (dom::GPUBufferUsage_Binding::MAP_WRITE |
                                      dom::GPUBufferUsage_Binding::MAP_READ);
-  bool shmAllocationFailed = false;
   if (hasMapFlags || aDesc.mMappedAtCreation) {
-    if (shmem.Size() < aDesc.mSize) {
-      MOZ_RELEASE_ASSERT(shmem.Size() == 0);
-      // If we requested a non-zero mappable buffer and get a size of zero, it
-      // indicates that the shmem allocation failed on the client side.
-      shmAllocationFailed = true;
-    } else {
-      uint64_t offset = 0;
-      uint64_t size = 0;
-
-      if (aDesc.mMappedAtCreation) {
-        size = aDesc.mSize;
-      }
-
-      BufferMapData data = {std::move(shmem), hasMapFlags, offset, size};
-      mSharedMemoryMap.insert({aBufferId, std::move(data)});
+    uint64_t offset = 0;
+    uint64_t size = 0;
+    if (aDesc.mMappedAtCreation) {
+      size = aDesc.mSize;
+      MOZ_RELEASE_ASSERT(shmem.Size() >= aDesc.mSize);
     }
+
+    BufferMapData data = {std::move(shmem), hasMapFlags, offset, size};
+    mSharedMemoryMap.insert({aBufferId, std::move(data)});
   }
 
   ErrorBuffer error;
   ffi::wgpu_server_device_create_buffer(mContext.get(), aDeviceId, aBufferId,
                                         label.Get(), aDesc.mSize, aDesc.mUsage,
-                                        aDesc.mMappedAtCreation,
-                                        shmAllocationFailed, error.ToFFI());
+                                        aDesc.mMappedAtCreation, error.ToFFI());
   ForwardError(aDeviceId, error);
   return IPC_OK();
 }
@@ -944,7 +935,7 @@ ipc::IPCResult WebGPUParent::RecvSwapChainPresent(
       ErrorBuffer error;
       ffi::wgpu_server_device_create_buffer(mContext.get(), data->mDeviceId,
                                             bufferId, nullptr, bufferSize,
-                                            usage, false, false, error.ToFFI());
+                                            usage, false, error.ToFFI());
       if (ForwardError(data->mDeviceId, error)) {
         return IPC_OK();
       }

gfx/wgpu_bindings/src/server.rs

@@ -18,15 +18,12 @@ use std::borrow::Cow;
 use std::slice;
 use std::sync::atomic::{AtomicU32, Ordering};
 
-// The seemingly redundant u64 suffixes help cbindgen with generating the right C++ code.
-// See https://github.com/mozilla/cbindgen/issues/849.
-
 /// We limit the size of buffer allocations for stability reason.
 /// We can reconsider this limit in the future. Note that some drivers (mesa for example),
 /// have issues when the size of a buffer, mapping or copy command does not fit into a
 /// signed 32 bits integer, so beyond a certain size, large allocations will need some form
 /// of driver allow/blocklist.
-pub const MAX_BUFFER_SIZE: wgt::BufferAddress = 1u64 << 30u64;
+const MAX_BUFFER_SIZE: wgt::BufferAddress = 1 << 30;
 // Mesa has issues with height/depth that don't fit in a 16 bits signed integers.
 const MAX_TEXTURE_EXTENT: u32 = std::i16::MAX as u32;
 
@@ -282,7 +279,6 @@ pub extern "C" fn wgpu_server_device_create_buffer(
     size: wgt::BufferAddress,
     usage: u32,
     mapped_at_creation: bool,
-    shm_allocation_failed: bool,
     mut error_buf: ErrorBuffer,
 ) {
     let utf8_label = label.map(|utf16| utf16.to_string());
@@ -290,7 +286,7 @@ pub extern "C" fn wgpu_server_device_create_buffer(
     let usage = wgt::BufferUsages::from_bits_retain(usage);
 
     // Don't trust the graphics driver with buffer sizes larger than our conservative max texture size.
-    if shm_allocation_failed || size > MAX_BUFFER_SIZE {
+    if size > MAX_BUFFER_SIZE {
         error_buf.init(ErrMsg {
             message: "Out of memory",
             r#type: ErrorBufferType::OutOfMemory,