Strict validation of where type ids are acceptable (#2142)

Fixes https://crbug.com/910239 * IdPass catches many instances of invalid references to types * Test updates * Added test to catch OpArrayLength issue
2024-11-27 17:40:28 +00:00 · 2018-12-03 11:03:52 -05:00 · 2018-12-03 11:03:52 -05:00 · d80259d35e
commit d80259d35e
parent b9f9a3bc9f
11 changed files with 116 additions and 112 deletions
--- a/source/val/validate_id.cpp
+++ b/source/val/validate_id.cpp
@ -164,8 +164,17 @@ spv_result_t IdPass(ValidationState_t& _, Instruction* inst) {
      case SPV_OPERAND_TYPE_ID:
      case SPV_OPERAND_TYPE_MEMORY_SEMANTICS_ID:
      case SPV_OPERAND_TYPE_SCOPE_ID:
-        if (_.IsDefinedId(operand_word)) {
-          ret = SPV_SUCCESS;
+        if (const auto def = _.FindDef(operand_word)) {
+          const auto opcode = inst->opcode();
+          if (spvOpcodeGeneratesType(def->opcode()) &&
+              !spvOpcodeGeneratesType(opcode) && !spvOpcodeIsDebug(opcode) &&
+              !spvOpcodeIsDecoration(opcode) && opcode != SpvOpFunction) {
+            return _.diag(SPV_ERROR_INVALID_ID, inst)
+                   << "Operand " << _.getIdName(operand_word)
+                   << " cannot be a type";
+          } else {
+            ret = SPV_SUCCESS;
+          }
        } else if (can_have_forward_declared_ids(i)) {
          ret = _.ForwardDeclareId(operand_word);
        } else {
--- a/test/val/val_arithmetics_test.cpp
+++ b/test/val/val_arithmetics_test.cpp
@ -605,10 +605,8 @@ TEST_F(ValidateArithmetics, DotNotVectorTypeOperand1) {
 )";

  CompileSuccessfully(GenerateCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("Expected float vector as operand: Dot operand index 2"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 6 cannot be a type"));
 }

 TEST_F(ValidateArithmetics, DotNotVectorTypeOperand2) {
--- a/test/val/val_atomics_test.cpp
+++ b/test/val/val_atomics_test.cpp
@ -379,10 +379,8 @@ TEST_F(ValidateAtomics, AtomicLoadWrongPointerType) {
 )";

  CompileSuccessfully(GenerateKernelCode(body));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("AtomicLoad: expected Pointer to be of type OpTypePointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 27 cannot be a type"));
 }

 TEST_F(ValidateAtomics, AtomicLoadWrongPointerDataType) {
@ -624,11 +622,8 @@ TEST_F(ValidateAtomics, AtomicExchangeWrongPointerType) {
 )";

  CompileSuccessfully(GenerateKernelCode(body));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr(
-          "AtomicExchange: expected Pointer to be of type OpTypePointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 33 cannot be a type"));
 }

 TEST_F(ValidateAtomics, AtomicExchangeWrongPointerDataType) {
@ -740,10 +735,8 @@ TEST_F(ValidateAtomics, AtomicCompareExchangeWrongPointerType) {
 )";

  CompileSuccessfully(GenerateKernelCode(body));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("AtomicCompareExchange: expected Pointer to be of type "
-                        "OpTypePointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 33 cannot be a type"));
 }

 TEST_F(ValidateAtomics, AtomicCompareExchangeWrongPointerDataType) {
--- a/test/val/val_barriers_test.cpp
+++ b/test/val/val_barriers_test.cpp
@ -874,10 +874,8 @@ OpMemoryBarrier %u32 %u32_0
 )";

  CompileSuccessfully(GenerateKernelCode(body));
-  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("MemoryBarrier: expected Memory Scope to be a 32-bit int"));
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 5 cannot be a type"));
 }

 TEST_F(ValidateBarriers,
--- a/test/val/val_composites_test.cpp
+++ b/test/val/val_composites_test.cpp
@ -320,11 +320,8 @@ TEST_F(ValidateComposites, CompositeConstructVectorWrongConsituent1) {
 )";

  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("Expected Constituents to be scalars or vectors of the same "
-                "type as Result Type components"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 5 cannot be a type"));
 }

 TEST_F(ValidateComposites, CompositeConstructVectorWrongConsituent2) {
@ -660,10 +657,8 @@ TEST_F(ValidateComposites, CompositeExtractNotObject) {
 )";

  CompileSuccessfully(GenerateShaderCode(body));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Expected Composite to be an object "
-                        "of composite type"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 11 cannot be a type"));
 }

 TEST_F(ValidateComposites, CompositeExtractNotComposite) {
--- a/test/val/val_conversion_test.cpp
+++ b/test/val/val_conversion_test.cpp
@ -972,9 +972,8 @@ TEST_F(ValidateConversion, PtrCastToGenericWrongInputType) {
 )";

  CompileSuccessfully(GenerateKernelCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Expected input to be a pointer: PtrCastToGeneric"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 4 cannot be a type"));
 }

 TEST_F(ValidateConversion, PtrCastToGenericWrongInputStorageClass) {
@ -1208,9 +1207,8 @@ TEST_F(ValidateConversion, BitcastInputHasNoType) {
 )";

  CompileSuccessfully(GenerateKernelCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Expected input to have a type: Bitcast"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 4 cannot be a type"));
 }

 TEST_F(ValidateConversion, BitcastWrongResultType) {
@ -1297,9 +1295,8 @@ OpFunctionEnd
 )";

  CompileSuccessfully(spirv);
