diff --git a/CHANGELOG.md b/CHANGELOG.md index 74efa7bd9a..2025dba103 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ Bottom level categories: - Remove `wgpu_types::Features::DEPTH24PLUS_STENCIL8`, making `wgpu::TextureFormat::Depth24PlusStencil8` available on all backends. By @Healthire in (#3151)[https://github.com/gfx-rs/wgpu/pull/3151] - Fix an integer overflow in `queue_write_texture` by @nical in (#3146)[https://github.com/gfx-rs/wgpu/pull/3146] - Make `RenderPassCompatibilityError` and `CreateShaderModuleError` not so huge. By @jimblandy in (#3226)[https://github.com/gfx-rs/wgpu/pull/3226] +- Check for invalid bitflag bits in wgpu-core and allow them to be captured/replayed by @nical in (#3229)[https://github.com/gfx-rs/wgpu/pull/3229] #### WebGPU diff --git a/Cargo.lock b/Cargo.lock index 32fadd6b61..0b0361fe8f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -173,16 +173,6 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" -[[package]] -name = "bitflags_serde_shim" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25c3d626f0280ec39b33a6fc5c6c1067432b4c41e94aee40ded197a6649bf025" -dependencies = [ - "bitflags", - "serde", -] - [[package]] name = "block" version = "0.1.6" @@ -2898,7 +2888,6 @@ name = "wgpu-types" version = "0.14.0" dependencies = [ "bitflags", - "bitflags_serde_shim", "serde", "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 1eb3768f0e..50fd3c2d51 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,7 +46,6 @@ version = "0.10" arrayvec = "0.7" async-executor = "1.0" bitflags = "1" -bitflags_serde_shim = "0.2" bit-vec = "0.6" bytemuck = "1.4" cargo-run-wasm = "0.2.0" diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index 81d0c5ccff..5feef3bab4 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -50,6 +50,8 @@ pub enum CreateBindGroupLayoutError { TooManyBindings(BindingTypeMaxCountError), #[error("Binding index {binding} is greater than the maximum index {maximum}")] InvalidBindingIndex { binding: u32, maximum: u32 }, + #[error("Invalid visibility {0:?}")] + InvalidVisibility(wgt::ShaderStages), } //TODO: refactor this to move out `enum BindingError`. diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 494fa41b7a..e6df1be03f 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -600,8 +600,8 @@ impl Device { let mut usage = conv::map_buffer_usage(desc.usage); - if desc.usage.is_empty() { - return Err(resource::CreateBufferError::EmptyUsage); + if desc.usage.is_empty() || desc.usage.contains_invalid_bits() { + return Err(resource::CreateBufferError::InvalidUsage(desc.usage)); } if !self @@ -717,8 +717,8 @@ impl Device { ) -> Result, resource::CreateTextureError> { use resource::{CreateTextureError, TextureDimensionError}; - if desc.usage.is_empty() { - return Err(CreateTextureError::EmptyUsage); + if desc.usage.is_empty() || desc.usage.contains_invalid_bits() { + return Err(CreateTextureError::InvalidUsage(desc.usage)); } conv::check_texture_dimension_size( @@ -1560,6 +1560,13 @@ impl Device { error, })?; } + + if entry.visibility.contains_invalid_bits() { + return Err( + binding_model::CreateBindGroupLayoutError::InvalidVisibility(entry.visibility), + ); + } + if entry.visibility.contains(wgt::ShaderStages::VERTEX) { if writable_storage == WritableStorage::Yes { required_features |= wgt::Features::VERTEX_WRITABLE_STORAGE; @@ -2650,6 +2657,10 @@ impl Device { for (i, cs) in color_targets.iter().enumerate() { if let Some(cs) = cs.as_ref() { let error = loop { + if cs.write_mask.contains_invalid_bits() { + break Some(pipeline::ColorStateError::InvalidWriteMask(cs.write_mask)); + } + let format_features = self.describe_format_features(adapter, cs.format)?; if !format_features .allowed_usages diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 5c091e1d96..cd5a8ffb09 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -303,6 +303,8 @@ pub enum ColorStateError { }, #[error("blend factors for {0:?} must be `One`")] InvalidMinMaxBlendFactors(wgt::BlendComponent), + #[error("invalid write mask {0:?}")] + InvalidWriteMask(wgt::ColorWrites), } #[derive(Clone, Debug, Error)] diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 6339d77b6b..4302df7e61 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -242,8 +242,8 @@ pub enum CreateBufferError { AccessError(#[from] BufferAccessError), #[error("buffers that are mapped at creation have to be aligned to `COPY_BUFFER_ALIGNMENT`")] UnalignedSize, - #[error("Buffers cannot have empty usage flags")] - EmptyUsage, + #[error("Invalid usage flags {0:?}")] + InvalidUsage(wgt::BufferUsages), #[error("`MAP` usage can only be combined with the opposite `COPY`, requested {0:?}")] UsageMismatch(wgt::BufferUsages), #[error("Buffer size {requested} is greater than the maximum buffer size ({maximum})")] @@ -490,8 +490,8 @@ pub enum TextureDimensionError { pub enum CreateTextureError { #[error(transparent)] Device(#[from] DeviceError), - #[error("Textures cannot have empty usage flags")] - EmptyUsage, + #[error("Invalid usage flags {0:?}")] + InvalidUsage(wgt::TextureUsages), #[error(transparent)] InvalidDimension(#[from] TextureDimensionError), #[error("Depth texture ({1:?}) can't be created as {0:?}")] diff --git a/wgpu-types/Cargo.toml b/wgpu-types/Cargo.toml index 44b046e37c..0748296a81 100644 --- a/wgpu-types/Cargo.toml +++ b/wgpu-types/Cargo.toml @@ -16,13 +16,12 @@ rustdoc-args = ["--cfg", "docsrs"] [lib] [features] -trace = ["serde", "bitflags_serde_shim"] -replay = ["serde", "bitflags_serde_shim"] +trace = ["serde"] +replay = ["serde"] [dependencies] bitflags.workspace = true serde = { workspace = true, features = ["serde_derive"], optional = true } -bitflags_serde_shim = { workspace = true, optional = true } [dev-dependencies] serde_json.workspace = true diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 59710afeca..a83adb9eaa 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -14,6 +14,48 @@ use serde::{Deserialize, Serialize}; use std::hash::{Hash, Hasher}; use std::{num::NonZeroU32, ops::Range}; +// Use this macro instead of the one provided by the bitflags_serde_shim crate +// because the latter produces an error when deserializing bits that are not +// specified in the bitflags, while we want deserialization to succeed and +// and unspecified bits to lead to errors handled in wgpu-core. +// Note that plainly deriving Serialize and Deserialized would have a similar +// behavior to this macro (unspecified bit do not produce an error). +macro_rules! impl_bitflags { + ($name:ident) => { + #[cfg(feature = "trace")] + impl serde::Serialize for $name { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.bits().serialize(serializer) + } + } + + #[cfg(feature = "replay")] + impl<'de> serde::Deserialize<'de> for $name { + fn deserialize(deserializer: D) -> Result<$name, D::Error> + where + D: serde::Deserializer<'de>, + { + let value = <_ as serde::Deserialize<'de>>::deserialize(deserializer)?; + // Note: newer version of bitflags replaced from_bits_unchecked with + // from_bits_retain which is not marked as unsafe (same implementation). + Ok(unsafe { $name::from_bits_unchecked(value) }) + } + } + + impl $name { + /// Returns true if the bitflags contains bits that are not part of + /// the bitflags definition. + pub fn contains_invalid_bits(&self) -> bool { + let all = Self::all().bits(); + (self.bits() | all) != all + } + } + }; +} + /// Integral type used for buffer offsets. pub type BufferAddress = u64; /// Integral type used for buffer slice sizes. @@ -115,8 +157,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(Backends); +impl_bitflags!(Backends); impl From for Backends { fn from(backend: Backend) -> Self { @@ -633,8 +674,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(Features); +impl_bitflags!(Features); impl Features { /// Mask of all features which are part of the upstream WebGPU standard. @@ -1090,8 +1130,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(DownlevelFlags); +impl_bitflags!(DownlevelFlags); impl DownlevelFlags { /// All flags that indicate if the backend is WebGPU compliant @@ -1213,8 +1252,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(ShaderStages); +impl_bitflags!(ShaderStages); /// Dimensions of a particular texture view. /// @@ -1666,8 +1704,7 @@ impl TextureFormatFeatureFlags { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(TextureFormatFeatureFlags); +impl_bitflags!(TextureFormatFeatureFlags); /// Features supported by a given texture format /// @@ -3085,8 +3122,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(ColorWrites); +impl_bitflags!(ColorWrites); impl Default for ColorWrites { fn default() -> Self { @@ -3644,8 +3680,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(BufferUsages); +impl_bitflags!(BufferUsages); /// Describes a [`Buffer`](../wgpu/struct.Buffer.html). /// @@ -3846,8 +3881,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(TextureUsages); +impl_bitflags!(TextureUsages); /// Configures a [`Surface`] for presentation. /// @@ -5046,8 +5080,7 @@ bitflags::bitflags! { } } -#[cfg(feature = "bitflags_serde_shim")] -bitflags_serde_shim::impl_serde_for_bitflags!(PipelineStatisticsTypes); +impl_bitflags!(PipelineStatisticsTypes); /// Argument buffer layout for draw_indirect commands. #[repr(C)] diff --git a/wgpu/tests/buffer_usages.rs b/wgpu/tests/buffer_usages.rs index 34075f8b7f..db734c108b 100644 --- a/wgpu/tests/buffer_usages.rs +++ b/wgpu/tests/buffer_usages.rs @@ -50,7 +50,8 @@ fn buffer_usage() { Bu::MAP_WRITE | Bu::COPY_SRC | Bu::STORAGE, Bu::all(), ]; - let always_fail = &[Bu::empty()]; + let invalid_bits = unsafe { Bu::from_bits_unchecked(0b1111111111111) }; + let always_fail = &[Bu::empty(), invalid_bits]; try_create( false,