Validate binding ranges against buffer size

This commit is contained in:
Andy Leiserson 2025-06-26 17:45:07 -07:00 committed by Jim Blandy
parent 3d0fe3a003
commit ef428fcab8
22 changed files with 362 additions and 212 deletions

View File

@ -94,8 +94,39 @@ impl WebGpuError for CreateBindGroupLayoutError {
}
}
//TODO: refactor this to move out `enum BindingError`.
#[derive(Clone, Debug, Error)]
#[non_exhaustive]
pub enum BindingError {
#[error(transparent)]
DestroyedResource(#[from] DestroyedResourceError),
#[error("Buffer {buffer}: Binding with size {binding_size} at offset {offset} would overflow buffer size of {buffer_size}")]
BindingRangeTooLarge {
buffer: ResourceErrorIdent,
offset: wgt::BufferAddress,
binding_size: u64,
buffer_size: u64,
},
#[error("Buffer {buffer}: Binding offset {offset} is greater than or equal to buffer size {buffer_size}")]
BindingOffsetTooLarge {
buffer: ResourceErrorIdent,
offset: wgt::BufferAddress,
buffer_size: u64,
},
}
impl WebGpuError for BindingError {
fn webgpu_error_type(&self) -> ErrorType {
match self {
Self::DestroyedResource(e) => e.webgpu_error_type(),
Self::BindingRangeTooLarge { .. } | Self::BindingOffsetTooLarge { .. } => {
ErrorType::Validation
}
}
}
}
// TODO: there may be additional variants here that can be extracted into
// `BindingError`.
#[derive(Clone, Debug, Error)]
#[non_exhaustive]
pub enum CreateBindGroupError {
@ -103,6 +134,8 @@ pub enum CreateBindGroupError {
Device(#[from] DeviceError),
#[error(transparent)]
DestroyedResource(#[from] DestroyedResourceError),
#[error(transparent)]
BindingError(#[from] BindingError),
#[error(
"Binding count declared with at most {expected} items, but {actual} items were provided"
)]
@ -113,12 +146,6 @@ pub enum CreateBindGroupError {
BindingArrayLengthMismatch { actual: usize, expected: usize },
#[error("Array binding provided zero elements")]
BindingArrayZeroLength,
#[error("The bound range {range:?} of {buffer} overflows its size ({size})")]
BindingRangeTooLarge {
buffer: ResourceErrorIdent,
range: Range<wgt::BufferAddress>,
size: u64,
},
#[error("Binding size {actual} of {buffer} is less than minimum {min}")]
BindingSizeTooSmall {
buffer: ResourceErrorIdent,
@ -233,6 +260,7 @@ impl WebGpuError for CreateBindGroupError {
let e: &dyn WebGpuError = match self {
Self::Device(e) => e,
Self::DestroyedResource(e) => e,
Self::BindingError(e) => e,
Self::MissingBufferUsage(e) => e,
Self::MissingTextureUsage(e) => e,
Self::ResourceUsageCompatibility(e) => e,
@ -240,7 +268,6 @@ impl WebGpuError for CreateBindGroupError {
Self::BindingArrayPartialLengthMismatch { .. }
| Self::BindingArrayLengthMismatch { .. }
| Self::BindingArrayZeroLength
| Self::BindingRangeTooLarge { .. }
| Self::BindingSizeTooSmall { .. }
| Self::BindingsNumMismatch { .. }
| Self::BindingZeroSize(_)

View File

@ -602,6 +602,7 @@ fn set_pipeline(
Ok(())
}
// This function is duplicative of `render::set_index_buffer`.
fn set_index_buffer(
state: &mut State,
buffer_guard: &crate::storage::Storage<Fallible<Buffer>>,
@ -620,21 +621,20 @@ fn set_index_buffer(
buffer.same_device(&state.device)?;
buffer.check_usage(wgt::BufferUsages::INDEX)?;
let end = match size {
Some(s) => offset + s.get(),
None => buffer.size,
};
let end = buffer.resolve_binding_size(offset, size)?;
state
.buffer_memory_init_actions
.extend(buffer.initialization_status.read().create_action(
&buffer,
offset..end,
offset..end.get(),
MemoryInitKind::NeedsInitializedMemory,
));
state.set_index_buffer(buffer, index_format, offset..end);
state.set_index_buffer(buffer, index_format, offset..end.get());
Ok(())
}
// This function is duplicative of `render::set_vertex_buffer`.
fn set_vertex_buffer(
state: &mut State,
buffer_guard: &crate::storage::Storage<Fallible<Buffer>>,
@ -662,18 +662,16 @@ fn set_vertex_buffer(
buffer.same_device(&state.device)?;
buffer.check_usage(wgt::BufferUsages::VERTEX)?;
let end = match size {
Some(s) => offset + s.get(),
None => buffer.size,
};
let end = buffer.resolve_binding_size(offset, size)?;
state
.buffer_memory_init_actions
.extend(buffer.initialization_status.read().create_action(
&buffer,
offset..end,
offset..end.get(),
MemoryInitKind::NeedsInitializedMemory,
));
state.vertex[slot as usize] = Some(VertexState::new(buffer, offset..end));
state.vertex[slot as usize] = Some(VertexState::new(buffer, offset..end.get()));
Ok(())
}
@ -965,10 +963,14 @@ impl RenderBundle {
size,
} => {
let buffer = buffer.try_raw(snatch_guard)?;
let bb = hal::BufferBinding {
buffer,
offset: *offset,
size: *size,
let bb = unsafe {
// SAFETY: The binding size was checked against the buffer size
// in `set_index_buffer` and again in `IndexState::flush`.
hal::BufferBinding::new_unchecked(
buffer,
*offset,
size.expect("size was resolved in `RenderBundleEncoder::finish`"),
)
};
unsafe { raw.set_index_buffer(bb, *index_format) };
}
@ -979,10 +981,14 @@ impl RenderBundle {
size,
} => {
let buffer = buffer.try_raw(snatch_guard)?;
let bb = hal::BufferBinding {
buffer,
offset: *offset,
size: *size,
let bb = unsafe {
// SAFETY: The binding size was checked against the buffer size
// in `set_vertex_buffer` and again in `VertexState::flush`.
hal::BufferBinding::new_unchecked(
buffer,
*offset,
size.expect("size was resolved in `RenderBundleEncoder::finish`"),
)
};
unsafe { raw.set_vertex_buffer(*slot, bb) };
}
@ -1131,6 +1137,9 @@ crate::impl_trackable!(RenderBundle);
/// [`RenderBundleEncoder::finish`] records the currently set index buffer here,
/// and calls [`State::flush_index`] before any indexed draw command to produce
/// a `SetIndexBuffer` command if one is necessary.
///
/// Binding ranges must be validated against the size of the buffer before
/// being stored in `IndexState`.
#[derive(Debug)]
struct IndexState {
buffer: Arc<Buffer>,
@ -1152,13 +1161,24 @@ impl IndexState {
/// Generate a `SetIndexBuffer` command to prepare for an indexed draw
/// command, if needed.
fn flush(&mut self) -> Option<ArcRenderCommand> {
// This was all checked before, but let's check again just in case.
let binding_size = self
.range
.end
.checked_sub(self.range.start)
.and_then(wgt::BufferSize::new);
assert!(
self.range.end <= self.buffer.size && binding_size.is_some(),
"index buffer range must have non-zero size and be contained in buffer",
);
if self.is_dirty {
self.is_dirty = false;
Some(ArcRenderCommand::SetIndexBuffer {
buffer: self.buffer.clone(),
index_format: self.format,
offset: self.range.start,
size: wgt::BufferSize::new(self.range.end - self.range.start),
size: binding_size,
})
} else {
None
@ -1174,6 +1194,9 @@ impl IndexState {
/// calls this type's [`flush`] method just before any draw command to
/// produce a `SetVertexBuffer` commands if one is necessary.
///
/// Binding ranges must be validated against the size of the buffer before
/// being stored in `VertexState`.
///
/// [`flush`]: IndexState::flush
#[derive(Debug)]
struct VertexState {
@ -1183,6 +1206,9 @@ struct VertexState {
}
impl VertexState {
/// Create a new `VertexState`.
///
/// The `range` must be contained within `buffer`.
fn new(buffer: Arc<Buffer>, range: Range<wgt::BufferAddress>) -> Self {
Self {
buffer,
@ -1195,13 +1221,24 @@ impl VertexState {
///
/// `slot` is the index of the vertex buffer slot that `self` tracks.
fn flush(&mut self, slot: u32) -> Option<ArcRenderCommand> {
// This was all checked before, but let's check again just in case.
let binding_size = self
.range
.end
.checked_sub(self.range.start)
.and_then(wgt::BufferSize::new);
assert!(
self.range.end <= self.buffer.size && binding_size.is_some(),
"vertex buffer range must have non-zero size and be contained in buffer",
);
if self.is_dirty {
self.is_dirty = false;
Some(ArcRenderCommand::SetVertexBuffer {
slot,
buffer: self.buffer.clone(),
offset: self.range.start,
size: wgt::BufferSize::new(self.range.end - self.range.start),
size: binding_size,
})
} else {
None

View File

@ -7,7 +7,7 @@ use wgt::error::{ErrorType, WebGpuError};
use super::bind::BinderError;
use crate::command::pass;
use crate::{
binding_model::{LateMinBufferBindingSizeMismatch, PushConstantUploadError},
binding_model::{BindingError, LateMinBufferBindingSizeMismatch, PushConstantUploadError},
resource::{
DestroyedResourceError, MissingBufferUsageError, MissingTextureUsageError,
ResourceErrorIdent,
@ -89,6 +89,8 @@ pub enum RenderCommandError {
MissingTextureUsage(#[from] MissingTextureUsageError),
#[error(transparent)]
PushConstants(#[from] PushConstantUploadError),
#[error(transparent)]
BindingError(#[from] BindingError),
#[error("Viewport size {{ w: {w}, h: {h} }} greater than device's requested `max_texture_dimension_2d` limit {max}, or less than zero")]
InvalidViewportRectSize { w: f32, h: f32, max: u32 },
#[error("Viewport has invalid rect {rect:?} for device's requested `max_texture_dimension_2d` limit; Origin less than -2 * `max_texture_dimension_2d` ({min}), or rect extends past 2 * `max_texture_dimension_2d` - 1 ({max})")]
@ -110,6 +112,7 @@ impl WebGpuError for RenderCommandError {
Self::MissingBufferUsage(e) => e,
Self::MissingTextureUsage(e) => e,
Self::PushConstants(e) => e,
Self::BindingError(e) => e,
Self::BindGroupIndexOutOfRange { .. }
| Self::VertexBufferIndexOutOfRange { .. }

View File

@ -1,5 +1,5 @@
use alloc::{borrow::Cow, sync::Arc, vec::Vec};
use core::{fmt, num::NonZeroU32, ops::Range, str};
use core::{fmt, num::NonZeroU32, str};
use arrayvec::ArrayVec;
use thiserror::Error;
@ -356,13 +356,17 @@ struct IndexState {
}
impl IndexState {
fn update_buffer(&mut self, range: Range<BufferAddress>, format: IndexFormat) {
fn update_buffer<B: hal::DynBuffer + ?Sized>(
&mut self,
binding: &hal::BufferBinding<'_, B>,
format: IndexFormat,
) {
self.buffer_format = Some(format);
let shift = match format {
IndexFormat::Uint16 => 1,
IndexFormat::Uint32 => 2,
};
self.limit = (range.end - range.start) >> shift;
self.limit = binding.size.get() >> shift;
}
fn reset(&mut self) {
@ -2322,6 +2326,7 @@ fn set_pipeline(
Ok(())
}
// This function is duplicative of `bundle::set_index_buffer`.
fn set_index_buffer(
state: &mut State,
cmd_buf: &Arc<CommandBuffer>,
@ -2341,33 +2346,27 @@ fn set_index_buffer(
buffer.same_device_as(cmd_buf.as_ref())?;
buffer.check_usage(BufferUsages::INDEX)?;
let buf_raw = buffer.try_raw(state.general.snatch_guard)?;
let end = match size {
Some(s) => offset + s.get(),
None => buffer.size,
};
state.index.update_buffer(offset..end, index_format);
let binding = buffer
.binding(offset, size, state.general.snatch_guard)
.map_err(RenderCommandError::from)?;
state.index.update_buffer(&binding, index_format);
state.general.buffer_memory_init_actions.extend(
buffer.initialization_status.read().create_action(
&buffer,
offset..end,
offset..(offset + binding.size.get()),
MemoryInitKind::NeedsInitializedMemory,
),
);
let bb = hal::BufferBinding {
buffer: buf_raw,
offset,
size,
};
unsafe {
hal::DynCommandEncoder::set_index_buffer(state.general.raw_encoder, bb, index_format);
hal::DynCommandEncoder::set_index_buffer(state.general.raw_encoder, binding, index_format);
}
Ok(())
}
// This function is duplicative of `render::set_vertex_buffer`.
fn set_vertex_buffer(
state: &mut State,
cmd_buf: &Arc<CommandBuffer>,
@ -2399,30 +2398,22 @@ fn set_vertex_buffer(
}
buffer.check_usage(BufferUsages::VERTEX)?;
let buf_raw = buffer.try_raw(state.general.snatch_guard)?;
//TODO: where are we checking that the offset is in bound?
let buffer_size = match size {
Some(s) => s.get(),
None => buffer.size - offset,
};
state.vertex.buffer_sizes[slot as usize] = Some(buffer_size);
let binding = buffer
.binding(offset, size, state.general.snatch_guard)
.map_err(RenderCommandError::from)?;
state.vertex.buffer_sizes[slot as usize] = Some(binding.size.get());
state.general.buffer_memory_init_actions.extend(
buffer.initialization_status.read().create_action(
&buffer,
offset..(offset + buffer_size),
offset..(offset + binding.size.get()),
MemoryInitKind::NeedsInitializedMemory,
),
);
let bb = hal::BufferBinding {
buffer: buf_raw,
offset,
size,
};
unsafe {
hal::DynCommandEncoder::set_vertex_buffer(state.general.raw_encoder, slot, bb);
hal::DynCommandEncoder::set_vertex_buffer(state.general.raw_encoder, slot, binding);
}
if let Some(pipeline) = state.pipeline.as_ref() {
state.vertex.update_limits(&pipeline.vertex_steps);

View File

@ -392,6 +392,17 @@ impl RenderCommand {
}
/// Equivalent to `RenderCommand` with the Ids resolved into resource Arcs.
///
/// In a render pass, commands are stored in this format between when they are
/// added to the pass, and when the pass is `end()`ed and the commands are
/// replayed to the HAL encoder. Validation occurs when the pass is ended, which
/// means that parameters stored in an `ArcRenderCommand` for a pass operation
/// have generally not been validated.
///
/// In a render bundle, commands are stored in this format between when the bundle
/// is `finish()`ed and when the bundle is executed. Validation occurs when the
/// bundle is finished, which means that parameters stored in an `ArcRenderCommand`
/// for a render bundle operation must have been validated.
#[doc(hidden)]
#[derive(Clone, Debug)]
pub enum ArcRenderCommand {
@ -405,12 +416,22 @@ pub enum ArcRenderCommand {
buffer: Arc<Buffer>,
index_format: wgt::IndexFormat,
offset: BufferAddress,
// For a render pass, this reflects the argument passed by the
// application, which may be `None`. For a render bundle, this reflects
// the validated size of the binding, and will be populated even in the
// case that the application omitted the size.
size: Option<BufferSize>,
},
SetVertexBuffer {
slot: u32,
buffer: Arc<Buffer>,
offset: BufferAddress,
// For a render pass, this reflects the argument passed by the
// application, which may be `None`. For a render bundle, this reflects
// the validated size of the binding, and will be populated even in the
// case that the application omitted the size.
size: Option<BufferSize>,
},
SetBlendConstant(Color),

View File

@ -383,6 +383,7 @@ impl Global {
/// - `hal_buffer` must be created from `device_id` corresponding raw handle.
/// - `hal_buffer` must be created respecting `desc`
/// - `hal_buffer` must be initialized
/// - `hal_buffer` must not have zero size.
pub unsafe fn create_buffer_from_hal<A: HalApi>(
&self,
hal_buffer: A::Buffer,
@ -404,7 +405,7 @@ impl Global {
trace.add(trace::Action::CreateBuffer(fid.id(), desc.clone()));
}
let (buffer, err) = device.create_buffer_from_hal(Box::new(hal_buffer), desc);
let (buffer, err) = unsafe { device.create_buffer_from_hal(Box::new(hal_buffer), desc) };
let id = fid.assign(buffer);
api_log!("Device::create_buffer -> {id:?}");

View File

@ -702,7 +702,8 @@ impl Device {
let buffer = unsafe { self.raw().create_buffer(&hal_desc) }
.map_err(|e| self.handle_hal_error_with_nonfatal_oom(e))?;
let timestamp_normalization_bind_group = Snatchable::new(
let timestamp_normalization_bind_group = Snatchable::new(unsafe {
// SAFETY: The size passed here must not overflow the buffer.
self.timestamp_normalizer
.get()
.unwrap()
@ -710,10 +711,10 @@ impl Device {
self,
&*buffer,
desc.label.as_deref(),
desc.size,
wgt::BufferSize::new(hal_desc.size).unwrap(),
desc.usage,
)?,
);
)
}?);
let indirect_validation_bind_groups =
self.create_indirect_validation_bind_groups(buffer.as_ref(), desc.size, desc.usage)?;
@ -809,28 +810,36 @@ impl Device {
Ok(texture)
}
pub(crate) fn create_buffer_from_hal(
/// # Safety
///
/// - `hal_buffer` must have been created on this device.
/// - `hal_buffer` must have been created respecting `desc` (in particular, the size).
/// - `hal_buffer` must be initialized.
/// - `hal_buffer` must not have zero size.
pub(crate) unsafe fn create_buffer_from_hal(
self: &Arc<Self>,
hal_buffer: Box<dyn hal::DynBuffer>,
desc: &resource::BufferDescriptor,
) -> (Fallible<Buffer>, Option<resource::CreateBufferError>) {
let timestamp_normalization_bind_group = match self
.timestamp_normalizer
.get()
.unwrap()
.create_normalization_bind_group(
self,
&*hal_buffer,
desc.label.as_deref(),
desc.size,
desc.usage,
) {
Ok(bg) => Snatchable::new(bg),
Err(e) => {
return (
Fallible::Invalid(Arc::new(desc.label.to_string())),
Some(e.into()),
)
let timestamp_normalization_bind_group = unsafe {
match self
.timestamp_normalizer
.get()
.unwrap()
.create_normalization_bind_group(
self,
&*hal_buffer,
desc.label.as_deref(),
wgt::BufferSize::new(desc.size).unwrap(),
desc.usage,
) {
Ok(bg) => Snatchable::new(bg),
Err(e) => {
return (
Fallible::Invalid(Arc::new(desc.label.to_string())),
Some(e.into()),
)
}
}
};
@ -2187,31 +2196,9 @@ impl Device {
buffer.same_device(self)?;
buffer.check_usage(pub_usage)?;
let raw_buffer = buffer.try_raw(snatch_guard)?;
let (bind_size, bind_end) = match bb.size {
Some(size) => {
let end = bb.offset + size.get();
if end > buffer.size {
return Err(Error::BindingRangeTooLarge {
buffer: buffer.error_ident(),
range: bb.offset..end,
size: buffer.size,
});
}
(size.get(), end)
}
None => {
if buffer.size < bb.offset {
return Err(Error::BindingRangeTooLarge {
buffer: buffer.error_ident(),
range: bb.offset..bb.offset,
size: buffer.size,
});
}
(buffer.size - bb.offset, buffer.size)
}
};
let bb = buffer.binding(bb.offset, bb.size, snatch_guard)?;
let bind_size = bb.size.get();
if bind_size > range_limit as u64 {
return Err(Error::BufferRangeTooLarge {
@ -2226,8 +2213,8 @@ impl Device {
dynamic_binding_info.push(binding_model::BindGroupDynamicBindingData {
binding_idx: binding,
buffer_size: buffer.size,
binding_range: bb.offset..bind_end,
maximum_dynamic_offset: buffer.size - bind_end,
binding_range: bb.offset..bb.offset + bind_size,
maximum_dynamic_offset: buffer.size - bb.offset - bind_size,
binding_type: binding_ty,
});
}
@ -2265,11 +2252,7 @@ impl Device {
MemoryInitKind::NeedsInitializedMemory,
));
Ok(hal::BufferBinding {
buffer: raw_buffer,
offset: bb.offset,
size: bb.size,
})
Ok(bb)
}
fn create_sampler_binding<'a>(

View File

@ -232,10 +232,9 @@ impl Dispatch {
resource_index: 0,
count: 1,
}],
buffers: &[hal::BufferBinding {
buffer: dst_buffer.as_ref(),
offset: 0,
size: Some(DST_BUFFER_SIZE),
buffers: &[unsafe {
// SAFETY: We just created the buffer with this size.
hal::BufferBinding::new_unchecked(dst_buffer.as_ref(), 0, DST_BUFFER_SIZE)
}],
samplers: &[],
textures: &[],
@ -278,10 +277,9 @@ impl Dispatch {
resource_index: 0,
count: 1,
}],
buffers: &[hal::BufferBinding {
buffer,
offset: 0,
size: Some(binding_size),
buffers: &[unsafe {
// SAFETY: We calculated the binding size to fit within the buffer.
hal::BufferBinding::new_unchecked(buffer, 0, binding_size)
}],
samplers: &[],
textures: &[],

View File

@ -135,10 +135,9 @@ impl Draw {
resource_index: 0,
count: 1,
}],
buffers: &[hal::BufferBinding {
buffer,
offset: 0,
size: Some(binding_size),
buffers: &[unsafe {
// SAFETY: We calculated the binding size to fit within the buffer.
hal::BufferBinding::new_unchecked(buffer, 0, binding_size)
}],
samplers: &[],
textures: &[],
@ -684,10 +683,9 @@ fn create_buffer_and_bind_group(
resource_index: 0,
count: 1,
}],
buffers: &[hal::BufferBinding {
buffer: buffer.as_ref(),
offset: 0,
size: Some(BUFFER_SIZE),
buffers: &[unsafe {
// SAFETY: We just created the buffer with this size.
hal::BufferBinding::new_unchecked(buffer.as_ref(), 0, BUFFER_SIZE)
}],
samplers: &[],
textures: &[],

View File

@ -17,7 +17,7 @@ use wgt::{
#[cfg(feature = "trace")]
use crate::device::trace;
use crate::{
binding_model::BindGroup,
binding_model::{BindGroup, BindingError},
device::{
queue, resource::DeferredDestroy, BufferMapPendingClosure, Device, DeviceError,
DeviceMismatch, HostMap, MissingDownlevelFlags, MissingFeatures,
@ -485,6 +485,76 @@ impl Buffer {
}
}
/// Resolve the size of a binding for buffer with `offset` and `size`.
///
/// If `size` is `None`, then the remainder of the buffer starting from
/// `offset` is used.
///
/// If the binding would overflow the buffer or is empty (see
/// [`hal::BufferBinding`]), then an error is returned.
pub fn resolve_binding_size(
&self,
offset: wgt::BufferAddress,
binding_size: Option<wgt::BufferSize>,
) -> Result<wgt::BufferSize, BindingError> {
let buffer_size = self.size;
match binding_size {
Some(binding_size) => {
match offset.checked_add(binding_size.get()) {
// `binding_size` is not zero which means `end == buffer_size` is ok.
Some(end) if end <= buffer_size => Ok(binding_size),
_ => Err(BindingError::BindingRangeTooLarge {
buffer: self.error_ident(),
offset,
binding_size: binding_size.get(),
buffer_size,
}),
}
}
None => {
// We require that `buffer_size - offset` converts to
// `BufferSize` (`NonZeroU64`) because bindings must not be
// empty.
buffer_size
.checked_sub(offset)
.and_then(wgt::BufferSize::new)
.ok_or_else(|| BindingError::BindingOffsetTooLarge {
buffer: self.error_ident(),
offset,
buffer_size,
})
}
}
}
/// Create a new [`hal::BufferBinding`] for the buffer with `offset` and
/// `size`.
///
/// If `size` is `None`, then the remainder of the buffer starting from
/// `offset` is used.
///
/// If the binding would overflow the buffer or is empty (see
/// [`hal::BufferBinding`]), then an error is returned.
pub fn binding<'a>(
&'a self,
offset: wgt::BufferAddress,
binding_size: Option<wgt::BufferSize>,
snatch_guard: &'a SnatchGuard,
) -> Result<hal::BufferBinding<'a, dyn hal::DynBuffer>, BindingError> {
let buf_raw = self.try_raw(snatch_guard)?;
let resolved_size = self.resolve_binding_size(offset, binding_size)?;
unsafe {
// SAFETY: The offset and size passed to hal::BufferBinding::new_unchecked must
// define a binding contained within the buffer.
Ok(hal::BufferBinding::new_unchecked(
buf_raw,
offset,
resolved_size,
))
}
}
/// Returns the mapping callback in case of error so that the callback can be fired outside
/// of the locks that are held in this function.
pub(crate) fn map_async(

View File

@ -242,12 +242,16 @@ impl TimestampNormalizer {
}
}
pub fn create_normalization_bind_group(
/// Create a bind group for normalizing timestamps in `buffer`.
///
/// This function is unsafe because it does not know that `buffer_size` is
/// the true size of the buffer.
pub unsafe fn create_normalization_bind_group(
&self,
device: &Device,
buffer: &dyn hal::DynBuffer,
buffer_label: Option<&str>,
buffer_size: u64,
buffer_size: wgt::BufferSize,
buffer_usages: wgt::BufferUsages,
) -> Result<TimestampNormalizationBindGroup, DeviceError> {
unsafe {
@ -263,7 +267,7 @@ impl TimestampNormalizer {
// at once to normalize the timestamps, we can't use it. We force the buffer to fail
// to allocate. The lowest max binding size is 128MB, and query sets must be small
// (no more than 4096), so this should never be hit in practice by sane programs.
if buffer_size > device.adapter.limits().max_storage_buffer_binding_size as u64 {
if buffer_size.get() > device.adapter.limits().max_storage_buffer_binding_size as u64 {
return Err(DeviceError::OutOfMemory);
}
@ -282,11 +286,7 @@ impl TimestampNormalizer {
.create_bind_group(&hal::BindGroupDescriptor {
label: Some(label),
layout: &*state.temporary_bind_group_layout,
buffers: &[hal::BufferBinding {
buffer,
offset: 0,
size: None,
}],
buffers: &[hal::BufferBinding::new_unchecked(buffer, 0, buffer_size)],
samplers: &[],
textures: &[],
acceleration_structures: &[],

View File

@ -445,10 +445,13 @@ impl<A: hal::Api> Example<A> {
let texture_view = unsafe { device.create_texture_view(&texture, &view_desc).unwrap() };
let global_group = {
let global_buffer_binding = hal::BufferBinding {
buffer: &global_buffer,
offset: 0,
size: None,
let global_buffer_binding = unsafe {
// SAFETY: This is the same size that was specified for buffer creation.
hal::BufferBinding::new_unchecked(
&global_buffer,
0,
global_buffer_desc.size.try_into().unwrap(),
)
};
let texture_binding = hal::TextureBinding {
view: &texture_view,
@ -483,10 +486,13 @@ impl<A: hal::Api> Example<A> {
};
let local_group = {
let local_buffer_binding = hal::BufferBinding {
buffer: &local_buffer,
offset: 0,
size: wgpu_types::BufferSize::new(size_of::<Locals>() as _),
let local_buffer_binding = unsafe {
// SAFETY: The size must fit within the buffer.
hal::BufferBinding::new_unchecked(
&local_buffer,
0,
wgpu_types::BufferSize::new(size_of::<Locals>() as _).unwrap(),
)
};
let local_group_desc = hal::BindGroupDescriptor {
label: Some("local"),

View File

@ -603,10 +603,13 @@ impl<A: hal::Api> Example<A> {
let texture_view = unsafe { device.create_texture_view(&texture, &view_desc).unwrap() };
let bind_group = {
let buffer_binding = hal::BufferBinding {
buffer: &uniform_buffer,
offset: 0,
size: None,
let buffer_binding = unsafe {
// SAFETY: The size matches the buffer allocation.
hal::BufferBinding::new_unchecked(
&uniform_buffer,
0,
wgpu_types::BufferSize::new_unchecked(uniforms_size as u64),
)
};
let texture_binding = hal::TextureBinding {
view: &texture_view,

View File

@ -534,7 +534,6 @@ impl crate::Device for super::Device {
return Ok(super::Buffer {
raw: None,
target,
size: desc.size,
map_flags: 0,
data: Some(Arc::new(MaybeMutex::new(vec![0; desc.size as usize]))),
offset_of_current_mapping: Arc::new(MaybeMutex::new(0)),
@ -634,7 +633,6 @@ impl crate::Device for super::Device {
Ok(super::Buffer {
raw,
target,
size: desc.size,
map_flags,
data,
offset_of_current_mapping: Arc::new(MaybeMutex::new(0)),
@ -1265,11 +1263,8 @@ impl crate::Device for super::Device {
let bb = &desc.buffers[entry.resource_index as usize];
super::RawBinding::Buffer {
raw: bb.buffer.raw.unwrap(),
offset: bb.offset as i32,
size: match bb.size {
Some(s) => s.get() as i32,
None => (bb.buffer.size - bb.offset) as i32,
},
offset: bb.offset.try_into().unwrap(),
size: bb.size.get().try_into().unwrap(),
}
}
wgt::BindingType::Sampler { .. } => {

View File

@ -342,7 +342,6 @@ impl Drop for Queue {
pub struct Buffer {
raw: Option<glow::Buffer>,
target: BindTarget,
size: wgt::BufferAddress,
map_flags: u32,
data: Option<Arc<MaybeMutex<Vec<u8>>>>,
offset_of_current_mapping: Arc<MaybeMutex<wgt::BufferAddress>>,

View File

@ -1968,6 +1968,13 @@ pub struct PipelineLayoutDescriptor<'a, B: DynBindGroupLayout + ?Sized> {
///
/// [`BindGroup`]: Api::BindGroup
///
/// ## Construction
///
/// The recommended way to construct a `BufferBinding` is using the `binding`
/// method on a wgpu-core `Buffer`, which will validate the binding size
/// against the buffer size. An unsafe `new_unchecked` constructor is also
/// provided for cases where direct construction is necessary.
///
/// ## Accessible region
///
/// `wgpu_hal` guarantees that shaders compiled with
@ -1992,39 +1999,48 @@ pub struct PipelineLayoutDescriptor<'a, B: DynBindGroupLayout + ?Sized> {
/// parts of which buffers shaders might observe. This optimization is only
/// sound if shader access is bounds-checked.
///
/// ## Zero-length bindings
///
/// Some back ends cannot tolerate zero-length regions; for example, see
/// [VUID-VkDescriptorBufferInfo-offset-00340][340] and
/// [VUID-VkDescriptorBufferInfo-range-00341][341], or the
/// documentation for GLES's [glBindBufferRange][bbr]. For this reason, a valid
/// `BufferBinding` must have `offset` strictly less than the size of the
/// buffer.
///
/// WebGPU allows zero-length bindings, and there is not currently a mechanism
/// in place
///
/// [`buffer`]: BufferBinding::buffer
/// [`offset`]: BufferBinding::offset
/// [`size`]: BufferBinding::size
/// [`Storage`]: wgt::BufferBindingType::Storage
/// [`Uniform`]: wgt::BufferBindingType::Uniform
/// [340]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorBufferInfo-offset-00340
/// [341]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorBufferInfo-range-00341
/// [bbr]: https://registry.khronos.org/OpenGL-Refpages/es3.0/html/glBindBufferRange.xhtml
/// [woob]: https://gpuweb.github.io/gpuweb/wgsl/#out-of-bounds-access-sec
#[derive(Debug)]
pub struct BufferBinding<'a, B: DynBuffer + ?Sized> {
/// The buffer being bound.
pub buffer: &'a B,
///
/// This is not fully `pub` to prevent direct construction of
/// `BufferBinding`s, while still allowing public read access to the `offset`
/// and `size` properties.
pub(crate) buffer: &'a B,
/// The offset at which the bound region starts.
///
/// This must be less than the size of the buffer. Some back ends
/// cannot tolerate zero-length regions; for example, see
/// [VUID-VkDescriptorBufferInfo-offset-00340][340] and
/// [VUID-VkDescriptorBufferInfo-range-00341][341], or the
/// documentation for GLES's [glBindBufferRange][bbr].
///
/// [340]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorBufferInfo-offset-00340
/// [341]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorBufferInfo-range-00341
/// [bbr]: https://registry.khronos.org/OpenGL-Refpages/es3.0/html/glBindBufferRange.xhtml
/// Because zero-length bindings are not permitted (see above), this must be
/// strictly less than the size of the buffer.
pub offset: wgt::BufferAddress,
/// The size of the region bound, in bytes.
///
/// If `None`, the region extends from `offset` to the end of the
/// buffer. Given the restrictions on `offset`, this means that
/// the size is always greater than zero.
pub size: Option<wgt::BufferSize>,
pub size: wgt::BufferSize,
}
impl<'a, T: DynBuffer + ?Sized> Clone for BufferBinding<'a, T> {
// We must implement this manually because `B` is not necessarily `Clone`.
impl<B: DynBuffer + ?Sized> Clone for BufferBinding<'_, B> {
fn clone(&self) -> Self {
BufferBinding {
buffer: self.buffer,
@ -2034,6 +2050,31 @@ impl<'a, T: DynBuffer + ?Sized> Clone for BufferBinding<'a, T> {
}
}
impl<'a, B: DynBuffer + ?Sized> BufferBinding<'a, B> {
/// Construct a `BufferBinding` with the given contents.
///
/// When possible, use the `binding` method on a wgpu-core `Buffer` instead
/// of this method. `Buffer::binding` validates the size of the binding
/// against the size of the buffer.
///
/// It is more difficult to provide a validating constructor here, due to
/// not having direct access to the size of a `DynBuffer`.
///
/// SAFETY: The caller is responsible for ensuring that a binding of `size`
/// bytes starting at `offset` is contained within the buffer.
pub unsafe fn new_unchecked(
buffer: &'a B,
offset: wgt::BufferAddress,
size: wgt::BufferSize,
) -> Self {
Self {
buffer,
offset,
size,
}
}
}
#[derive(Debug)]
pub struct TextureBinding<'a, T: DynTextureView + ?Sized> {
pub view: &'a T,

View File

@ -977,15 +977,9 @@ impl crate::CommandEncoder for super::CommandEncoder {
let encoder = self.state.render.as_ref().unwrap();
encoder.set_vertex_buffer(buffer_index, Some(&binding.buffer.raw), binding.offset);
let buffer_size = binding.resolve_size();
if buffer_size > 0 {
self.state.vertex_buffer_size_map.insert(
buffer_index,
core::num::NonZeroU64::new(buffer_size).unwrap(),
);
} else {
self.state.vertex_buffer_size_map.remove(&buffer_index);
}
self.state
.vertex_buffer_size_map
.insert(buffer_index, binding.size);
if let Some((index, sizes)) = self
.state

View File

@ -340,10 +340,6 @@ impl super::Device {
}
}
pub unsafe fn buffer_from_raw(raw: metal::Buffer, size: wgt::BufferAddress) -> super::Buffer {
super::Buffer { raw, size }
}
pub fn raw_device(&self) -> &Mutex<metal::Device> {
&self.shared.device
}
@ -373,10 +369,7 @@ impl crate::Device for super::Device {
raw.set_label(label);
}
self.counters.buffers.add(1);
Ok(super::Buffer {
raw,
size: desc.size,
})
Ok(super::Buffer { raw })
})
}
unsafe fn destroy_buffer(&self, _buffer: super::Buffer) {
@ -935,14 +928,9 @@ impl crate::Device for super::Device {
let end = start + 1;
bg.buffers
.extend(desc.buffers[start..end].iter().map(|source| {
// Given the restrictions on `BufferBinding::offset`,
// this should never be `None`.
let remaining_size = wgt::BufferSize::new(
source.buffer.size - source.offset,
);
let binding_size = match ty {
wgt::BufferBindingType::Storage { .. } => {
source.size.or(remaining_size)
Some(source.size)
}
_ => None,
};

View File

@ -502,7 +502,6 @@ impl crate::Queue for Queue {
#[derive(Debug)]
pub struct Buffer {
raw: metal::Buffer,
size: wgt::BufferAddress,
}
unsafe impl Send for Buffer {}
@ -516,15 +515,6 @@ impl Buffer {
}
}
impl crate::BufferBinding<'_, Buffer> {
fn resolve_size(&self) -> wgt::BufferAddress {
match self.size {
Some(size) => size.get(),
None => self.buffer.size - self.offset,
}
}
}
#[derive(Debug)]
pub struct Texture {
raw: metal::Texture,

View File

@ -1802,9 +1802,7 @@ impl crate::Device for super::Device {
vk::DescriptorBufferInfo::default()
.buffer(binding.buffer.raw)
.offset(binding.offset)
.range(
binding.size.map_or(vk::WHOLE_SIZE, wgt::BufferSize::get),
)
.range(binding.size.get())
},
));
write.buffer_info(local_buffer_infos)

View File

@ -322,6 +322,7 @@ impl Device {
/// - `hal_buffer` must be created from this device internal handle
/// - `hal_buffer` must be created respecting `desc`
/// - `hal_buffer` must be initialized
/// - `hal_buffer` must not have zero size
#[cfg(wgpu_core)]
#[must_use]
pub unsafe fn create_buffer_from_hal<A: wgc::hal_api::HalApi>(

View File

@ -170,6 +170,12 @@ impl ContextWgpuCore {
}
}
/// # Safety
///
/// - `hal_buffer` must be created from `device`.
/// - `hal_buffer` must be created respecting `desc`
/// - `hal_buffer` must be initialized
/// - `hal_buffer` must not have zero size.
pub unsafe fn create_buffer_from_hal<A: wgc::hal_api::HalApi>(
&self,
hal_buffer: A::Buffer,