mirror of
https://github.com/gfx-rs/wgpu.git
synced 2024-11-25 16:24:24 +00:00
[naga spv-out] Bounds-check runtime-sized array access correctly.
Do not neglect to apply bounds checks to indexing operations on runtime-sized arrays, even when they are accessed via an `AccessIndex` instruction. Before this commit, `BlockContext::write_expression_pointer` would not apply bounds checks to `OpAccessChain` indices provided by an `AccessIndex` instruction, apparently with the rationale that any out-of-bounds accesses should have been reported by constant evaluation. While it is true that the `index` operand of an `AccessIndex` expression is known at compile time, and that the WGSL constant evaluation rules require accesses that can be statically determined to be out-of-bounds to be shader creation or pipeline creation time errors, accesses to runtime-sized arrays don't follow this pattern: even if the index is known, the length with which it must be compared is not. Fixes #4441.
This commit is contained in:
Jim Blandy
parent
7283185305
commit
3693da27f9
@ -1828,6 +1828,7 @@ impl<'w> BlockContext<'w> {
|
||||
is_non_uniform_binding_array |=
|
||||
self.is_nonuniform_binding_array_access(base, index);
|
||||
|
||||
let index = crate::proc::index::GuardedIndex::Expression(index);
|
||||
let index_id =
|
||||
self.write_access_chain_index(base, index, &mut accumulated_checks, block)?;
|
||||
self.temp_list.push(index_id);
|
||||
@ -1835,8 +1836,30 @@ impl<'w> BlockContext<'w> {
|
||||
base
|
||||
}
|
||||
crate::Expression::AccessIndex { base, index } => {
|
||||
let const_id = self.get_index_constant(index);
|
||||
self.temp_list.push(const_id);
|
||||
// Decide whether we're indexing a struct (bounds checks
|
||||
// forbidden) or anything else (bounds checks required).
|
||||
let mut base_ty = self.fun_info[base].ty.inner_with(&self.ir_module.types);
|
||||
if let crate::TypeInner::Pointer { base, .. } = *base_ty {
|
||||
base_ty = &self.ir_module.types[base].inner;
|
||||
}
|
||||
let index_id = if let crate::TypeInner::Struct { .. } = *base_ty {
|
||||
self.get_index_constant(index)
|
||||
} else {
|
||||
// `index` is constant, so this can't possibly require
|
||||
// setting `is_nonuniform_binding_array_access`.
|
||||
|
||||
// Even though the index value is statically known, `base`
|
||||
// may be a runtime-sized array, so we still need to go
|
||||
// through the bounds check process.
|
||||
self.write_access_chain_index(
|
||||
base,
|
||||
crate::proc::index::GuardedIndex::Known(index),
|
||||
&mut accumulated_checks,
|
||||
block,
|
||||
)?
|
||||
};
|
||||
|
||||
self.temp_list.push(index_id);
|
||||
base
|
||||
}
|
||||
crate::Expression::GlobalVariable(handle) => {
|
||||
@ -1923,7 +1946,7 @@ impl<'w> BlockContext<'w> {
|
||||
fn write_access_chain_index(
|
||||
&mut self,
|
||||
base: Handle<crate::Expression>,
|
||||
index: Handle<crate::Expression>,
|
||||
index: crate::proc::index::GuardedIndex,
|
||||
accumulated_checks: &mut Option<Word>,
|
||||
block: &mut Block,
|
||||
) -> Result<Word, Error> {
|
||||
|
||||
@ -508,11 +508,10 @@ impl<'w> BlockContext<'w> {
|
||||
pub(super) fn write_bounds_check(
|
||||
&mut self,
|
||||
base: Handle<crate::Expression>,
|
||||
index: Handle<crate::Expression>,
|
||||
mut index: GuardedIndex,
|
||||
block: &mut Block,
|
||||
) -> Result<BoundsCheckResult, Error> {
|
||||
// If the value of `index` is known at compile time, find it now.
|
||||
let mut index = GuardedIndex::Expression(index);
|
||||
index.try_resolve_to_constant(self.ir_function, self.ir_module);
|
||||
|
||||
let policy = self.writer.bounds_check_policies.choose_policy(
|
||||
@ -546,6 +545,7 @@ impl<'w> BlockContext<'w> {
|
||||
let result_type_id = self.get_expression_type_id(&self.fun_info[expr_handle].ty);
|
||||
|
||||
let base_id = self.cached[base];
|
||||
let index = GuardedIndex::Expression(index);
|
||||
|
||||
let result_id = match self.write_bounds_check(base, index, block)? {
|
||||
BoundsCheckResult::KnownInBounds(known_index) => {
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
; SPIR-V
|
||||
; Version: 1.1
|
||||
; Generator: rspirv
|
||||
; Bound: 174
|
||||
; Bound: 180
|
||||
OpCapability Shader
|
||||
OpCapability Linkage
|
||||
OpExtension "SPV_KHR_storage_buffer_storage_class"
|
||||
@ -52,7 +52,7 @@ OpDecorate %12 Binding 0
|
||||
%129 = OpTypeFunction %2 %11 %7
|
||||
%138 = OpTypeFunction %2 %11 %11 %3
|
||||
%158 = OpTypeFunction %2 %3
|
||||
%166 = OpConstant %6 1000
|
||||
%168 = OpConstant %6 1000
|
||||
%16 = OpFunction %3 None %17
|
||||
%15 = OpFunctionParameter %11
|
||||
%14 = OpLabel
|
||||
@ -238,16 +238,22 @@ OpFunctionEnd
|
||||
%163 = OpLabel
|
||||
OpBranch %165
|
||||
%165 = OpLabel
|
||||
%167 = OpAccessChain %20 %12 %35 %166
|
||||
%168 = OpLoad %3 %167
|
||||
OpReturnValue %168
|
||||
%166 = OpArrayLength %6 %12 3
|
||||
%167 = OpISub %6 %166 %32
|
||||
%169 = OpExtInst %6 %1 UMin %168 %167
|
||||
%170 = OpAccessChain %20 %12 %35 %169
|
||||
%171 = OpLoad %3 %170
|
||||
OpReturnValue %171
|
||||
OpFunctionEnd
|
||||
%171 = OpFunction %2 None %158
|
||||
%170 = OpFunctionParameter %3
|
||||
%169 = OpLabel
|
||||
OpBranch %172
|
||||
%174 = OpFunction %2 None %158
|
||||
%173 = OpFunctionParameter %3
|
||||
%172 = OpLabel
|
||||
%173 = OpAccessChain %20 %12 %35 %166
|
||||
OpStore %173 %170
|
||||
OpBranch %175
|
||||
%175 = OpLabel
|
||||
%176 = OpArrayLength %6 %12 3
|
||||
%177 = OpISub %6 %176 %32
|
||||
%178 = OpExtInst %6 %1 UMin %168 %177
|
||||
%179 = OpAccessChain %20 %12 %35 %178
|
||||
OpStore %179 %173
|
||||
OpReturn
|
||||
OpFunctionEnd
|
||||
@ -1,7 +1,7 @@
|
||||
; SPIR-V
|
||||
; Version: 1.1
|
||||
; Generator: rspirv
|
||||
; Bound: 211
|
||||
; Bound: 220
|
||||
OpCapability Shader
|
||||
OpCapability Linkage
|
||||
OpExtension "SPV_KHR_storage_buffer_storage_class"
|
||||
@ -56,7 +56,7 @@ OpDecorate %12 Binding 0
|
||||
%159 = OpTypeFunction %2 %11 %7
|
||||
%170 = OpTypeFunction %2 %11 %11 %3
|
||||
%195 = OpTypeFunction %2 %3
|
||||
%203 = OpConstant %6 1000
|
||||
%204 = OpConstant %6 1000
|
||||
%16 = OpFunction %3 None %17
|
||||
%15 = OpFunctionParameter %11
|
||||
%14 = OpLabel
|
||||
@ -314,16 +314,31 @@ OpFunctionEnd
|
||||
%200 = OpLabel
|
||||
OpBranch %202
|
||||
%202 = OpLabel
|
||||
%204 = OpAccessChain %20 %12 %37 %203
|
||||
%205 = OpLoad %3 %204
|
||||
OpReturnValue %205
|
||||
%203 = OpArrayLength %6 %12 3
|
||||
%205 = OpULessThan %22 %204 %203
|
||||
OpSelectionMerge %207 None
|
||||
OpBranchConditional %205 %208 %207
|
||||
%208 = OpLabel
|
||||
%206 = OpAccessChain %20 %12 %37 %204
|
||||
%209 = OpLoad %3 %206
|
||||
OpBranch %207
|
||||
%207 = OpLabel
|
||||
%210 = OpPhi %3 %25 %202 %209 %208
|
||||
OpReturnValue %210
|
||||
OpFunctionEnd
|
||||
%208 = OpFunction %2 None %195
|
||||
%207 = OpFunctionParameter %3
|
||||
%206 = OpLabel
|
||||
OpBranch %209
|
||||
%209 = OpLabel
|
||||
%210 = OpAccessChain %20 %12 %37 %203
|
||||
OpStore %210 %207
|
||||
%213 = OpFunction %2 None %195
|
||||
%212 = OpFunctionParameter %3
|
||||
%211 = OpLabel
|
||||
OpBranch %214
|
||||
%214 = OpLabel
|
||||
%215 = OpArrayLength %6 %12 3
|
||||
%216 = OpULessThan %22 %204 %215
|
||||
OpSelectionMerge %218 None
|
||||
OpBranchConditional %216 %219 %218
|
||||
%219 = OpLabel
|
||||
%217 = OpAccessChain %20 %12 %37 %204
|
||||
OpStore %217 %212
|
||||
OpBranch %218
|
||||
%218 = OpLabel
|
||||
OpReturn
|
||||
OpFunctionEnd
|
||||
Loading…
Reference in New Issue
Block a user