Fix access chain struct checks

Fixes https://crbug.com/oss-fuzz/66948 * Negative indices are invalid for struct access * Fix typos
alan-baker · Feb 22, 2024 · e239c76 · e239c76
1 parent 1b643ea
commit e239c76
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 4 deletions.
diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp
@@ -1386,10 +1386,10 @@ spv_result_t ValidateAccessChain(ValidationState_t& _,
         // should be less than the number of struct members.
         const int64_t num_struct_members =
             static_cast<int64_t>(type_pointee->words().size() - 2);
-        if (cur_index >= num_struct_members) {
+        if (cur_index >= num_struct_members || cur_index < 0) {
           return _.diag(SPV_ERROR_INVALID_ID, cur_word_instr)
                  << "Index is out of bounds: " << instr_name
-                 << " can not find index " << cur_index
+                 << " cannot find index " << cur_index
                  << " into the structure <id> "
                  << _.getIdName(type_pointee->id()) << ". This structure has "
                  << num_struct_members << " members. Largest valid index is "
@@ -1410,7 +1410,7 @@ spv_result_t ValidateAccessChain(ValidationState_t& _,
       }
     }
   }
-  // At this point, we have fully walked down from the base using the indeces.
+  // At this point, we have fully walked down from the base using the indices.
   // The type being pointed to should be the same as the result type.
   if (type_pointee->id() != result_type_pointee->id()) {
     return _.diag(SPV_ERROR_INVALID_ID, inst)

diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
@@ -4225,7 +4225,7 @@ OpReturn
 OpFunctionEnd
   )";
   const std::string expected_err = "Index is out of bounds: " + instr +
-                                   " can not find index 3 into the structure "
+                                   " cannot find index 3 into the structure "
                                    "<id> '25[%_struct_25]'. This structure "
                                    "has 3 members. Largest valid index is 2.";
   CompileSuccessfully(spirv);

diff --git a/test/val/val_memory_test.cpp b/test/val/val_memory_test.cpp
@@ -5115,6 +5115,64 @@ TEST_F(ValidateMemory, VulkanPtrAccessChainWorkgroupNoArrayStrideSuccess) {
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_2));
 }
 
+TEST_F(ValidateMemory, AccessChainNegativeStructIndex32) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%void = OpTypeVoid
+%void_fn = OpTypeFunction %void
+%int = OpTypeInt 32 1
+%_struct_4 = OpTypeStruct %int %int %int
+%_ptr_Function__struct_4 = OpTypePointer Function %_struct_4
+%_ptr_Function_int = OpTypePointer Function %int
+%int_n224 = OpConstant %int -224
+%fn = OpFunction %void Inline %void_fn
+%entry = OpLabel
+%var = OpVariable %_ptr_Function__struct_4 Function
+%gep = OpInBoundsAccessChain %_ptr_Function_int %var %int_n224
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Index is out of bounds"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("cannot find index -224"));
+}
+
+TEST_F(ValidateMemory, AccessChainNegativeStructIndex64) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability Linkage
+OpCapability Int64
+OpMemoryModel Logical GLSL450
+%void = OpTypeVoid
+%void_fn = OpTypeFunction %void
+%int = OpTypeInt 32 1
+%long = OpTypeInt 64 1
+%_struct_4 = OpTypeStruct %int %int %int
+%_ptr_Function__struct_4 = OpTypePointer Function %_struct_4
+%_ptr_Function_int = OpTypePointer Function %int
+%long_n224 = OpConstant %long -224
+%fn = OpFunction %void Inline %void_fn
+%entry = OpLabel
+%var = OpVariable %_ptr_Function__struct_4 Function
+%gep = OpInBoundsAccessChain %_ptr_Function_int %var %long_n224
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Index is out of bounds"));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("cannot find index -224"));
+}
+
 }  // namespace
 }  // namespace val
 }  // namespace spvtools