Skip to content

Commit 6e2394b

Browse files
authored
[naga] Validate override array sizes. (#6882)
When an array type `A`'s size is `PendingArraySize::Override(h)`, verify that: - `h` is a valid override handle, - the override's type precedes `A` in the type arena, and - the override's expression does not participate in a type/global expression cycle. Fixes #6880.
1 parent e6d2a6e commit 6e2394b

File tree

1 file changed

+93
-3
lines changed

1 file changed

+93
-3
lines changed

naga/src/valid/handles.rs

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ impl super::Validator {
9292
// types and expressions behind `global_exprs_iter`, and none of
9393
// those refer to `th`/`t`, because they passed the same checks
9494
// before we reached `th`/`t`.
95-
if let Some(max_expr) = Self::validate_type_handles((th, t))? {
95+
if let Some(max_expr) = Self::validate_type_handles((th, t), overrides)? {
9696
max_expr.check_valid_for(global_expressions)?;
9797
// Since `t` refers to `max_expr`, if we want our invariants to
9898
// remain true, we must advance `global_exprs_iter` beyond
@@ -299,6 +299,7 @@ impl super::Validator {
299299
/// expression and type arenas.
300300
fn validate_type_handles(
301301
(handle, ty): (Handle<crate::Type>, &crate::Type),
302+
overrides: &Arena<crate::Override>,
302303
) -> Result<Option<Handle<crate::Expression>>, InvalidHandleError> {
303304
let max_expr = match ty.inner {
304305
crate::TypeInner::Scalar { .. }
@@ -320,7 +321,12 @@ impl super::Validator {
320321
match size {
321322
crate::ArraySize::Pending(pending) => match pending {
322323
crate::PendingArraySize::Expression(expr) => Some(expr),
323-
crate::PendingArraySize::Override(_) => None,
324+
crate::PendingArraySize::Override(h) => {
325+
Self::validate_override_handle(h, overrides)?;
326+
let override_ = &overrides[h];
327+
handle.check_dep(override_.ty)?;
328+
override_.init
329+
}
324330
},
325331
crate::ArraySize::Constant(_) | crate::ArraySize::Dynamic => None,
326332
}
@@ -890,7 +896,7 @@ fn constant_deps() {
890896
}
891897

892898
#[test]
893-
fn override_deps() {
899+
fn array_size_deps() {
894900
use super::Validator;
895901
use crate::{ArraySize, Expression, PendingArraySize, Scalar, Span, Type, TypeInner};
896902

@@ -928,3 +934,87 @@ fn override_deps() {
928934
m.global_expressions[ex_zero] = Expression::ZeroValue(ty_arr);
929935
assert!(Validator::validate_module_handles(&m).is_err());
930936
}
937+
938+
#[test]
939+
fn array_size_override() {
940+
use super::Validator;
941+
use crate::{ArraySize, Override, PendingArraySize, Scalar, Span, Type, TypeInner};
942+
943+
let nowhere = Span::default();
944+
945+
let mut m = crate::Module::default();
946+
947+
let ty_u32 = m.types.insert(
948+
Type {
949+
name: Some("u32".to_string()),
950+
inner: TypeInner::Scalar(Scalar::U32),
951+
},
952+
nowhere,
953+
);
954+
955+
let bad_override: Handle<Override> = Handle::new(NonMaxU32::new(1000).unwrap());
956+
let _ty_arr = m.types.insert(
957+
Type {
958+
name: Some("bad_array".to_string()),
959+
inner: TypeInner::Array {
960+
base: ty_u32,
961+
size: ArraySize::Pending(PendingArraySize::Override(bad_override)),
962+
stride: 4,
963+
},
964+
},
965+
nowhere,
966+
);
967+
968+
assert!(Validator::validate_module_handles(&m).is_err());
969+
}
970+
971+
#[test]
972+
fn override_init_deps() {
973+
use super::Validator;
974+
use crate::{ArraySize, Expression, Override, PendingArraySize, Scalar, Span, Type, TypeInner};
975+
976+
let nowhere = Span::default();
977+
978+
let mut m = crate::Module::default();
979+
980+
let ty_u32 = m.types.insert(
981+
Type {
982+
name: Some("u32".to_string()),
983+
inner: TypeInner::Scalar(Scalar::U32),
984+
},
985+
nowhere,
986+
);
987+
let ex_zero = m
988+
.global_expressions
989+
.append(Expression::ZeroValue(ty_u32), nowhere);
990+
let r#override = m.overrides.append(
991+
Override {
992+
name: Some("bad_override".into()),
993+
id: None,
994+
ty: ty_u32,
995+
init: Some(ex_zero),
996+
},
997+
nowhere,
998+
);
999+
let ty_arr = m.types.insert(
1000+
Type {
1001+
name: Some("bad_array".to_string()),
1002+
inner: TypeInner::Array {
1003+
base: ty_u32,
1004+
size: ArraySize::Pending(PendingArraySize::Override(r#override)),
1005+
stride: 4,
1006+
},
1007+
},
1008+
nowhere,
1009+
);
1010+
let ex_arr = m
1011+
.global_expressions
1012+
.append(Expression::ZeroValue(ty_arr), nowhere);
1013+
1014+
assert!(Validator::validate_module_handles(&m).is_ok());
1015+
1016+
// Mutate `r#override`'s initializer to `ex_arr`, introducing a cycle.
1017+
// Validation should catch the cycle.
1018+
m.overrides[r#override].init = Some(ex_arr);
1019+
assert!(Validator::validate_module_handles(&m).is_err());
1020+
}

0 commit comments

Comments
 (0)