-  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Expected int scalar as input: ConvertUToPtr"));
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 1 cannot be a type"));
 }

 }  // namespace
--- a/test/val/val_derivatives_test.cpp
+++ b/test/val/val_derivatives_test.cpp
@ -119,11 +119,8 @@ TEST_F(ValidateDerivatives, OpDPdxWrongResultType) {
 )";

  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("Expected Result Type to be float scalar or vector type: "
-                "DPdx"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 10 cannot be a type"));
 }

 TEST_F(ValidateDerivatives, OpDPdxWrongPType) {
--- a/test/val/val_ext_inst_test.cpp
+++ b/test/val/val_ext_inst_test.cpp
@ -3335,7 +3335,7 @@ TEST_P(ValidateOpenCLStdUMul24Like, FloatResultType) {
 TEST_P(ValidateOpenCLStdUMul24Like, U64ResultType) {
  const std::string ext_inst_name = GetParam();
  const std::string body =
-      "%val1 = OpExtInst %u64 %extinst " + ext_inst_name + " %u64_0 %u64\n";
+      "%val1 = OpExtInst %u64 %extinst " + ext_inst_name + " %u64_0 %u64_0\n";

  CompileSuccessfully(GenerateKernelCode(body));
  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
@ -4133,10 +4133,8 @@ TEST_P(ValidateOpenCLStdVStoreHalfLike, PNotPointer) {
  }

  CompileSuccessfully(GenerateKernelCode(ss.str()));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpenCL.std " + ext_inst_name +
-                        ": expected operand P to be a pointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 89 cannot be a type"));
 }

 TEST_P(ValidateOpenCLStdVStoreHalfLike, ConstPointer) {
@ -4306,10 +4304,8 @@ TEST_P(ValidateOpenCLStdVLoadHalfLike, PNotPointer) {
     << " %u32_1 %f16_ptr_workgroup 2\n";

  CompileSuccessfully(GenerateKernelCode(ss.str()));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpenCL.std " + ext_inst_name +
-                        ": expected operand P to be a pointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 89 cannot be a type"));
 }

 TEST_P(ValidateOpenCLStdVLoadHalfLike, OffsetWrongStorageType) {
@ -4479,10 +4475,8 @@ TEST_F(ValidateExtInst, VLoadNPNotPointer) {
        "%f32_ptr_uniform_constant 2\n";

  CompileSuccessfully(GenerateKernelCode(ss.str()));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("OpenCL.std vloadn: expected operand P to be a pointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 120 cannot be a type"));
 }

 TEST_F(ValidateExtInst, VLoadNWrongStorageClass) {
@ -4592,10 +4586,8 @@ TEST_F(ValidateExtInst, VLoadHalfPNotPointer) {
        "%f16_ptr_uniform_constant\n";

  CompileSuccessfully(GenerateKernelCode(ss.str()));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("OpenCL.std vload_half: expected operand P to be a pointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 114 cannot be a type"));
 }

 TEST_F(ValidateExtInst, VLoadHalfWrongStorageClass) {
@ -4746,10 +4738,8 @@ TEST_F(ValidateExtInst, VStoreNPNotPointer) {
        "%f32_ptr_generic\n";

  CompileSuccessfully(GenerateKernelCode(ss.str()));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("OpenCL.std vstoren: expected operand P to be a pointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 124 cannot be a type"));
 }

 TEST_F(ValidateExtInst, VStoreNPNotGeneric) {
@ -5061,10 +5051,8 @@ TEST_F(ValidateExtInst, OpenCLStdPrintfFormatNotPointer) {
 )";

  CompileSuccessfully(GenerateKernelCode(body));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("OpenCL.std printf: expected operand Format to be a pointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 134 cannot be a type"));
 }

 TEST_F(ValidateExtInst, OpenCLStdPrintfFormatNotUniformConstStorageClass) {
@ -5154,10 +5142,8 @@ TEST_F(ValidateExtInst, OpenCLStdPrefetchPtrNotPointer) {
 )";

  CompileSuccessfully(GenerateKernelCode(body));
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("OpenCL.std prefetch: expected operand Ptr to be a pointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 99 cannot be a type"));
 }

 TEST_F(ValidateExtInst, OpenCLStdPrefetchPtrNotCrossWorkgroup) {
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@ -1207,16 +1207,28 @@ TEST_F(ValidateIdWithMessage, OpConstantCompositeArrayWithUndefGood) {
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
-TEST_F(ValidateIdWithMessage, OpConstantCompositeArrayConstConstituentBad) {
+TEST_F(ValidateIdWithMessage, OpConstantCompositeArrayConstConstituentTypeBad) {
  std::string spirv = kGLSL450MemoryModel + R"(
 %1 = OpTypeInt 32 0
 %2 = OpConstant %1 4
 %3 = OpTypeArray %1 %2
 %4 = OpConstantComposite %3 %2 %2 %2 %1)";  // Uses a type as operand
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 1 cannot be a type"));
+}
+TEST_F(ValidateIdWithMessage, OpConstantCompositeArrayConstConstituentBad) {
+  std::string spirv = kGLSL450MemoryModel + R"(
+%1 = OpTypeInt 32 0
+%2 = OpConstant %1 4
+%3 = OpTypeArray %1 %2
+%4 = OpTypePointer Uniform %1
+%5 = OpVariable %4 Uniform
+%6 = OpConstantComposite %3 %2 %2 %2 %5)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpConstantComposite Constituent <id> '1' is not a "
+              HasSubstr("OpConstantComposite Constituent <id> '5' is not a "
                        "constant or undef."));
 }
 TEST_F(ValidateIdWithMessage, OpConstantCompositeArrayConstituentTypeBad) {
@ -1524,11 +1536,13 @@ TEST_F(ValidateIdWithMessage,
 %2 = OpTypeVector %1 4
 %3 = OpTypeInt 32 0
 %4 = OpSpecConstant %1 3.14
-%6 = OpSpecConstantComposite %2 %3 %4 %4 %4)";
+%5 = OpTypePointer Uniform %1
+%6 = OpVariable %5 Uniform
+%7 = OpSpecConstantComposite %2 %6 %4 %4 %4)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpSpecConstantComposite Constituent <id> '3' is not a "
+              HasSubstr("OpSpecConstantComposite Constituent <id> '6' is not a "
                        "constant or undef."));
 }

