Properly check that user-defined IO uses IO-shareable types.

Only numeric scalars and vectors, and structs whose members are such
values, are permitted as the types of user-defined IO.
This commit is contained in:
Jim Blandy 2022-04-29 22:02:45 -07:00 committed by Teodor Tanasoaia
parent dec07027ee
commit 57e1793143
4 changed files with 142 additions and 8 deletions

View File

@ -67,6 +67,15 @@ impl super::ScalarValue {
}
}
impl super::ScalarKind {
pub const fn is_numeric(self) -> bool {
match self {
crate::ScalarKind::Sint | crate::ScalarKind::Uint | crate::ScalarKind::Float => true,
crate::ScalarKind::Bool => false,
}
}
}
pub const POINTER_SPAN: u32 = 4;
impl super::TypeInner {

View File

@ -39,6 +39,8 @@ pub enum GlobalVariableError {
pub enum VaryingError {
#[error("The type {0:?} does not match the varying")]
InvalidType(Handle<crate::Type>),
#[error("The type {0:?} cannot be used for user-defined entry point inputs or outputs")]
NotIOShareableType(Handle<crate::Type>),
#[error("Interpolation is not valid")]
InvalidInterpolation,
#[error("Interpolation must be specified on vertex shader outputs and fragment shader inputs")]
@ -101,6 +103,7 @@ struct VaryingContext<'a> {
stage: crate::ShaderStage,
output: bool,
types: &'a UniqueArena<crate::Type>,
type_info: &'a Vec<super::r#type::TypeInfo>,
location_mask: &'a mut BitSet,
built_ins: &'a mut crate::FastHashSet<crate::BuiltIn>,
capabilities: Capabilities,
@ -270,6 +273,13 @@ impl VaryingContext<'_> {
interpolation,
sampling,
} => {
// Only IO-shareable types may be stored in locations.
if !self.type_info[ty.index()]
.flags
.contains(super::TypeFlags::IO_SHAREABLE)
{
return Err(VaryingError::NotIOShareableType(ty));
}
if !self.location_mask.insert(location as usize) {
return Err(VaryingError::BindingCollision { location });
}
@ -485,6 +495,7 @@ impl super::Validator {
stage: ep.stage,
output: false,
types: &module.types,
type_info: &self.types,
location_mask: &mut self.location_mask,
built_ins: &mut argument_built_ins,
capabilities: self.capabilities,
@ -500,6 +511,7 @@ impl super::Validator {
stage: ep.stage,
output: true,
types: &module.types,
type_info: &self.types,
location_mask: &mut self.location_mask,
built_ins: &mut result_built_ins,
capabilities: self.capabilities,

View File

@ -227,13 +227,17 @@ impl super::Validator {
if !self.check_width(kind, width) {
return Err(TypeError::InvalidWidth(kind, width));
}
let shareable = if kind.is_numeric() {
TypeFlags::IO_SHAREABLE | TypeFlags::HOST_SHAREABLE
} else {
TypeFlags::empty()
};
TypeInfo::new(
TypeFlags::DATA
| TypeFlags::SIZED
| TypeFlags::COPY
| TypeFlags::IO_SHAREABLE
| TypeFlags::HOST_SHAREABLE
| TypeFlags::ARGUMENT,
| TypeFlags::ARGUMENT
| shareable,
width as u32,
)
}
@ -241,14 +245,19 @@ impl super::Validator {
if !self.check_width(kind, width) {
return Err(TypeError::InvalidWidth(kind, width));
}
let shareable = if kind.is_numeric() {
TypeFlags::IO_SHAREABLE | TypeFlags::HOST_SHAREABLE
} else {
TypeFlags::empty()
};
let count = if size >= crate::VectorSize::Tri { 4 } else { 2 };
TypeInfo::new(
TypeFlags::DATA
| TypeFlags::SIZED
| TypeFlags::COPY
| TypeFlags::IO_SHAREABLE
| TypeFlags::HOST_SHAREABLE
| TypeFlags::ARGUMENT,
| TypeFlags::ARGUMENT
| shareable,
count * (width as u32),
)
}
@ -265,7 +274,6 @@ impl super::Validator {
TypeFlags::DATA
| TypeFlags::SIZED
| TypeFlags::COPY
| TypeFlags::IO_SHAREABLE
| TypeFlags::HOST_SHAREABLE
| TypeFlags::ARGUMENT,
count * (width as u32),
@ -469,8 +477,7 @@ impl super::Validator {
}
};
let base_mask =
TypeFlags::COPY | TypeFlags::HOST_SHAREABLE | TypeFlags::IO_SHAREABLE;
let base_mask = TypeFlags::COPY | TypeFlags::HOST_SHAREABLE;
TypeInfo {
flags: TypeFlags::DATA | (base_info.flags & base_mask) | sized_flag,
uniform_layout,

View File

@ -1362,3 +1362,109 @@ fn wrong_access_mode() {
if name == "store"
}
}
#[test]
fn io_shareable_types() {
for numeric in "i32 u32 f32".split_whitespace() {
let types = format!(
"{} vec2<{}> vec3<{}> vec4<{}>",
numeric, numeric, numeric, numeric
);
for ty in types.split_whitespace() {
check_one_validation! {
&format!("@vertex
fn f(@location(0) arg: {}) -> @builtin(position) vec4<f32>
{{ return vec4<f32>(0.0); }}",
ty),
Ok(_module)
}
}
}
for ty in "bool
vec2<bool> vec3<bool> vec4<bool>
array<f32,4>
mat2x2<f32>
ptr<function,f32>"
.split_whitespace()
{
check_one_validation! {
&format!("@vertex
fn f(@location(0) arg: {}) -> @builtin(position) vec4<f32>
{{ return vec4<f32>(0.0); }}",
ty),
Err(
naga::valid::ValidationError::EntryPoint {
stage: naga::ShaderStage::Vertex,
name,
error: naga::valid::EntryPointError::Argument(
0,
naga::valid::VaryingError::NotIOShareableType(
_,
),
),
},
)
if name == "f"
}
}
}
#[test]
fn host_shareable_types() {
// Host-shareable, constructible types.
let types = "i32 u32 f32
vec2<i32> vec3<u32> vec4<f32>
mat4x4<f32>
array<mat4x4<f32>,4>
AStruct";
for ty in types.split_whitespace() {
check_one_validation! {
&format!("struct AStruct {{ member: array<mat4x4<f32>, 8> }};
@group(0) @binding(0) var<uniform> ubuf: {};
@group(0) @binding(1) var<storage> sbuf: {};",
ty, ty),
Ok(_module)
}
}
// Host-shareable but not constructible types.
let types = "atomic<i32> atomic<u32>
array<atomic<u32>,4>
array<u32>
AStruct";
for ty in types.split_whitespace() {
check_one_validation! {
&format!("struct AStruct {{ member: array<atomic<u32>, 8> }};
@group(0) @binding(1) var<storage> sbuf: {};",
ty),
Ok(_module)
}
}
// Types that are neither host-shareable nor constructible.
for ty in "bool ptr<storage,i32>".split_whitespace() {
check_one_validation! {
&format!("@group(0) @binding(0) var<storage> sbuf: {};", ty),
Err(
naga::valid::ValidationError::GlobalVariable {
name,
handle: _,
error: naga::valid::GlobalVariableError::MissingTypeFlags { .. },
},
)
if name == "sbuf"
}
check_one_validation! {
&format!("@group(0) @binding(0) var<uniform> ubuf: {};", ty),
Err(naga::valid::ValidationError::GlobalVariable {
name,
handle: _,
error: naga::valid::GlobalVariableError::MissingTypeFlags { .. },
},
)
if name == "ubuf"
}
}
}