Bug 1772557: Assign WebGPU RenderBundles ids even if encoding fails. r=webgpu-reviewers,ErichDonGubler

In `RenderBundleEncoder::Finish`, never construct RenderBundles with an id of zero, as this will cause a panic in the GPU process when we deserialize command buffers that refer to such bundles. Instead, assign the invalid RenderBundle an id, and report to the GPU process that this id is associated with an error. Differential Revision: https://phabricator.services.mozilla.com/D177077
2023-06-14 23:33:29 +00:00 · 2023-06-14 23:33:29 +00:00 · 8820a3ea9e
--- a/dom/webgpu/ObjectModel.h
+++ b/dom/webgpu/ObjectModel.h
@ -27,9 +27,6 @@ class ChildOf {
 };

 class ObjectBase : public nsWrapperCache {
- private:
-  nsString mLabel;
-
 protected:
  virtual ~ObjectBase() = default;

@ -55,6 +52,10 @@ class ObjectBase : public nsWrapperCache {
  void SetLabel(const nsAString& aLabel);

  auto CLabel() const { return NS_ConvertUTF16toUTF8(mLabel); }
+
+ protected:
+  // Object label, initialized from GPUObjectDescriptorBase.label.
+  nsString mLabel;
 };

 }  // namespace mozilla::webgpu
--- a/dom/webgpu/RenderBundleEncoder.cpp
+++ b/dom/webgpu/RenderBundleEncoder.cpp
@ -188,6 +188,11 @@ already_AddRefed<RenderBundle> RenderBundleEncoder::Finish(
      MOZ_ASSERT(encoder);
      id = bridge->RenderBundleEncoderFinish(*encoder, mParent->mId, aDesc);
    }
+  } else {
+    auto bridge = mParent->GetBridge();
+    if (bridge && bridge->CanSend()) {
+      id = bridge->RenderBundleEncoderFinishError(mParent->mId, mLabel);
+    }
  }
  RefPtr<RenderBundle> bundle = new RenderBundle(mParent, id);
  return bundle.forget();
--- a/dom/webgpu/ipc/WebGPUChild.cpp
+++ b/dom/webgpu/ipc/WebGPUChild.cpp
@ -4,11 +4,13 @@
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

 #include "WebGPUChild.h"
+
 #include "js/RootingAPI.h"
 #include "js/String.h"
 #include "js/TypeDecls.h"
 #include "js/Value.h"
 #include "js/Warnings.h"  // JS::WarnUTF8
+#include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/EnumTypeTraits.h"
 #include "mozilla/dom/Console.h"
@ -27,6 +29,8 @@
 #include "mozilla/ipc/RawShmem.h"
 #include "nsGlobalWindowInner.h"

