Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ By @cwfitzgerald in [#8609](https://github.com/gfx-rs/wgpu/pull/8609).
#### Naga

- Prevent UB with invalid ray query calls on spirv. By @Vecvec in [#8390](https://github.com/gfx-rs/wgpu/pull/8390).
- Update the set of binding_array capabilities. In most cases, they are set automatically from `wgpu` features, and this change should not be user-visible. By @andyleiserson in TBD.

### Bug Fixes

Expand Down
30 changes: 17 additions & 13 deletions naga/src/valid/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,30 +551,34 @@ impl FunctionInfo {
base: array_element_ty_handle,
..
} => {
// these are nasty aliases, but these idents are too long and break rustfmt
let sto = super::Capabilities::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
let uni = super::Capabilities::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING;
let st_sb = super::Capabilities::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING;
let sampler = super::Capabilities::SAMPLER_NON_UNIFORM_INDEXING;

// We're a binding array, so lets use the type of _what_ we are array of to determine if we can non-uniformly index it.
let array_element_ty =
&resolve_context.types[array_element_ty_handle].inner;

needed_caps |= match *array_element_ty {
// If we're an image, use the appropriate limit.
// If we're an image, use the appropriate capability.
crate::TypeInner::Image { class, .. } => match class {
crate::ImageClass::Storage { .. } => sto,
_ => st_sb,
crate::ImageClass::Storage { .. } => {
super::Capabilities::STORAGE_TEXTURE_BINDING_ARRAY_NON_UNIFORM_INDEXING
}
_ => {
super::Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY_NON_UNIFORM_INDEXING
}
},
crate::TypeInner::Sampler { .. } => sampler,
// If we're anything but an image, assume we're a buffer and use the address space.
crate::TypeInner::Sampler { .. } => {
super::Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY_NON_UNIFORM_INDEXING
}
// If we're anything but an image or sampler, assume we're a buffer and use the address space.
_ => {
if let E::GlobalVariable(global_handle) = expression_arena[base] {
let global = &resolve_context.global_vars[global_handle];
match global.space {
crate::AddressSpace::Uniform => uni,
crate::AddressSpace::Storage { .. } => st_sb,
crate::AddressSpace::Uniform => {
super::Capabilities::BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING
}
crate::AddressSpace::Storage { .. } => {
super::Capabilities::STORAGE_BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING
}
_ => unreachable!(),
}
} else {
Expand Down
79 changes: 76 additions & 3 deletions naga/src/valid/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ pub enum GlobalVariableError {
InvalidUsage(crate::AddressSpace),
#[error("Type isn't compatible with address space {0:?}")]
InvalidType(crate::AddressSpace),
#[error("Type {0:?} isn't compatible with binding arrays")]
InvalidBindingArray(Handle<crate::Type>),
#[error("Type flags {seen:?} do not meet the required {required:?}")]
MissingTypeFlags {
required: super::TypeFlags,
Expand Down Expand Up @@ -643,9 +645,80 @@ impl super::Validator {
// series of individually bound resources, so we can (mostly)
// validate a `binding_array<T>` as if it were just a plain `T`.
crate::TypeInner::BindingArray { base, .. } => match var.space {
crate::AddressSpace::Storage { .. }
| crate::AddressSpace::Uniform
| crate::AddressSpace::Handle => base,
crate::AddressSpace::Storage { .. } => {
if !self
.capabilities
.contains(Capabilities::STORAGE_BUFFER_BINDING_ARRAY)
{
return Err(GlobalVariableError::UnsupportedCapability(
Capabilities::STORAGE_BUFFER_BINDING_ARRAY,
));
}
base
}
crate::AddressSpace::Uniform => {
if !self
.capabilities
.contains(Capabilities::BUFFER_BINDING_ARRAY)
{
return Err(GlobalVariableError::UnsupportedCapability(
Capabilities::BUFFER_BINDING_ARRAY,
));
}
base
}
crate::AddressSpace::Handle => {
match gctx.types[base].inner {
crate::TypeInner::Image { class, .. } => match class {
crate::ImageClass::Storage { .. } => {
if !self
.capabilities
.contains(Capabilities::STORAGE_TEXTURE_BINDING_ARRAY)
{
return Err(GlobalVariableError::UnsupportedCapability(
Capabilities::STORAGE_TEXTURE_BINDING_ARRAY,
));
}
}
crate::ImageClass::Sampled { .. } | crate::ImageClass::Depth { .. } => {
if !self
.capabilities
.contains(Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY)
{
return Err(GlobalVariableError::UnsupportedCapability(
Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY,
));
}
}
crate::ImageClass::External => {
// This should have been rejected in `validate_type`.
unreachable!("binding arrays of external images are not supported");
}
},
crate::TypeInner::Sampler { .. } => {
if !self
.capabilities
.contains(Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY)
{
return Err(GlobalVariableError::UnsupportedCapability(
Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY,
));
}
}
crate::TypeInner::AccelerationStructure { .. } => {
return Err(GlobalVariableError::InvalidBindingArray(base));
}
crate::TypeInner::RayQuery { .. } => {
// This should have been rejected in `validate_type`.
unreachable!("binding arrays of ray queries are not supported");
}
_ => {
// Fall through to the regular validation, which will reject `base`
// as invalid in `AddressSpace::Handle`.
}
}
base
}
_ => return Err(GlobalVariableError::InvalidUsage(var.space)),
},
_ => var.ty,
Expand Down
26 changes: 17 additions & 9 deletions naga/src/valid/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ bitflags::bitflags! {
#[cfg_attr(feature = "serialize", derive(serde::Serialize))]
#[cfg_attr(feature = "deserialize", derive(serde::Deserialize))]
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct Capabilities: u32 {
pub struct Capabilities: u64 {
/// Support for [`AddressSpace::Immediate`][1].
///
/// [1]: crate::AddressSpace::Immediate
Expand All @@ -94,14 +94,14 @@ bitflags::bitflags! {
///
/// [1]: crate::BuiltIn::PrimitiveIndex
const PRIMITIVE_INDEX = 1 << 2;
/// Support for non-uniform indexing of sampled textures and storage buffer arrays.
const SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING = 1 << 3;
/// Support for non-uniform indexing of storage texture arrays.
const STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING = 1 << 4;
/// Support for non-uniform indexing of uniform buffer arrays.
const UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING = 1 << 5;
/// Support for non-uniform indexing of samplers.
const SAMPLER_NON_UNIFORM_INDEXING = 1 << 6;
/// Support for binding arrays of sampled textures and samplers.
const TEXTURE_AND_SAMPLER_BINDING_ARRAY = 1 << 3;
/// Support for binding arrays of uniform buffers.
const BUFFER_BINDING_ARRAY = 1 << 4;
/// Support for binding arrays of storage textures.
const STORAGE_TEXTURE_BINDING_ARRAY = 1 << 5;
/// Support for binding arrays of storage buffers.
const STORAGE_BUFFER_BINDING_ARRAY = 1 << 6;
/// Support for [`BuiltIn::ClipDistance`].
///
/// [`BuiltIn::ClipDistance`]: crate::BuiltIn::ClipDistance
Expand Down Expand Up @@ -192,6 +192,14 @@ bitflags::bitflags! {
const MESH_SHADER = 1 << 30;
/// Support for mesh shaders which output points.
const MESH_SHADER_POINT_TOPOLOGY = 1 << 31;
/// Support for non-uniform indexing of binding arrays of sampled textures and samplers.
const TEXTURE_AND_SAMPLER_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 32;
/// Support for non-uniform indexing of binding arrays of uniform buffers.
const BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 33;
/// Support for non-uniform indexing of binding arrays of storage textures.
const STORAGE_TEXTURE_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 34;
/// Support for non-uniform indexing of binding arrays of storage buffers.
const STORAGE_BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 35;
}
}

Expand Down
4 changes: 3 additions & 1 deletion naga/src/valid/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,7 @@ impl super::Validator {

if base_info.flags.contains(TypeFlags::DATA) {
// Currently Naga only supports binding arrays of structs for non-handle types.
// `validate_global_var` relies on ray queries (which are `DATA`) being rejected here
match gctx.types[base].inner {
crate::TypeInner::Struct { .. } => {}
_ => return Err(TypeError::BindingArrayBaseTypeNotStruct(base)),
Expand All @@ -815,7 +816,8 @@ impl super::Validator {
}
) {
// Binding arrays of external textures are not yet supported.
// https://github.com/gfx-rs/wgpu/issues/8027
// See <https://github.com/gfx-rs/wgpu/issues/8027>. Note that
// `validate_global_var` relies on this error being raised here.
return Err(TypeError::BindingArrayBaseExternalTextures);
}

Expand Down
2 changes: 2 additions & 0 deletions naga/tests/in/spv/binding-arrays.dynamic.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
god_mode = true

[spv-in]
adjust_coordinate_space = true
2 changes: 2 additions & 0 deletions naga/tests/in/spv/binding-arrays.static.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
god_mode = true

[spv-in]
adjust_coordinate_space = true
1 change: 1 addition & 0 deletions naga/tests/in/wgsl/binding-buffer-arrays.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
targets = "WGSL | SPIRV"
god_mode = true

[bounds_check_policies]
index = "ReadZeroSkipWrite"
Expand Down
112 changes: 111 additions & 1 deletion naga/tests/naga/wgsl_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3093,6 +3093,19 @@ fn binding_array_non_struct() {
..
})
}

check_validation! {
r#"
enable wgpu_ray_query;
@group(0) @binding(0)
var<storage> ray_query_array: binding_array<ray_query, 10>;
"#:
Err(naga::valid::ValidationError::Type {
source: naga::valid::TypeError::BindingArrayBaseTypeNotStruct(_),
..
}),
Capabilities::RAY_QUERY
}
}

#[test]
Expand Down Expand Up @@ -4358,7 +4371,7 @@ fn ray_query_vertex_return_enable_extension() {
check_extension_validation!(
Capabilities::RAY_HIT_VERTEX_POSITION,
r#"enable wgpu_ray_query;
@group(0) @binding(0)
var acc_struct: acceleration_structure<vertex_return>;
"#,
Expand All @@ -4379,3 +4392,100 @@ fn ray_query_vertex_return_enable_extension() {
})
);
}

#[test]
fn binding_array_requires_capability() {
check_validation! {
r#"
struct Buffer { data: u32 }
@group(0) @binding(0)
var<storage> storage_array: binding_array<Buffer, 10>;
"#:
Err(naga::valid::ValidationError::GlobalVariable {
source: naga::valid::GlobalVariableError::UnsupportedCapability(
Capabilities::STORAGE_BUFFER_BINDING_ARRAY
),
..
})
}

check_validation! {
r#"
struct Buffer { data: u32 }
@group(0) @binding(0)
var<uniform> uniform_array: binding_array<Buffer, 10>;
"#:
Err(naga::valid::ValidationError::GlobalVariable {
source: naga::valid::GlobalVariableError::UnsupportedCapability(
Capabilities::BUFFER_BINDING_ARRAY
),
..
})
}

check_validation! {
r#"
@group(0) @binding(0)
var storage_texture_array: binding_array<texture_storage_2d<rgba8unorm, write>, 10>;
"#:
Err(naga::valid::ValidationError::GlobalVariable {
source: naga::valid::GlobalVariableError::UnsupportedCapability(
Capabilities::STORAGE_TEXTURE_BINDING_ARRAY
),
..
})
}

check_validation! {
r#"
@group(0) @binding(0)
var sampled_texture_array: binding_array<texture_2d<f32>, 10>;
"#:
Err(naga::valid::ValidationError::GlobalVariable {
source: naga::valid::GlobalVariableError::UnsupportedCapability(
Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY
),
..
})
}

check_validation! {
r#"
@group(0) @binding(0)
var sampler_array: binding_array<sampler, 10>;
"#:
Err(naga::valid::ValidationError::GlobalVariable {
source: naga::valid::GlobalVariableError::UnsupportedCapability(
Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY
),
..
})
}

// Binding arrays of external textures are not yet supported.
check_validation! {
r#"
@group(0) @binding(0)
var external_texture_array: binding_array<texture_external, 10>;
"#:
Err(naga::valid::ValidationError::Type {
source: naga::valid::TypeError::BindingArrayBaseExternalTextures,
..
}),
Capabilities::TEXTURE_EXTERNAL
}

// Acceleration structures are not allowed in binding arrays
check_validation! {
r#"
enable wgpu_ray_query;
@group(0) @binding(0)
var acc_struct_array: binding_array<acceleration_structure, 10>;
"#:
Err(naga::valid::ValidationError::GlobalVariable {
source: naga::valid::GlobalVariableError::InvalidBindingArray(_),
..
}),
Capabilities::all()
}
}
Loading
Loading