Skip to content

Commit

Permalink
fix: command_encoder_clear_buffer: err. on offset + size > u64::MAX
Browse files Browse the repository at this point in the history
Rust would have made this operation either an overflow in release mode,
or a panic in debug mode. Neither seem appropriate for this context,
where I suspect an error should be returned instead. Web browsers, for
instance, shouldn't crash simply because of an issue of this nature.
Users may, quite reasonably, have bad arguments to this in early stages
of development!
  • Loading branch information
ErichDonGubler committed Feb 21, 2024
1 parent 938cd13 commit 5b12fc6
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
32 changes: 32 additions & 0 deletions tests/tests/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,35 @@ static CLEAR_OFFSET_OUTSIDE_RESOURCE_BOUNDS: GpuTestConfiguration = GpuTestConfi
"Clear of 20..20 would end up overrunning the bounds of the buffer of size 16"
));
});

#[gpu_test]
static CLEAR_OFFSET_PLUS_SIZE_OUTSIDE_U64_BOUNDS: GpuTestConfiguration =
GpuTestConfiguration::new()
.parameters(TestParameters::default().test_features_limits())
.run_sync(|ctx| {
let buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
label: None,
size: 16, // unimportant for this test
usage: wgpu::BufferUsages::COPY_DST,
mapped_at_creation: false,
});

let max_valid_offset = u64::MAX - (u64::MAX % wgpu::COPY_BUFFER_ALIGNMENT);
let smallest_aligned_invalid_size = wgpu::COPY_BUFFER_ALIGNMENT;

ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
ctx.device
.create_command_encoder(&Default::default())
.clear_buffer(
&buffer,
max_valid_offset,
Some(smallest_aligned_invalid_size),
);
let err_msg = pollster::block_on(ctx.device.pop_error_scope())
.unwrap()
.to_string();
assert!(err_msg.contains(concat!(
"Clear starts at offset 18446744073709551612 with size of 4, ",
"but these added together exceed `u64::MAX`"
)));
});
13 changes: 12 additions & 1 deletion wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ pub enum ClearError {
UnalignedFillSize(BufferAddress),
#[error("Buffer offset {0:?} is not a multiple of `COPY_BUFFER_ALIGNMENT`")]
UnalignedBufferOffset(BufferAddress),
#[error("Clear starts at offset {start_offset} with size of {requested_size}, but these added together exceed `u64::MAX`")]
OffsetPlusSizeExceeds64BitBounds {
start_offset: BufferAddress,
requested_size: BufferAddress,
},
#[error("Clear of {start_offset}..{end_offset} would end up overrunning the bounds of the buffer of size {buffer_size}")]
BufferOverrun {
start_offset: BufferAddress,
Expand Down Expand Up @@ -122,7 +127,13 @@ impl Global {
if size % wgt::COPY_BUFFER_ALIGNMENT != 0 {
return Err(ClearError::UnalignedFillSize(size));
}
let end_offset = offset + size;
let end_offset =
offset
.checked_add(size)
.ok_or(ClearError::OffsetPlusSizeExceeds64BitBounds {
start_offset: offset,
requested_size: size,
})?;
if end_offset > dst_buffer.size {
return Err(ClearError::BufferOverrun {
start_offset: offset,
Expand Down

0 comments on commit 5b12fc6

Please sign in to comment.