@ -1651,11 +1665,13 @@ TEST_F(ValidateIdWithMessage,
 %3 = OpTypeVector %1 4
 %4 = OpTypeMatrix %3 4
 %5 = OpSpecConstantComposite %3 %2 %2 %2 %2
- %6 = OpSpecConstantComposite %4 %5 %5 %5 %1)";
+ %6 = OpTypePointer Uniform %1
+ %7 = OpVariable %6 Uniform
+ %8 = OpSpecConstantComposite %4 %5 %5 %5 %7)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpSpecConstantComposite Constituent <id> '1' is not a "
+              HasSubstr("OpSpecConstantComposite Constituent <id> '7' is not a "
                        "constant composite or undef."));
 }

@ -1770,11 +1786,13 @@ TEST_F(ValidateIdWithMessage, OpSpecConstantCompositeArrayConstConstituentBad) {
 %1 = OpTypeInt 32 0
 %2 = OpConstant %1 4
 %3 = OpTypeArray %1 %2
-%4 = OpSpecConstantComposite %3 %2 %2 %2 %1)";
+%4 = OpTypePointer Uniform %1
+%5 = OpVariable %4 Uniform
+%6 = OpSpecConstantComposite %3 %2 %2 %2 %5)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpSpecConstantComposite Constituent <id> '1' is not a "
+              HasSubstr("OpSpecConstantComposite Constituent <id> '5' is not a "
                        "constant or undef."));
 }

