[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 all 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.

naga/src/back/spv/block.rs

@@ -1773,15 +1773,38 @@ 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);
 
                     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) => {
@@ -1868,7 +1891,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> {

naga/src/back/spv/index.rs

@@ -479,11 +479,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(
@@ -517,6 +516,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) => {

naga/tests/out/spv/bounds-check-restrict.spvasm

@@ -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
-OpFunctionEnd
-%171 = OpFunction  %2  None %158
-%170 = OpFunctionParameter  %3
-%169 = OpLabel
-OpBranch %172
+%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
+%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

naga/tests/out/spv/bounds-check-zero.spvasm

@@ -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