[core, hal, types] Clarify wgpu_hal's bounds check promises.

CHANGELOG.md

@@ -96,6 +96,7 @@ By @teoxoy [#6134](https://github.com/gfx-rs/wgpu/pull/6134).
 - Fix crash when dropping the surface after the device. By @wumpf in [#6052](https://github.com/gfx-rs/wgpu/pull/6052)
 - Fix error message that is thrown in create_render_pass to no longer say `compute_pass`. By @matthew-wong1 [#6041](https://github.com/gfx-rs/wgpu/pull/6041)
 - Add `VideoFrame` to `ExternalImageSource` enum. By @jprochazk in [#6170](https://github.com/gfx-rs/wgpu/pull/6170)
+- Document `wgpu_hal` bounds-checking promises, and adapt `wgpu_core`'s lazy initialization logic to the slightly weaker-than-expected guarantees. By @jimblandy in [#6201](https://github.com/gfx-rs/wgpu/pull/6201)
 
 #### GLES / OpenGL
 

wgpu-core/src/binding_model.rs

@@ -884,6 +884,16 @@ pub(crate) fn buffer_binding_type_alignment(
     }
 }
 
+pub(crate) fn buffer_binding_type_bounds_check_alignment(
+    alignments: &hal::Alignments,
+    binding_type: wgt::BufferBindingType,
+) -> wgt::BufferAddress {
+    match binding_type {
+        wgt::BufferBindingType::Uniform => alignments.uniform_bounds_check_alignment.get(),
+        wgt::BufferBindingType::Storage { .. } => wgt::COPY_BUFFER_ALIGNMENT,
+    }
+}
+
 #[derive(Debug)]
 pub struct BindGroup {
     pub(crate) raw: Snatchable<Box<dyn hal::DynBindGroup>>,

wgpu-core/src/device/global.rs

@@ -795,14 +795,14 @@ impl Global {
                 Err(..) => break 'error binding_model::CreateBindGroupError::InvalidLayout,
             };
 
-            fn map_entry<'a>(
+            fn resolve_entry<'a>(
                 e: &BindGroupEntry<'a>,
                 buffer_storage: &Storage<resource::Buffer>,
                 sampler_storage: &Storage<resource::Sampler>,
                 texture_view_storage: &Storage<resource::TextureView>,
             ) -> Result<ResolvedBindGroupEntry<'a>, binding_model::CreateBindGroupError>
             {
-                let map_buffer = |bb: &BufferBinding| {
+                let resolve_buffer = |bb: &BufferBinding| {
                     buffer_storage
                         .get_owned(bb.buffer_id)
                         .map(|buffer| ResolvedBufferBinding {
@@ -814,42 +814,45 @@ impl Global {
                             binding_model::CreateBindGroupError::InvalidBufferId(bb.buffer_id)
                         })
                 };
-                let map_sampler = |id: &id::SamplerId| {
+                let resolve_sampler = |id: &id::SamplerId| {
                     sampler_storage
                         .get_owned(*id)
                         .map_err(|_| binding_model::CreateBindGroupError::InvalidSamplerId(*id))
                 };
-                let map_view = |id: &id::TextureViewId| {
+                let resolve_view = |id: &id::TextureViewId| {
                     texture_view_storage
                         .get_owned(*id)
                         .map_err(|_| binding_model::CreateBindGroupError::InvalidTextureViewId(*id))
                 };
                 let resource = match e.resource {
                     BindingResource::Buffer(ref buffer) => {
-                        ResolvedBindingResource::Buffer(map_buffer(buffer)?)
+                        ResolvedBindingResource::Buffer(resolve_buffer(buffer)?)
                     }
                     BindingResource::BufferArray(ref buffers) => {
                         let buffers = buffers
                             .iter()
-                            .map(map_buffer)
+                            .map(resolve_buffer)
                             .collect::<Result<Vec<_>, _>>()?;
                         ResolvedBindingResource::BufferArray(Cow::Owned(buffers))
                     }
                     BindingResource::Sampler(ref sampler) => {
-                        ResolvedBindingResource::Sampler(map_sampler(sampler)?)
+                        ResolvedBindingResource::Sampler(resolve_sampler(sampler)?)
                     }
                     BindingResource::SamplerArray(ref samplers) => {
                         let samplers = samplers
                             .iter()
-                            .map(map_sampler)
+                            .map(resolve_sampler)
                             .collect::<Result<Vec<_>, _>>()?;
                         ResolvedBindingResource::SamplerArray(Cow::Owned(samplers))
                     }
                     BindingResource::TextureView(ref view) => {
-                        ResolvedBindingResource::TextureView(map_view(view)?)
+                        ResolvedBindingResource::TextureView(resolve_view(view)?)
                     }
                     BindingResource::TextureViewArray(ref views) => {
-                        let views = views.iter().map(map_view).collect::<Result<Vec<_>, _>>()?;
+                        let views = views
+                            .iter()
+                            .map(resolve_view)
+                            .collect::<Result<Vec<_>, _>>()?;
                         ResolvedBindingResource::TextureViewArray(Cow::Owned(views))
                     }
                 };
@@ -865,7 +868,7 @@ impl Global {
                 let sampler_guard = hub.samplers.read();
                 desc.entries
                     .iter()
-                    .map(|e| map_entry(e, &buffer_guard, &sampler_guard, &texture_view_guard))
+                    .map(|e| resolve_entry(e, &buffer_guard, &sampler_guard, &texture_view_guard))
                     .collect::<Result<Vec<_>, _>>()
             };
             let entries = match entries {

wgpu-core/src/device/resource.rs

@@ -39,7 +39,9 @@ use once_cell::sync::OnceCell;
 
 use smallvec::SmallVec;
 use thiserror::Error;
-use wgt::{DeviceLostReason, TextureFormat, TextureSampleType, TextureViewDimension};
+use wgt::{
+    math::align_to, DeviceLostReason, TextureFormat, TextureSampleType, TextureViewDimension,
+};
 
 use std::{
     borrow::Cow,
@@ -1629,7 +1631,7 @@ impl Device {
 
     /// Generate information about late-validated buffer bindings for pipelines.
     //TODO: should this be combined with `get_introspection_bind_group_layouts` in some way?
-    pub(crate) fn make_late_sized_buffer_groups(
+    fn make_late_sized_buffer_groups(
         shader_binding_sizes: &FastHashMap<naga::ResourceBinding, wgt::BufferSize>,
         layout: &binding_model::PipelineLayout,
     ) -> ArrayVec<pipeline::LateSizedBufferGroup, { hal::MAX_BIND_GROUPS }> {
@@ -1881,16 +1883,15 @@ impl Device {
         Ok(bgl)
     }
 
-    pub(crate) fn create_buffer_binding<'a>(
-        self: &Arc<Self>,
+    fn create_buffer_binding<'a>(
+        &self,
         bb: &'a binding_model::ResolvedBufferBinding,
         binding: u32,
         decl: &wgt::BindGroupLayoutEntry,
         used_buffer_ranges: &mut Vec<BufferInitTrackerAction>,
         dynamic_binding_info: &mut Vec<binding_model::BindGroupDynamicBindingData>,
         late_buffer_binding_sizes: &mut FastHashMap<u32, wgt::BufferSize>,
         used: &mut BindGroupStates,
-        limits: &wgt::Limits,
         snatch_guard: &'a SnatchGuard<'a>,
     ) -> Result<hal::BufferBinding<'a, dyn hal::DynBuffer>, binding_model::CreateBindGroupError>
     {
@@ -1915,7 +1916,7 @@ impl Device {
             wgt::BufferBindingType::Uniform => (
                 wgt::BufferUsages::UNIFORM,
                 hal::BufferUses::UNIFORM,
-                limits.max_uniform_buffer_binding_size,
+                self.limits.max_uniform_buffer_binding_size,
             ),
             wgt::BufferBindingType::Storage { read_only } => (
                 wgt::BufferUsages::STORAGE,
@@ -1924,12 +1925,12 @@ impl Device {
                 } else {
                     hal::BufferUses::STORAGE_READ_WRITE
                 },
-                limits.max_storage_buffer_binding_size,
+                self.limits.max_storage_buffer_binding_size,
             ),
         };
 
         let (align, align_limit_name) =
-            binding_model::buffer_binding_type_alignment(limits, binding_ty);
+            binding_model::buffer_binding_type_alignment(&self.limits, binding_ty);
         if bb.offset % align as u64 != 0 {
             return Err(Error::UnalignedBufferOffset(
                 bb.offset,
@@ -2005,10 +2006,21 @@ impl Device {
             late_buffer_binding_sizes.insert(binding, late_size);
         }
 
+        // This was checked against the device's alignment requirements above,
+        // which should always be a multiple of `COPY_BUFFER_ALIGNMENT`.
         assert_eq!(bb.offset % wgt::COPY_BUFFER_ALIGNMENT, 0);
+
+        // `wgpu_hal` only restricts shader access to bound buffer regions with
+        // a certain resolution. For the sake of lazy initialization, round up
+        // the size of the bound range to reflect how much of the buffer is
+        // actually going to be visible to the shader.
+        let bounds_check_alignment =
+            binding_model::buffer_binding_type_bounds_check_alignment(&self.alignments, binding_ty);
+        let visible_size = align_to(bind_size, bounds_check_alignment);
+
         used_buffer_ranges.extend(buffer.initialization_status.read().create_action(
             buffer,
-            bb.offset..bb.offset + bind_size,
+            bb.offset..bb.offset + visible_size,
             MemoryInitKind::NeedsInitializedMemory,
         ));
 
@@ -2020,7 +2032,7 @@ impl Device {
     }
 
     fn create_sampler_binding<'a>(
-        self: &Arc<Self>,
+        &self,
         used: &mut BindGroupStates,
         binding: u32,
         decl: &wgt::BindGroupLayoutEntry,
@@ -2068,8 +2080,8 @@ impl Device {
         Ok(sampler.raw())
     }
 
-    pub(crate) fn create_texture_binding<'a>(
-        self: &Arc<Self>,
+    fn create_texture_binding<'a>(
+        &self,
         binding: u32,
         decl: &wgt::BindGroupLayoutEntry,
         view: &'a Arc<TextureView>,
@@ -2167,7 +2179,6 @@ impl Device {
                         &mut dynamic_binding_info,
                         &mut late_buffer_binding_sizes,
                         &mut used,
-                        &self.limits,
                         &snatch_guard,
                     )?;
 
@@ -2189,7 +2200,6 @@ impl Device {
                             &mut dynamic_binding_info,
                             &mut late_buffer_binding_sizes,
                             &mut used,
-                            &self.limits,
                             &snatch_guard,
                         )?;
                         hal_buffers.push(bb);
@@ -2325,7 +2335,7 @@ impl Device {
         Ok(bind_group)
     }
 
-    pub(crate) fn check_array_binding(
+    fn check_array_binding(
         features: wgt::Features,
         count: Option<NonZeroU32>,
         num_bindings: usize,
@@ -2358,8 +2368,8 @@ impl Device {
         Ok(())
     }
 
-    pub(crate) fn texture_use_parameters(
-        self: &Arc<Self>,
+    fn texture_use_parameters(
+        &self,
         binding: u32,
         decl: &wgt::BindGroupLayoutEntry,
         view: &TextureView,
@@ -3449,10 +3459,7 @@ impl Device {
         Ok(cache)
     }
 
-    pub(crate) fn get_texture_format_features(
-        &self,
-        format: TextureFormat,
-    ) -> wgt::TextureFormatFeatures {
+    fn get_texture_format_features(&self, format: TextureFormat) -> wgt::TextureFormatFeatures {
         // Variant of adapter.get_texture_format_features that takes device features into account
         use wgt::TextureFormatFeatureFlags as tfsc;
         let mut format_features = self.adapter.get_texture_format_features(format);
@@ -3466,7 +3473,7 @@ impl Device {
         format_features
     }
 
-    pub(crate) fn describe_format_features(
+    fn describe_format_features(
         &self,
         format: TextureFormat,
     ) -> Result<wgt::TextureFormatFeatures, MissingFeatures> {

wgpu-core/src/resource.rs

@@ -106,8 +106,8 @@ pub(crate) trait ParentDevice: Labeled {
         }
     }
 
-    fn same_device(&self, device: &Arc<Device>) -> Result<(), DeviceError> {
-        if Arc::ptr_eq(self.device(), device) {
+    fn same_device(&self, device: &Device) -> Result<(), DeviceError> {
+        if std::ptr::eq(&**self.device(), device) {
             Ok(())
         } else {
             Err(DeviceError::DeviceMismatch(Box::new(DeviceMismatch {

wgpu-hal/src/dx12/adapter.rs

@@ -524,6 +524,9 @@ impl super::Adapter {
                         Direct3D12::D3D12_TEXTURE_DATA_PITCH_ALIGNMENT as u64,
                     )
                     .unwrap(),
+                    // Direct3D correctly bounds-checks all array accesses:
+                    // https://microsoft.github.io/DirectX-Specs/d3d/archive/D3D11_3_FunctionalSpec.htm#18.6.8.2%20Device%20Memory%20Reads
+                    uniform_bounds_check_alignment: wgt::BufferSize::new(1).unwrap(),
                 },
                 downlevel,
             },

wgpu-hal/src/gles/adapter.rs

@@ -841,6 +841,16 @@ impl super::Adapter {
                 alignments: crate::Alignments {
                     buffer_copy_offset: wgt::BufferSize::new(4).unwrap(),
                     buffer_copy_pitch: wgt::BufferSize::new(4).unwrap(),
+                    // #6151: `wgpu_hal::gles` doesn't ask Naga to inject bounds
+                    // checks in GLSL, and it doesn't request extensions like
+                    // `KHR_robust_buffer_access_behavior` that would provide
+                    // them, so we can't really implement the checks promised by
+                    // [`crate::BufferBinding`].
+                    //
+                    // Since this is a pre-existing condition, for the time
+                    // being, provide 1 as the value here, to cause as little
+                    // trouble as possible.
+                    uniform_bounds_check_alignment: wgt::BufferSize::new(1).unwrap(),
                 },
             },
         })