diff --git a/CHANGELOG.md b/CHANGELOG.md index dee57f6e78..ee63a2bde9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -131,6 +131,10 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148] - Lower `QUERY_SET_MAX_QUERIES` (and enforced limits) from 8192 to 4096 to match WebGPU spec. By @ErichDonGubler in [#6525](https://github.com/gfx-rs/wgpu/pull/6525). - Allow non-filterable float on texture bindings never used with samplers when using a derived bind group layout. By @ErichDonGubler in [#6531](https://github.com/gfx-rs/wgpu/pull/6531/). - Replace potentially unsound usage of `PreHashedMap` with `FastHashMap`. By @jamienicol in [#6541](https://github.com/gfx-rs/wgpu/pull/6541). +- Add missing validation for timestamp writes in compute and render passes. By @ErichDonGubler in [#6578](https://github.com/gfx-rs/wgpu/pull/6578). + - Check the status of the `TIMESTAMP_QUERY` feature before other validation. + - Check that indices are in-bounds for the query set. + - Check that begin and end indices are not equal. #### Naga diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 28e76c1115..f27ad1a8bb 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -28,7 +28,9 @@ use crate::{ use thiserror::Error; use wgt::{BufferAddress, DynamicOffset}; -use super::{bind::BinderError, memory_init::CommandBufferTextureMemoryActions}; +use super::{ + bind::BinderError, memory_init::CommandBufferTextureMemoryActions, SimplifiedQueryType, +}; use crate::ray_tracing::TlasAction; use std::sync::Arc; use std::{fmt, mem::size_of, str}; @@ -309,26 +311,53 @@ impl Global { }; arc_desc.timestamp_writes = if let Some(tw) = desc.timestamp_writes { - let query_set = match hub.query_sets.get(tw.query_set).get() { + let &PassTimestampWrites { + query_set, + beginning_of_pass_write_index, + end_of_pass_write_index, + } = tw; + + match cmd_buf + .device + .require_features(wgt::Features::TIMESTAMP_QUERY) + { + Ok(()) => (), + Err(e) => return make_err(e.into(), arc_desc), + } + + let query_set = match hub.query_sets.get(query_set).get() { Ok(query_set) => query_set, Err(e) => return make_err(e.into(), arc_desc), }; + match query_set.same_device(&cmd_buf.device) { Ok(()) => (), Err(e) => return make_err(e.into(), arc_desc), } - match cmd_buf - .device - .require_features(wgt::Features::TIMESTAMP_QUERY) + + for idx in [beginning_of_pass_write_index, end_of_pass_write_index] + .into_iter() + .flatten() { - Ok(()) => (), - Err(e) => return make_err(e.into(), arc_desc), + match query_set.validate_query(SimplifiedQueryType::Timestamp, idx, None) { + Ok(()) => (), + Err(e) => return make_err(e.into(), arc_desc), + } + } + + if let Some((begin, end)) = beginning_of_pass_write_index.zip(end_of_pass_write_index) { + if begin == end { + return make_err( + CommandEncoderError::TimestampWriteIndicesEqual { idx: begin }, + arc_desc, + ); + } } Some(ArcPassTimestampWrites { query_set, - beginning_of_pass_write_index: tw.beginning_of_pass_write_index, - end_of_pass_write_index: tw.end_of_pass_write_index, + beginning_of_pass_write_index, + end_of_pass_write_index, }) } else { None diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 6a06d1f83d..1f1cafaf5e 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -654,6 +654,12 @@ pub enum CommandEncoderError { InvalidResource(#[from] InvalidResourceError), #[error(transparent)] MissingFeatures(#[from] MissingFeatures), + #[error( + "begin and end indices of pass timestamp writes are both set to {idx}, which is not allowed" + )] + TimestampWriteIndicesEqual { idx: u32 }, + #[error(transparent)] + TimestampWritesInvalid(#[from] QueryUseError), } impl Global { diff --git a/wgpu-core/src/command/query.rs b/wgpu-core/src/command/query.rs index d783721fb4..b2dff276eb 100644 --- a/wgpu-core/src/command/query.rs +++ b/wgpu-core/src/command/query.rs @@ -160,7 +160,7 @@ pub enum ResolveError { } impl QuerySet { - fn validate_query( + pub(crate) fn validate_query( self: &Arc, query_type: SimplifiedQueryType, query_index: u32, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index ad3a5b6ca3..21c9a75f5b 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1,6 +1,7 @@ use crate::binding_model::BindGroup; use crate::command::{ validate_and_begin_occlusion_query, validate_and_begin_pipeline_statistics_query, + SimplifiedQueryType, }; use crate::init_tracker::BufferInitTrackerAction; use crate::pipeline::RenderPipeline; @@ -1392,15 +1393,37 @@ impl Global { }; arc_desc.timestamp_writes = if let Some(tw) = desc.timestamp_writes { - let query_set = query_sets.get(tw.query_set).get()?; - query_set.same_device(device)?; + let &PassTimestampWrites { + query_set, + beginning_of_pass_write_index, + end_of_pass_write_index, + } = tw; + + let query_set = query_sets.get(query_set).get()?; device.require_features(wgt::Features::TIMESTAMP_QUERY)?; + query_set.same_device(device)?; + + for idx in [beginning_of_pass_write_index, end_of_pass_write_index] + .into_iter() + .flatten() + { + query_set.validate_query(SimplifiedQueryType::Timestamp, idx, None)?; + } + + if let Some((begin, end)) = + beginning_of_pass_write_index.zip(end_of_pass_write_index) + { + if begin == end { + return Err(CommandEncoderError::TimestampWriteIndicesEqual { idx: begin }); + } + } + Some(ArcPassTimestampWrites { query_set, - beginning_of_pass_write_index: tw.beginning_of_pass_write_index, - end_of_pass_write_index: tw.end_of_pass_write_index, + beginning_of_pass_write_index, + end_of_pass_write_index, }) } else { None