deno: Reject zero-size bindings in createBindGroup

Author: andyleiserson

cts_runner/test.lst

@@ -33,6 +33,7 @@ webgpu:api,operation,vertex_state,correctness:setVertexBuffer_offset_and_attribu
 webgpu:api,validation,buffer,create:*
 webgpu:api,validation,buffer,destroy:*
 fails-if(dx12) webgpu:api,validation,capability_checks,limits,maxBindGroups:setBindGroup,*
+webgpu:api,validation,createBindGroup:buffer_offset_and_size_for_bind_groups_match:*
 webgpu:api,validation,createBindGroup:buffer,effective_buffer_binding_size:*
 webgpu:api,validation,createBindGroup:buffer,resource_binding_size:*
 // Fails because we coerce a size of 0 in `GPUDevice.createBindGroup(…)` to `buffer.size - offset`.

deno_webgpu/device.rs

@@ -11,7 +11,7 @@ use deno_core::v8;
 use deno_core::webidl::WebIdlInterfaceConverter;
 use deno_core::GarbageCollected;
 use deno_error::JsErrorBox;
-use wgpu_core::binding_model::BindingResource;
+use wgpu_core::binding_model::{BindingResource, CreateBindGroupError};
 use wgpu_core::pipeline::ProgrammableStageDescriptor;
 use wgpu_types::BindingType;
 
@@ -404,49 +404,73 @@ impl GPUDevice {
     &self,
     #[webidl] descriptor: super::bind_group::GPUBindGroupDescriptor,
   ) -> GPUBindGroup {
-    let entries = descriptor
-      .entries
-      .into_iter()
-      .map(|entry| wgpu_core::binding_model::BindGroupEntry {
-        binding: entry.binding,
-        resource: match entry.resource {
-          GPUBindingResource::Sampler(sampler) => {
-            BindingResource::Sampler(sampler.id)
-          }
-          GPUBindingResource::Texture(texture) => {
-            BindingResource::TextureView(texture.default_view_id())
-          }
-          GPUBindingResource::TextureView(texture_view) => {
-            BindingResource::TextureView(texture_view.id)
-          }
-          GPUBindingResource::Buffer(buffer) => {
-            BindingResource::Buffer(wgpu_core::binding_model::BufferBinding {
-              buffer: buffer.id,
-              offset: 0,
-              size: NonZeroU64::new(buffer.size),
-            })
-          }
-          GPUBindingResource::BufferBinding(buffer_binding) => {
-            BindingResource::Buffer(wgpu_core::binding_model::BufferBinding {
-              buffer: buffer_binding.buffer.id,
-              offset: buffer_binding.offset,
-              size: buffer_binding.size.and_then(NonZeroU64::new),
-            })
-          }
-        },
-      })
-      .collect::<Vec<_>>();
+    let mut entries = Vec::with_capacity(descriptor.entries.len());
+    let (id, err) = 'error: {
+      for entry in descriptor.entries.iter() {
+        entries.push(wgpu_core::binding_model::BindGroupEntry {
+          binding: entry.binding,
+          resource: match &entry.resource {
+            GPUBindingResource::Sampler(sampler) => {
+              BindingResource::Sampler(sampler.id)
+            }
+            GPUBindingResource::Texture(texture) => {
+              BindingResource::TextureView(texture.default_view_id())
+            }
+            GPUBindingResource::TextureView(texture_view) => {
+              BindingResource::TextureView(texture_view.id)
+            }
+            GPUBindingResource::Buffer(buffer) => {
+              BindingResource::Buffer(wgpu_core::binding_model::BufferBinding {
+                buffer: buffer.id,
+                offset: 0,
+                size: NonZeroU64::new(buffer.size),
+              })
+            }
+            GPUBindingResource::BufferBinding(buffer_binding) => {
+              let size = buffer_binding.size.map(NonZeroU64::new);
+              match size {
+                None | Some(Some(_)) => {
+                  // Size non-zero or omitted
+                  BindingResource::Buffer(
+                    wgpu_core::binding_model::BufferBinding {
+                      buffer: buffer_binding.buffer.id,
+                      offset: buffer_binding.offset,
+                      size: size.unwrap_or_default(),
+                    },
+                  )
+                }
+                Some(None) => {
+                  // Explicit zero size. Not representable in wgpu API.
+                  let id = self.instance.create_bind_group_error(
+                    None,
+                    crate::transform_label(descriptor.label.clone()),
+                  );
+                  let error_ident =
+                    wgpu_core::resource::ResourceErrorIdent::new(
+                      "Buffer",
+                      buffer_binding.buffer.label.clone(),
+                    );
+                  break 'error (
+                    id,
+                    Some(CreateBindGroupError::BindingZeroSize(error_ident)),
+                  );
+                }
+              }
+            }
+          },
+        });
+      }
 
-    let wgpu_descriptor = wgpu_core::binding_model::BindGroupDescriptor {
-      label: crate::transform_label(descriptor.label.clone()),
-      layout: descriptor.layout.id,
-      entries: Cow::Owned(entries),
-    };
+      let wgpu_descriptor = wgpu_core::binding_model::BindGroupDescriptor {
+        label: crate::transform_label(descriptor.label.clone()),
+        layout: descriptor.layout.id,
+        entries: Cow::Owned(entries),
+      };
 
-    let (id, err) =
       self
         .instance
-        .device_create_bind_group(self.id, &wgpu_descriptor, None);
+        .device_create_bind_group(self.id, &wgpu_descriptor, None)
+    };
 
     self.error_handler.push_error(err);
 

wgpu-core/src/device/global.rs

@@ -232,6 +232,15 @@ impl Global {
         fid.assign(Fallible::Invalid(Arc::new(label.to_string())));
     }
 
+    pub fn create_bind_group_error(
+        &self,
+        id_in: Option<id::BindGroupId>,
+        label: Option<Cow<'_, str>>,
+    ) -> id::BindGroupId {
+        let fid = self.hub.bind_groups.prepare(id_in);
+        fid.assign(Fallible::Invalid(Arc::new(label.to_string())))
+    }
+
     pub fn buffer_destroy(&self, buffer_id: id::BufferId) {
         profiling::scope!("Buffer::destroy");
         api_log!("Buffer::destroy {buffer_id:?}");

wgpu-core/src/resource.rs

@@ -91,6 +91,17 @@ impl fmt::Display for ResourceErrorIdent {
     }
 }
 
+impl ResourceErrorIdent {
+    /// Prefer [`Labeled::error_ident`] when possible.
+    #[doc(hidden)]
+    pub fn new(r#type: &'static str, label: String) -> Self {
+        Self {
+            r#type: Cow::Borrowed(r#type),
+            label,
+        }
+    }
+}
+
 pub trait ParentDevice: Labeled {
     fn device(&self) -> &Arc<Device>;