@ -1864,11 +1882,13 @@ TEST_F(ValidateIdWithMessage, OpSpecConstantCompositeStructNonConstBad) {
 %3 = OpTypeStruct %1 %1 %2
 %4 = OpSpecConstant %1 42
 %5 = OpUndef %2
-%6 = OpSpecConstantComposite %3 %4 %1 %5)";
+%6 = OpTypePointer Uniform %1
+%7 = OpVariable %6 Uniform
+%8 = OpSpecConstantComposite %3 %4 %7 %5)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpSpecConstantComposite Constituent <id> '1' is not a "
+              HasSubstr("OpSpecConstantComposite Constituent <id> '7' is not a "
                        "constant or undef."));
 }

@ -1955,9 +1975,7 @@ TEST_F(ValidateIdWithMessage, OpVariableInitializerIsTypeBad) {
 %3 = OpVariable %2 Input %2)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpVariable Initializer <id> '2' is not a constant or "
-                        "module-scope variable"));
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 2 cannot be a type"));
 }

 TEST_F(ValidateIdWithMessage, OpVariableInitializerIsFunctionVarBad) {
@ -2444,15 +2462,16 @@ TEST_F(ValidateIdWithMessage, OpStorePointerBad) {
 %4 = OpTypeFunction %1
 %5 = OpConstant %2 42
 %6 = OpVariable %3 UniformConstant
-%7 = OpFunction %1 None %4
-%8 = OpLabel
-     OpStore %3 %5
+%7 = OpConstant %2 0
+%8 = OpFunction %1 None %4
+%9 = OpLabel
+     OpStore %7 %5
     OpReturn
     OpFunctionEnd)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("OpStore Pointer <id> '3' is not a logical pointer."));
+              HasSubstr("OpStore Pointer <id> '7' is not a logical pointer."));
 }

 // Disabled as bitcasting type to object is now not valid.
@ -3339,6 +3358,7 @@ const char kDeeplyNestedStructureSetup[] = R"(
 %_ptr_Uniform_v3float = OpTypePointer Uniform %v3float
 %blockName_var = OpVariable %_ptr_Uniform_blockName Uniform
 %spec_int = OpSpecConstant %int 2
+%float_0 = OpConstant %float 0
 %func = OpFunction %void None %void_f
 %my_label = OpLabel
 )";
