From d6465702b65535df7884df0b86fed14748d22132 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Wed, 21 Feb 2024 15:10:29 -0500 Subject: [PATCH] fix: `command_encoder_clear_buffer`: err. on `offset + size > u64::MAX` 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! --- tests/tests/buffer.rs | 32 ++++++++++++++++++++++++++++++++ wgpu-core/src/command/clear.rs | 13 ++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/tests/buffer.rs b/tests/tests/buffer.rs index f9d2e03c69..a5fcf3e595 100644 --- a/tests/tests/buffer.rs +++ b/tests/tests/buffer.rs @@ -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()) + .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`" + ))); + }); diff --git a/wgpu-core/src/command/clear.rs b/wgpu-core/src/command/clear.rs index 25dc417b80..e404fabb14 100644 --- a/wgpu-core/src/command/clear.rs +++ b/wgpu-core/src/command/clear.rs @@ -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, @@ -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,