+#include <utility>
+
 namespace mozilla::webgpu {

 NS_IMPL_CYCLE_COLLECTION(WebGPUChild)
@ -456,6 +460,21 @@ RawId WebGPUChild::RenderBundleEncoderFinish(
  return id;
 }

+RawId WebGPUChild::RenderBundleEncoderFinishError(RawId aDeviceId,
+                                                  const nsString& aLabel) {
+  webgpu::StringHelper label(aLabel);
+
+  ipc::ByteBuf bb;
+  RawId id = ffi::wgpu_client_create_render_bundle_error(
+      mClient.get(), aDeviceId, label.Get(), ToFFI(&bb));
+
+  if (!SendDeviceAction(aDeviceId, std::move(bb))) {
+    MOZ_CRASH("IPC failure");
+  }
+
+  return id;
+}
+
 RawId WebGPUChild::DeviceCreateBindGroupLayout(
    RawId aSelfId, const dom::GPUBindGroupLayoutDescriptor& aDesc) {
  struct OptionalData {
--- a/dom/webgpu/ipc/WebGPUChild.h
+++ b/dom/webgpu/ipc/WebGPUChild.h
@ -79,6 +79,7 @@ class WebGPUChild final : public PWebGPUChild, public SupportsWeakPtr {
  RawId RenderBundleEncoderFinish(ffi::WGPURenderBundleEncoder& aEncoder,
                                  RawId aDeviceId,
                                  const dom::GPURenderBundleDescriptor& aDesc);
+  RawId RenderBundleEncoderFinishError(RawId aDeviceId, const nsString& aLabel);
  RawId DeviceCreateBindGroupLayout(
      RawId aSelfId, const dom::GPUBindGroupLayoutDescriptor& aDesc);
  RawId DeviceCreatePipelineLayout(
--- a/dom/webgpu/mochitest/mochitest.ini
+++ b/dom/webgpu/mochitest/mochitest.ini
@ -40,3 +40,5 @@ fail-if = (os == 'linux' && os_version == '18.04') || (os == 'win' && os_version
 [test_submit_render_empty.worker.html]
 skip-if = true # Bug 1818379 - no webgpu in worker scopes, see bug 1808820
 fail-if = (os == 'linux' && os_version == '18.04') || (os == 'win' && os_version == '6.1') || (os == 'mac')
+[test_too_many_color_attachments.html]
+fail-if = (os == 'linux' && os_version == '18.04') || (os == 'win' && os_version == '6.1') || (os == 'mac')
--- a/dom/webgpu/mochitest/test_too_many_color_attachments.html
+++ b/dom/webgpu/mochitest/test_too_many_color_attachments.html
@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+    <title>Test for Bug 1772557</title>
+    <script src="/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" href="/tests/SimpleTest/test.css" />
+    <script>
+      ok(
+        SpecialPowers.getBoolPref("dom.webgpu.enabled"),
+        "Pref should be enabled."
+      );
+
+      add_task(too_many_color_attachments);
+
+      async function too_many_color_attachments() {
+        const adapter = await navigator.gpu.requestAdapter({});
+        const device = await adapter.requestDevice({});
+        device.pushErrorScope("validation");
+        const texture = device.createTexture({
+          size: [60, 37, 238],
+          format: "rg16sint",
+          usage: GPUTextureUsage.RENDER_ATTACHMENT,
+        });
+        const view = texture.createView({});
+        const encoder = device.createRenderBundleEncoder({
+          colorFormats: [
+            "rg8uint",
+            "rg8uint",
+            "rg8uint",
+            "rg8uint",
+            "rg8uint",
+            "rg8uint",
+            "rg8uint",
+            "rg8uint",
+            "rg8uint",
+          ],
+        });
+        const renderBundle = encoder.finish({});
+        const commandEncoder = device.createCommandEncoder({});
+        const renderPassEncoder = commandEncoder.beginRenderPass({
+          colorAttachments: [
+            {
+              view,
+              loadOp: "load",
+              storeOp: "store",
+            },
+          ],
+        });
+        renderPassEncoder.executeBundles([renderBundle]);
+        renderPassEncoder.end();
+        await device.popErrorScope();
+        ok(true, "test completed without crashing");
+      }
+    </script>
+  </head>
+  <body>
+    <a
+      target="_blank"
+      href="https://bugzilla.mozilla.org/show_bug.cgi?id=1772557"
+      >Mozilla Bug 1772557</a
+    >
+    <p id="display"></p>
+    <div id="content" style="display: none"></div>
+    <pre id="test"></pre>
+  </body>
+</html>
--- a/gfx/wgpu_bindings/src/client.rs
+++ b/gfx/wgpu_bindings/src/client.rs
@ -700,6 +700,28 @@ pub unsafe extern "C" fn wgpu_client_create_render_bundle(
    id
 }

+#[no_mangle]
+pub unsafe extern "C" fn wgpu_client_create_render_bundle_error(
+    client: &Client,
+    device_id: id::DeviceId,
+    label: Option<&nsACString>,
+    bb: &mut ByteBuf,
+) -> id::RenderBundleId {
+    let label = wgpu_string(label);
+
+    let backend = device_id.backend();
+    let id = client
+        .identities
+        .lock()
+        .select(backend)
+        .render_bundles
+        .alloc(backend);
+
+    let action = DeviceAction::CreateRenderBundleError(id, label);
+    *bb = make_byte_buf(&action);
+    id
+}
+
 #[repr(C)]
 pub struct ComputePassDescriptor<'a> {
    pub label: Option<&'a nsACString>,
--- a/gfx/wgpu_bindings/src/lib.rs
+++ b/gfx/wgpu_bindings/src/lib.rs
@ -147,6 +147,10 @@ enum DeviceAction<'a> {
        wgc::command::RenderBundleEncoder,
        wgc::command::RenderBundleDescriptor<'a>,
    ),
+    CreateRenderBundleError(
+        id::RenderBundleId,
+        wgc::Label<'a>,
+    ),
    CreateCommandEncoder(
        id::CommandEncoderId,
        wgt::CommandEncoderDescriptor<wgc::Label<'a>>,
--- a/gfx/wgpu_bindings/src/server.rs
+++ b/gfx/wgpu_bindings/src/server.rs
@ -533,6 +533,9 @@ impl Global {
                    error_buf.init(err);
                }
            }
+            DeviceAction::CreateRenderBundleError(buffer_id, label) => {
+                self.create_render_bundle_error::<A>(buffer_id, label);
+            }
            DeviceAction::CreateCommandEncoder(id, desc) => {
                let (_, error) = self.device_create_command_encoder::<A>(self_id, &desc, id);
                if let Some(err) = error {