Skip to content

Commit

Permalink
Check that stores are permitted by the pointer's storage access.
Browse files Browse the repository at this point in the history
Fixes #1533.
  • Loading branch information
jimblandy committed Nov 16, 2021
1 parent 1dcde48 commit 49a8ba9
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 4 deletions.
15 changes: 15 additions & 0 deletions src/proc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,21 @@ impl super::TypeInner {
}
}

impl super::StorageClass {
pub fn access(self) -> crate::StorageAccess {
use crate::StorageAccess as Sa;
match self {
crate::StorageClass::Function |
crate::StorageClass::Private |
crate::StorageClass::WorkGroup => Sa::LOAD | Sa::STORE,
crate::StorageClass::Uniform => Sa::LOAD,
crate::StorageClass::Storage { access } => access,
crate::StorageClass::Handle => Sa::LOAD,
crate::StorageClass::PushConstant => Sa::LOAD,
}
}
}

impl super::MathFunction {
pub fn argument_count(&self) -> usize {
match *self {
Expand Down
17 changes: 13 additions & 4 deletions src/valid/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,10 +606,11 @@ impl super::Validator {
}
_ => {}
}
let good = match *context
.resolve_pointer_type(pointer)
.map_err(|e| e.with_span())?
{

let pointer_ty = context.resolve_pointer_type(pointer)
.map_err(|e| e.with_span())?;

let good = match *pointer_ty {
Ti::Pointer { base, class: _ } => match context.types[base].inner {
Ti::Atomic { kind, width } => *value_ty == Ti::Scalar { kind, width },
ref other => value_ty == other,
Expand All @@ -634,6 +635,14 @@ impl super::Validator {
.with_handle(pointer, context.expressions)
.with_handle(value, context.expressions));
}

if let Some(class) = pointer_ty.pointer_class() {
if !class.access().contains(crate::StorageAccess::STORE) {
return Err(FunctionError::InvalidStorePointer(pointer)
.with_span_static(context.expressions.get_span(pointer),
"writing to this location is not permitted"));
}
}
}
S::ImageStore {
image,
Expand Down
43 changes: 43 additions & 0 deletions tests/wgsl-errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,3 +1029,46 @@ fn missing_default_case() {
)
}
}

#[test]
fn wrong_access_mode() {
// The assignments to `global.i` should be forbidden, because they are in
// variables whose access mode is `read`, not `read_write`.
check_validation_error! {
"
[[block]]
struct Globals {
i: i32;
};
[[group(0), binding(0)]]
var<storage> globals: Globals;
fn store(v: i32) {
globals.i = v;
}
",
"
[[block]]
struct Globals {
i: i32;
};
[[group(0), binding(0)]]
var<uniform> globals: Globals;
fn store(v: i32) {
globals.i = v;
}
":
Err(
naga::valid::ValidationError::Function {
name,
error: naga::valid::FunctionError::InvalidStorePointer(_),
..
},
)
if name == "store"
}
}

0 comments on commit 49a8ba9

Please sign in to comment.