@ -3383,7 +3403,7 @@ OpFunctionEnd
  )";

  const std::string expected_err = "The Result Type of " + instr +
-                                   " <id> '35' must be "
+                                   " <id> '36' must be "
                                   "OpTypePointer. Found OpTypeFloat.";
  CompileSuccessfully(spirv);
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
@ -3401,12 +3421,9 @@ TEST_P(AccessChainInstructionTest, AccessChainBaseTypeVoidBad) {
 OpReturn
 OpFunctionEnd
  )";
-  const std::string expected_err = "The Base <id> '1' in " + instr +
-                                   " instruction must "
-                                   "be a pointer.";
  CompileSuccessfully(spirv);
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(), HasSubstr(expected_err));
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 1 cannot be a type"));
 }

 // Invalid. The base type of an access chain instruction must be a pointer.
@ -3421,12 +3438,9 @@ TEST_P(AccessChainInstructionTest, AccessChainBaseTypeNonPtrVariableBad) {
 OpReturn
 OpFunctionEnd
  )";
-  const std::string expected_err = "The Base <id> '8' in " + instr +
-                                   " instruction must "
-                                   "be a pointer.";
  CompileSuccessfully(spirv);
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(), HasSubstr(expected_err));
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 8 cannot be a type"));
 }

 // Invalid: The storage class of Base and Result do not match.
@ -3645,7 +3659,7 @@ TEST_P(AccessChainInstructionTest, AccessChainUndefinedIndexBad) {
  std::string spirv = kGLSL450MemoryModel + kDeeplyNestedStructureSetup + R"(
 %entry = )" +
                      instr + R"( %_ptr_Private_float %my_matrix )" + elem +
-                      R"(%float %int_1
+                      R"(%float_0 %int_1
 OpReturn
 OpFunctionEnd
  )";
@ -4842,10 +4856,7 @@ OpBranchConditional %bool %target_t %target_f

  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr(
-          "Condition operand for OpBranchConditional must be of boolean type"));
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 3 cannot be a type"));
 }

 // TODO: OpSwitch
@ -4907,9 +4918,7 @@ TEST_F(ValidateIdWithMessage, OpReturnValueIsType) {
     OpFunctionEnd)";
  CompileSuccessfully(spirv.c_str());
  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("OpReturnValue Value <id> '1' does not represent a value."));
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 1 cannot be a type"));
 }

 TEST_F(ValidateIdWithMessage, OpReturnValueIsLabel) {
--- a/test/val/val_image_test.cpp
+++ b/test/val/val_image_test.cpp
@ -671,9 +671,8 @@ TEST_F(ValidateImage, ImageTexelPointerImageNotResultTypePointer) {
 )";

  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Expected Image to be OpTypePointer"));
+  ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 136 cannot be a type"));
 }

 TEST_F(ValidateImage, ImageTexelPointerImageNotImage) {
--- a/test/val/val_memory_test.cpp
+++ b/test/val/val_memory_test.cpp
@ -839,6 +839,29 @@ TEST_F(ValidateMemory, ArrayLenIndexNotPointerToStruct) {
          "an OpTypeStruct.\n  %12 = OpArrayLength %uint %11 0\n"));
 }

+TEST_F(ValidateMemory, ArrayLenPointerIsAType) {
+  std::string spirv = R"(
+               OpCapability Shader
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %1 "main"
+               OpExecutionMode %1 OriginUpperLeft
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+      %float = OpTypeFloat 32
+       %uint = OpTypeInt 32 0
+          %1 = OpFunction %void None %3
+          %9 = OpLabel
+         %12 = OpArrayLength %uint %float 0
+               OpReturn
+               OpFunctionEnd
+
+)";
+
+  CompileSuccessfully(spirv.c_str());
+  EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 4 cannot be a type"));
+}
+
 TEST_F(ValidateMemory, PushConstantNotStructGood) {
  std::string spirv = R"(
            OpCapability Shader