spvBinaryParse: fix overruns, handle image format
authorDavid Neto <dneto@google.com>
Tue, 24 Nov 2015 23:37:24 +0000 (18:37 -0500)
committerDavid Neto <dneto@google.com>
Mon, 30 Nov 2015 15:44:23 +0000 (10:44 -0500)
Add unit tests for all diagnostics issued by spvBinaryParse.

Handle image format operands in the binary parser and the
disassembler.

Document that the callback function pointers can be null,
in which case they are ignored.

Detect exhaustion of input when parsing an operand,
to avoid buffer overruns on some invalid input cases.

Fix the description strings for some operand types.
Make the diagnostic messages for those operand types
consistent between the assembler and binary parser.

Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/29

include/libspirv/libspirv.h
source/binary.cpp
source/disassemble.cpp
source/operand.cpp
source/text.cpp
test/BinaryParse.cpp
test/TextToBinary.Annotation.cpp
test/TextToBinary.Constant.cpp
test/TextToBinary.ControlFlow.cpp
test/TextToBinary.Function.cpp
test/TextToBinary.TypeDeclaration.cpp

index 26f71a4..c23cde9 100644 (file)
@@ -412,7 +412,9 @@ typedef spv_result_t (*spv_parsed_instruction_fn_t)(
     void* user_data, const spv_parsed_instruction_t* parsed_instruction);
 
 // Parses a SPIR-V binary, specified as counted sequence of 32-bit words.
-// Parsing feedback is provided via two callbacks.  In a valid parse, the
+// Parsing feedback is provided via two callbacks provided as function
+// pointers.  Each callback function pointer can be a null pointer, in
+// which case it is never called.  Otherwise, in a valid parse the
 // parsed-header callback is called once, and then the parsed-instruction
 // callback once for each instruction in the stream.  The user_data parameter
 // is supplied as context to the callbacks.  Returns SPV_SUCCESS on successful
index 49daf42..918b908 100755 (executable)
@@ -182,6 +182,20 @@ class Parser {
     return diagnostic(SPV_ERROR_INVALID_BINARY);
   }
 
+  // Issues a diagnostic describing an exhaustion of input condition when
+  // trying to decode an instruction operand, and returns
+  // SPV_ERROR_INVALID_BINARY.
+  spv_result_t exhaustedInputDiagnostic(size_t inst_offset, SpvOp opcode,
+                                        spv_operand_type_t type) {
+    return diagnostic() << "End of input reached while decoding Op"
+                        << spvOpcodeString(opcode) << " starting at word "
+                        << inst_offset
+                        << ((_.word_index < _.num_words) ? ": truncated "
+                                                         : ": missing ")
+                        << spvOperandTypeStr(type) << " operand at word offset "
+                        << _.word_index - inst_offset << ".";
+  }
+
   // Returns the endian-corrected word at the current position.
   uint32_t peek() const { return peekAt(_.word_index); }
 
@@ -359,8 +373,9 @@ spv_result_t Parser::parseInstruction() {
 
     if (auto error =
             parseOperand(inst_offset, &inst, type, &endian_converted_words,
-                         &operands, &expected_operands))
+                         &operands, &expected_operands)) {
       return error;
+    }
   }
 
   if (!expected_operands.empty() &&
@@ -372,8 +387,9 @@ spv_result_t Parser::parseInstruction() {
   }
 
   if ((inst_offset + inst_word_count) != _.word_index) {
-    return diagnostic() << "Invalid word count: Instruction starting at word "
-                        << inst_offset << " says it has " << inst_word_count
+    return diagnostic() << "Invalid word count: Op" << opcode_desc->name
+                        << " starting at word " << inst_offset
+                        << " says it has " << inst_word_count
                         << " words, but found " << _.word_index - inst_offset
                         << " words instead.";
   }
@@ -436,6 +452,9 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
   parsed_operand.number_kind = SPV_NUMBER_NONE;
   parsed_operand.number_bit_width = 0;
 
+  if (_.word_index >= _.num_words)
+    return exhaustedInputDiagnostic(inst_offset, inst->opcode, type);
+
   const uint32_t word = peek();
 
   // Do the words in this operand have to be converted to native endianness?
@@ -485,7 +504,9 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
 
     case SPV_OPERAND_TYPE_SCOPE_ID:
     case SPV_OPERAND_TYPE_MEMORY_SEMANTICS_ID:
-      if (!word) return diagnostic() << spvOperandTypeStr(type) << " Id is 0";
+      // Check for trivially invalid values.  The operand descriptions already
+      // have the word "ID" in them.
+      if (!word) return diagnostic() << spvOperandTypeStr(type) << " is 0";
       break;
 
     case SPV_OPERAND_TYPE_EXTENSION_INSTRUCTION_NUMBER: {
@@ -571,11 +592,24 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
       convert_operand_endianness = false;
       const char* string =
           reinterpret_cast<const char*>(_.words + _.word_index);
-      size_t string_num_words = (strlen(string) / 4) + 1;  // Account for null.
+      // Compute the length of the string, but make sure we don't run off the
+      // end of the input.
+      const size_t remaining_input_bytes =
+          sizeof(uint32_t) * (_.num_words - _.word_index);
+      const size_t string_num_content_bytes =
+          strnlen(string, remaining_input_bytes);
+      // If there was no terminating null byte, then that's an end-of-input
+      // error.
+      if (string_num_content_bytes == remaining_input_bytes)
+        return exhaustedInputDiagnostic(inst_offset, inst->opcode, type);
+      // Account for null in the word length, so add 1 for null, then add 3 to
+      // make sure we round up.  The following is equivalent to:
+      //    (string_num_content_bytes + 1 + 3) / 4
+      const size_t string_num_words = string_num_content_bytes / 4 + 1;
       // Make sure we can record the word count without overflow.
-      // We still might have a string that's 64K words, but would still
-      // make the instruction too long because of earlier operands.
-      // That will be caught later at the end of the instruciton.
+      //
+      // This error can't currently be triggered because of validity
+      // checks elsewhere.
       if (string_num_words > std::numeric_limits<uint16_t>::max()) {
         return diagnostic() << "Literal string is longer than "
                             << std::numeric_limits<uint16_t>::max()
@@ -614,6 +648,7 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
     case SPV_OPERAND_TYPE_DIMENSIONALITY:
     case SPV_OPERAND_TYPE_SAMPLER_ADDRESSING_MODE:
     case SPV_OPERAND_TYPE_SAMPLER_FILTER_MODE:
+    case SPV_OPERAND_TYPE_SAMPLER_IMAGE_FORMAT:
     case SPV_OPERAND_TYPE_FP_ROUNDING_MODE:
     case SPV_OPERAND_TYPE_LINKAGE_TYPE:
     case SPV_OPERAND_TYPE_ACCESS_QUALIFIER:
@@ -687,6 +722,14 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
   operands->push_back(parsed_operand);
 
   const size_t index_after_operand = _.word_index + parsed_operand.num_words;
+
+  // Avoid buffer overrun for the cases where the operand has more than one
+  // word, and where it isn't a string.  (Those other cases have already been
+  // handled earlier.)  For example, this error can occur for a multi-word
+  // argument to OpConstant, or a multi-word case literal operand for OpSwitch.
+  if (_.num_words < index_after_operand)
+    return exhaustedInputDiagnostic(inst_offset, inst->opcode, type);
+
   if (_.requires_endian_conversion) {
     // Copy instruction words.  Translate to native endianness as needed.
     if (convert_operand_endianness) {
index 2e16177..1a7c939 100644 (file)
@@ -291,6 +291,7 @@ void Disassembler::EmitOperand(const spv_parsed_instruction_t& inst,
     case SPV_OPERAND_TYPE_DIMENSIONALITY:
     case SPV_OPERAND_TYPE_SAMPLER_ADDRESSING_MODE:
     case SPV_OPERAND_TYPE_SAMPLER_FILTER_MODE:
+    case SPV_OPERAND_TYPE_SAMPLER_IMAGE_FORMAT:
     case SPV_OPERAND_TYPE_FP_ROUNDING_MODE:
     case SPV_OPERAND_TYPE_LINKAGE_TYPE:
     case SPV_OPERAND_TYPE_ACCESS_QUALIFIER:
index 6f3736c..02c1b03 100644 (file)
@@ -176,10 +176,7 @@ static const spv_operand_desc_t storageClassEntries[] = {
      SpvStorageClassOutput,
      SPV_CAPABILITY_AS_MASK(SpvCapabilityShader),
      {SPV_OPERAND_TYPE_NONE}},
-    {"Workgroup",
-     SpvStorageClassWorkgroup,
-     0,
-     {SPV_OPERAND_TYPE_NONE}},
+    {"Workgroup", SpvStorageClassWorkgroup, 0, {SPV_OPERAND_TYPE_NONE}},
     {"CrossWorkgroup",
      SpvStorageClassCrossWorkgroup,
      0,
@@ -1206,7 +1203,7 @@ const char* spvOperandTypeStr(spv_operand_type_t type) {
     case SPV_OPERAND_TYPE_DIMENSIONALITY:
       return "dimensionality";
     case SPV_OPERAND_TYPE_SAMPLER_ADDRESSING_MODE:
-      return "addressing mode";
+      return "sampler addressing mode";
     case SPV_OPERAND_TYPE_SAMPLER_FILTER_MODE:
       return "sampler filter mode";
     case SPV_OPERAND_TYPE_SAMPLER_IMAGE_FORMAT:
@@ -1232,12 +1229,12 @@ const char* spvOperandTypeStr(spv_operand_type_t type) {
     case SPV_OPERAND_TYPE_FUNCTION_CONTROL:
       return "function control";
     case SPV_OPERAND_TYPE_MEMORY_SEMANTICS_ID:
-      return "memory semantics";
+      return "memory semantics ID";
     case SPV_OPERAND_TYPE_MEMORY_ACCESS:
     case SPV_OPERAND_TYPE_OPTIONAL_MEMORY_ACCESS:
       return "memory access";
     case SPV_OPERAND_TYPE_SCOPE_ID:
-      return "execution scope ID";
+      return "scope ID";
     case SPV_OPERAND_TYPE_GROUP_OPERATION:
       return "group operation";
     case SPV_OPERAND_TYPE_KERNEL_ENQ_FLAGS:
@@ -1248,7 +1245,7 @@ const char* spvOperandTypeStr(spv_operand_type_t type) {
       return "capability";
     case SPV_OPERAND_TYPE_IMAGE:
     case SPV_OPERAND_TYPE_OPTIONAL_IMAGE:
-      return "image operand";
+      return "image";
     case SPV_OPERAND_TYPE_OPTIONAL_CIV:
       return "context-insensitive value";
 
index f7d1fe2..c86ff6e 100755 (executable)
@@ -385,7 +385,7 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
       uint32_t value;
       if (grammar.parseMaskOperand(type, textValue, &value)) {
         return context->diagnostic() << "Invalid " << spvOperandTypeStr(type)
-                                     << " '" << textValue << "'.";
+                                     << " operand '" << textValue << "'.";
       }
       if (auto error = context->binaryEncodeU32(value, pInst)) return error;
       // Prepare to parse the operands for this logical operand.
@@ -645,16 +645,15 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
            << SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX;
   }
 
-  pInst->words[0] = spvOpcodeMake(uint16_t(pInst->words.size()), opcodeEntry->opcode);
+  pInst->words[0] =
+      spvOpcodeMake(uint16_t(pInst->words.size()), opcodeEntry->opcode);
 
   return SPV_SUCCESS;
 }
 
 namespace {
 
-enum {
-  kAssemblerVersion = 0
-};
+enum { kAssemblerVersion = 0 };
 
 /// @brief Populate a binary stream's words with this generator's header.
 ///
@@ -662,8 +661,7 @@ enum {
 /// @param[in] bound the upper ID bound
 ///
 /// @return result code
-spv_result_t
-SetHeader(uint32_t* words, const uint32_t bound) {
+spv_result_t SetHeader(uint32_t* words, const uint32_t bound) {
   if (!words) return SPV_ERROR_INVALID_BINARY;
 
   words[SPV_INDEX_MAGIC_NUMBER] = SpvMagicNumber;
index 2878af2..fc5756c 100644 (file)
@@ -185,11 +185,19 @@ class CaptureParseResults
   Instructions instructions_;
 };
 
+// The SPIR-V module header words for the Khronos Assembler generator,
+// for a module with an ID bound of 1.
+const uint32_t kHeaderForBound1[] = {
+    SpvMagicNumber, SpvVersion,
+    SPV_GENERATOR_WORD(SPV_GENERATOR_KHRONOS_ASSEMBLER, 0), 1 /*bound*/,
+    0 /*schema*/};
+
 // Returns the expected SPIR-V module header words for the Khronos
 // Assembler generator, and with a given Id bound.
 Words ExpectedHeaderForBound(uint32_t bound) {
-  return {SpvMagicNumber, SpvVersion,
-          SPV_GENERATOR_WORD(SPV_GENERATOR_KHRONOS_ASSEMBLER, 0), bound, 0};
+  Words result{std::begin(kHeaderForBound1), std::end(kHeaderForBound1)};
+  result[SPV_INDEX_BOUND] = bound;
+  return result;
 }
 
 // Returns a parsed operand for a non-number value at the given word offset
@@ -265,6 +273,42 @@ TEST_F(BinaryParseTest, EmptyModuleHasValidHeaderAndNoInstructionCallbacks) {
   EXPECT_THAT(instructions(), Eq(Instructions{}));
 }
 
+TEST_F(BinaryParseTest,
+       ModuleWithSingleInstructionHasValidHeaderAndInstructionCallback) {
+  const auto binary = CompileSuccessfully("%1 = OpTypeVoid");
+  spv_diagnostic diagnostic = nullptr;
+  EXPECT_EQ(SPV_SUCCESS,
+            spvBinaryParse(context, this, binary.data(), binary.size(), Header,
+                           Instruction, &diagnostic));
+  EXPECT_EQ(nullptr, diagnostic);
+  EXPECT_THAT(headers(), Eq(Sentences{ExpectedHeaderForBound(2)}));
+  EXPECT_THAT(instructions(),
+              Eq(Instructions{MakeParsedVoidTypeInstruction(1)}));
+}
+
+TEST_F(BinaryParseTest, NullHeaderCallbackIsIgnored) {
+  const auto binary = CompileSuccessfully("%1 = OpTypeVoid");
+  spv_diagnostic diagnostic = nullptr;
+  EXPECT_EQ(SPV_SUCCESS,
+            spvBinaryParse(context, this, binary.data(), binary.size(), nullptr,
+                           Instruction, &diagnostic));
+  EXPECT_EQ(nullptr, diagnostic);
+  EXPECT_THAT(headers(), Eq(Sentences{}));  // No header callback.
+  EXPECT_THAT(instructions(),
+              Eq(Instructions{MakeParsedVoidTypeInstruction(1)}));
+}
+
+TEST_F(BinaryParseTest, NullInstructionCallbackIsIgnored) {
+  const auto binary = CompileSuccessfully("%1 = OpTypeVoid");
+  spv_diagnostic diagnostic = nullptr;
+  EXPECT_EQ(SPV_SUCCESS,
+            spvBinaryParse(context, this, binary.data(), binary.size(), Header,
+                           nullptr, &diagnostic));
+  EXPECT_EQ(nullptr, diagnostic);
+  EXPECT_THAT(headers(), Eq(Sentences{ExpectedHeaderForBound(2)}));
+  EXPECT_THAT(instructions(), Eq(Instructions{}));  // No instruction callback.
+}
+
 // Check the result of multiple instruction callbacks.
 //
 // This test exercises non-default values for the following members of the
@@ -446,7 +490,269 @@ TEST_F(BinaryParseTest, ExtendedInstruction) {
   EXPECT_THAT(instructions(), ElementsAre(_, ParsedInstruction(parsed_inst)));
 }
 
-// TODO(dneto): Add tests for spvBinaryParse:
-//  - test each diagnostic in binary.cpp
+// A binary parser diagnostic test case where we provide the words array
+// pointer and word count explicitly.
+struct WordsAndCountDiagnosticCase {
+  const uint32_t* words;
+  size_t num_words;
+  std::string expected_diagnostic;
+};
+
+using BinaryParseWordsAndCountDiagnosticTest = spvtest::TextToBinaryTestBase<
+    ::testing::TestWithParam<WordsAndCountDiagnosticCase>>;
+
+TEST_P(BinaryParseWordsAndCountDiagnosticTest, WordAndCountCases) {
+  spv_diagnostic diagnostic = nullptr;
+  EXPECT_EQ(
+      SPV_ERROR_INVALID_BINARY,
+      spvBinaryParse(context, this, GetParam().words, GetParam().num_words,
+                     nullptr, nullptr, &diagnostic));
+  ASSERT_NE(nullptr, diagnostic);
+  EXPECT_THAT(diagnostic->error, Eq(GetParam().expected_diagnostic));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    BinaryParseDiagnostic, BinaryParseWordsAndCountDiagnosticTest,
+    ::testing::ValuesIn(std::vector<WordsAndCountDiagnosticCase>{
+        {nullptr, 0, "Missing module."},
+        {kHeaderForBound1, 0,
+         "Module has incomplete header: only 0 words instead of 5"},
+        {kHeaderForBound1, 1,
+         "Module has incomplete header: only 1 words instead of 5"},
+        {kHeaderForBound1, 2,
+         "Module has incomplete header: only 2 words instead of 5"},
+        {kHeaderForBound1, 3,
+         "Module has incomplete header: only 3 words instead of 5"},
+        {kHeaderForBound1, 4,
+         "Module has incomplete header: only 4 words instead of 5"},
+    }));
+
+// A binary parser diagnostic test case where a vector of words is
+// provided.  We'll use this to express cases that can't be created
+// via the assembler.  Either we want to make a malformed instruction,
+// or an invalid case the assembler would reject.
+struct WordVectorDiagnosticCase {
+  Words words;
+  std::string expected_diagnostic;
+};
+
+using BinaryParseWordVectorDiagnosticTest = spvtest::TextToBinaryTestBase<
+    ::testing::TestWithParam<WordVectorDiagnosticCase>>;
+
+TEST_P(BinaryParseWordVectorDiagnosticTest, WordVectorCases) {
+  spv_diagnostic diagnostic = nullptr;
+  const auto& words = GetParam().words;
+  EXPECT_EQ(SPV_ERROR_INVALID_BINARY,
+            spvBinaryParse(context, this, words.data(), words.size(), nullptr,
+                           nullptr, &diagnostic));
+  ASSERT_NE(nullptr, diagnostic);
+  EXPECT_THAT(diagnostic->error, Eq(GetParam().expected_diagnostic));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    BinaryParseDiagnostic, BinaryParseWordVectorDiagnosticTest,
+    ::testing::ValuesIn(std::vector<WordVectorDiagnosticCase>{
+        {Concatenate({ExpectedHeaderForBound(1), {spvOpcodeMake(0, SpvOpNop)}}),
+         "Invalid instruction word count: 0"},
+        {Concatenate({ExpectedHeaderForBound(1),
+                      {spvOpcodeMake(1, static_cast<SpvOp>(0xffff))}}),
+         "Invalid opcode: 65535"},
+        {Concatenate({ExpectedHeaderForBound(1),
+                      MakeInstruction(SpvOpNop, {42})}),
+         "Invalid instruction OpNop starting at word 5: expected "
+         "no more operands after 1 words, but stated word count is 2."},
+        {Concatenate({ExpectedHeaderForBound(1),
+                      MakeInstruction(SpvOpTypeVoid, {1, 2})}),
+         "Invalid instruction OpTypeVoid starting at word 5: expected "
+         "no more operands after 2 words, but stated word count is 3."},
+        {Concatenate({ExpectedHeaderForBound(1),
+                      MakeInstruction(SpvOpTypeVoid, {1, 2, 5, 9, 10})}),
+         "Invalid instruction OpTypeVoid starting at word 5: expected "
+         "no more operands after 2 words, but stated word count is 6."},
+        {Concatenate({ExpectedHeaderForBound(1),
+                      MakeInstruction(SpvOpTypeInt, {1, 32, 1, 9})}),
+         "Invalid instruction OpTypeInt starting at word 5: expected "
+         "no more operands after 4 words, but stated word count is 5."},
+        {Concatenate({ExpectedHeaderForBound(1),
+                      MakeInstruction(SpvOpTypeInt, {1})}),
+         "End of input reached while decoding OpTypeInt starting at word 5:"
+         " expected more operands after 2 words."},
+
+        // Check several cases for running off the end of input.
+
+        // Detect a missing single word operand.
+        {Concatenate({ExpectedHeaderForBound(1),
+                      {spvOpcodeMake(2, SpvOpTypeStruct)}}),
+         "End of input reached while decoding OpTypeStruct starting at word"
+         " 5: missing result ID operand at word offset 1."},
+        // Detect this a missing a multi-word operand to OpConstant.
+        // We also lie and say the OpConstant instruction has 5 words when
+        // it only has 3.  Corresponds to something like this:
+        //    %1 = OpTypeInt 64 0
+        //    %2 = OpConstant %1 <missing>
+        {Concatenate({ExpectedHeaderForBound(3),
+                      {MakeInstruction(SpvOpTypeInt, {1, 64, 0})},
+                      {spvOpcodeMake(5, SpvOpConstant), 1, 2}}),
+         "End of input reached while decoding OpConstant starting at word"
+         " 9: missing possibly multi-word literal number operand at word "
+         "offset 3."},
+        // Detect when we provide only one word from the 64-bit literal,
+        // and again lie about the number of words in the instruction.
+        {Concatenate({ExpectedHeaderForBound(3),
+                      {MakeInstruction(SpvOpTypeInt, {1, 64, 0})},
+                      {spvOpcodeMake(5, SpvOpConstant), 1, 2, 42}}),
+         "End of input reached while decoding OpConstant starting at word"
+         " 9: truncated possibly multi-word literal number operand at word "
+         "offset 3."},
+        // Detect when a required string operand is missing.
+        // Also, lie about the length of the instruction.
+        {Concatenate({ExpectedHeaderForBound(3),
+                      {spvOpcodeMake(3, SpvOpString), 1}}),
+         "End of input reached while decoding OpString starting at word"
+         " 5: missing literal string operand at word offset 2."},
+        // Detect when a required string operand is truncated: it's missing
+        // a null terminator.  Catching the error avoids a buffer overrun.
+        {Concatenate({ExpectedHeaderForBound(3),
+                      {spvOpcodeMake(4, SpvOpString), 1, 0x41414141,
+                       0x41414141}}),
+         "End of input reached while decoding OpString starting at word"
+         " 5: truncated literal string operand at word offset 2."},
+        // Detect when an optional string operand is truncated: it's missing
+        // a null terminator.  Catching the error avoids a buffer overrun.
+        // (It is valid for an optional string operand to be absent.)
+        {Concatenate({ExpectedHeaderForBound(3),
+                      {spvOpcodeMake(6, SpvOpSource),
+                       uint32_t(SpvSourceLanguageOpenCL_C), 210,
+                       1 /* file id */,
+                       /*start of string*/ 0x41414141, 0x41414141}}),
+         "End of input reached while decoding OpSource starting at word"
+         " 5: truncated literal string operand at word offset 4."},
+
+        // (End of input exhaustion test cases.)
+
+        // In this case the instruction word count is too small, where
+        // it would truncate a multi-word operand to OpConstant.
+        {Concatenate({ExpectedHeaderForBound(3),
+                      {MakeInstruction(SpvOpTypeInt, {1, 64, 0})},
+                      {spvOpcodeMake(4, SpvOpConstant), 1, 2, 44, 44}}),
+         "Invalid word count: OpConstant starting at word 9 says it has 4"
+         " words, but found 5 words instead."},
+        // Word count is to small, where it would truncate a literal string.
+        {Concatenate({ExpectedHeaderForBound(2),
+                      {spvOpcodeMake(3, SpvOpString), 1, 0x41414141, 0}}),
+         "Invalid word count: OpString starting at word 5 says it has 3"
+         " words, but found 4 words instead."},
+        {Concatenate({ExpectedHeaderForBound(2),
+                      {spvOpcodeMake(2, SpvOpTypeVoid), 0}}),
+         "Error: Result Id is 0"},
+        {Concatenate({
+             ExpectedHeaderForBound(2),
+             {spvOpcodeMake(2, SpvOpTypeVoid), 1},
+             {spvOpcodeMake(2, SpvOpTypeBool), 1},
+         }),
+         "Id 1 is defined more than once"},
+        {Concatenate({ExpectedHeaderForBound(3),
+                      MakeInstruction(SpvOpExtInst, {2, 3, 100, 4, 5})}),
+         "OpExtInst set Id 100 does not reference an OpExtInstImport result "
+         "Id"},
+        {Concatenate({ExpectedHeaderForBound(3),
+                      MakeInstruction(SpvOpSwitch, {1, 2, 42, 3})}),
+         "Invalid OpSwitch: selector id 1 has no type"},
+        {Concatenate({ExpectedHeaderForBound(3),
+                      MakeInstruction(SpvOpTypeInt, {1, 32, 0}),
+                      MakeInstruction(SpvOpSwitch, {1, 3, 42, 3})}),
+         "Invalid OpSwitch: selector id 1 is a type, not a value"},
+        {Concatenate({ExpectedHeaderForBound(3),
+                      MakeInstruction(SpvOpTypeFloat, {1, 32}),
+                      MakeInstruction(SpvOpConstant, {1, 2, 0x78f00000}),
+                      MakeInstruction(SpvOpSwitch, {2, 3, 42, 3})}),
+         "Invalid OpSwitch: selector id 2 is not a scalar integer"},
+        {Concatenate({ExpectedHeaderForBound(3),
+                      MakeInstruction(SpvOpExtInstImport, {1},
+                                      MakeVector("invalid-import"))}),
+         "Invalid extended instruction import 'invalid-import'"},
+        {Concatenate({
+             ExpectedHeaderForBound(3),
+             MakeInstruction(SpvOpTypeInt, {1, 32, 0}),
+             MakeInstruction(SpvOpConstant, {2, 2, 42}),
+         }),
+         "Type Id 2 is not a type"},
+        {Concatenate({
+             ExpectedHeaderForBound(3), MakeInstruction(SpvOpTypeBool, {1}),
+             MakeInstruction(SpvOpConstant, {1, 2, 42}),
+         }),
+         "Type Id 1 is not a scalar numeric type"},
+    }));
+
+// A binary parser diagnostic case generated from an assembly text input.
+struct AssemblyDiagnosticCase {
+  std::string assembly;
+  std::string expected_diagnostic;
+};
+
+using BinaryParseAssemblyDiagnosticTest = spvtest::TextToBinaryTestBase<
+    ::testing::TestWithParam<AssemblyDiagnosticCase>>;
+
+TEST_P(BinaryParseAssemblyDiagnosticTest, AssemblyCases) {
+  spv_diagnostic diagnostic = nullptr;
+  auto words = CompileSuccessfully(GetParam().assembly);
+  EXPECT_EQ(SPV_ERROR_INVALID_BINARY,
+            spvBinaryParse(context, this, words.data(), words.size(), nullptr,
+                           nullptr, &diagnostic));
+  ASSERT_NE(nullptr, diagnostic);
+  EXPECT_THAT(diagnostic->error, Eq(GetParam().expected_diagnostic));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    BinaryParseDiagnostic, BinaryParseAssemblyDiagnosticTest,
+    ::testing::ValuesIn(std::vector<AssemblyDiagnosticCase>{
+        {"%1 = OpConstant !0 42", "Error: Type Id is 0"},
+        // A required id is 0.
+        {"OpName !0 \"foo\"", "Id is 0"},
+        // An optional id is 0, in this case the optional
+        // initializer.
+        {"%2 = OpVariable %1 CrossWorkgroup !0", "Id is 0"},
+        {"OpControlBarrier !0 %1 %2", "scope ID is 0"},
+        {"OpControlBarrier %1 !0 %2", "scope ID is 0"},
+        {"OpControlBarrier %1 %2 !0", "memory semantics ID is 0"},
+        {"%import = OpExtInstImport \"GLSL.std.450\" "
+         "%result = OpExtInst %type %import !999999 %x",
+         "Invalid extended instruction number: 999999"},
+        {"%2 = OpSpecConstantOp %1 !1000 %2",
+         "Invalid OpSpecConstantOp opcode: 1000"},
+        {"OpCapability !9999", "Invalid capability operand: 9999"},
+        {"OpSource !9999 100", "Invalid source language operand: 9999"},
+        {"OpEntryPoint !9999", "Invalid execution model operand: 9999"},
+        {"OpMemoryModel !9999", "Invalid addressing model operand: 9999"},
+        {"OpMemoryModel Logical !9999", "Invalid memory model operand: 9999"},
+        {"OpExecutionMode %1 !9999", "Invalid execution mode operand: 9999"},
+        {"OpTypeForwardPointer %1 !9999",
+         "Invalid storage class operand: 9999"},
+        {"%2 = OpTypeImage %1 !9999", "Invalid dimensionality operand: 9999"},
+        {"%2 = OpTypeImage %1 1D 0 0 0 0 !9999",
+         "Invalid image format operand: 9999"},
+        {"OpDecorate %1 FPRoundingMode !9999",
+         "Invalid floating-point rounding mode operand: 9999"},
+        {"OpDecorate %1 LinkageAttributes \"C\" !9999",
+         "Invalid linkage type operand: 9999"},
+        {"%1 = OpTypePipe !9999", "Invalid access qualifier operand: 9999"},
+        {"OpDecorate %1 FuncParamAttr !9999",
+         "Invalid function parameter attribute operand: 9999"},
+        {"OpDecorate %1 !9999", "Invalid decoration operand: 9999"},
+        {"OpDecorate %1 BuiltIn !9999", "Invalid built-in operand: 9999"},
+        {"%2 = OpGroupIAdd %1 %3 !9999",
+         "Invalid group operation operand: 9999"},
+        {"OpDecorate %1 FPFastMathMode !63",
+         "Invalid floating-point fast math mode operand: 63 has invalid mask "
+         "component 32"},
+        {"%2 = OpFunction %2 !31",
+         "Invalid function control operand: 31 has invalid mask component 16"},
+        {"OpLoopMerge %1 %2 !7",
+         "Invalid loop control operand: 7 has invalid mask component 4"},
+        {"%2 = OpImageFetch %1 %image %coord !511",
+         "Invalid image operand: 511 has invalid mask component 256"},
+        {"OpSelectionMerge %1 !7",
+         "Invalid selection control operand: 7 has invalid mask component 4"},
+    }));
 
 }  // anonymous namespace
index 3a937ea..717d41b 100644 (file)
@@ -268,7 +268,7 @@ TEST_F(OpDecorateEnumTest, CombinedFPFastMathMask) {
 
 TEST_F(OpDecorateEnumTest, WrongFPFastMathMode) {
   EXPECT_THAT(CompileFailure("OpDecorate %1 FPFastMathMode NotNaN|xxyyzz"),
-              Eq("Invalid floating-point fast math mode 'NotNaN|xxyyzz'."));
+              Eq("Invalid floating-point fast math mode operand 'NotNaN|xxyyzz'."));
 }
 
 // Test OpDecorate Linkage
index c98c7ef..9e4f047 100644 (file)
@@ -71,7 +71,7 @@ INSTANTIATE_TEST_CASE_P(
 
 TEST_F(SamplerAddressingModeTest, WrongMode) {
   EXPECT_THAT(CompileFailure("%r = OpConstantSampler %t xxyyzz 0 Nearest"),
-              Eq("Invalid addressing mode 'xxyyzz'."));
+              Eq("Invalid sampler addressing mode 'xxyyzz'."));
 }
 
 // Test Sampler Filter Mode enum values
index 9b8789a..355a396 100644 (file)
@@ -74,7 +74,7 @@ TEST_F(OpSelectionMergeTest, CombinedSelectionControlMask) {
 TEST_F(OpSelectionMergeTest, WrongSelectionControl) {
   // Case sensitive: "flatten" != "Flatten" and thus wrong.
   EXPECT_THAT(CompileFailure("OpSelectionMerge %1 flatten|DontFlatten"),
-              Eq("Invalid selection control 'flatten|DontFlatten'."));
+              Eq("Invalid selection control operand 'flatten|DontFlatten'."));
 }
 
 // Test OpLoopMerge
@@ -109,7 +109,7 @@ TEST_F(OpLoopMergeTest, CombinedLoopControlMask) {
 
 TEST_F(OpLoopMergeTest, WrongLoopControl) {
   EXPECT_THAT(CompileFailure("OpLoopMerge %m %c none"),
-              Eq("Invalid loop control 'none'."));
+              Eq("Invalid loop control operand 'none'."));
 }
 
 // Test OpSwitch
index a11006d..a343cd2 100644 (file)
@@ -79,7 +79,7 @@ TEST_F(OpFunctionControlTest, CombinedFunctionControlMask) {
 
 TEST_F(OpFunctionControlTest, WrongFunctionControl) {
   EXPECT_THAT(CompileFailure("%r = OpFunction %t Inline|Unroll %ft"),
-              Eq("Invalid function control 'Inline|Unroll'."));
+              Eq("Invalid function control operand 'Inline|Unroll'."));
 }
 
 // TODO(dneto): OpFunctionParameter
index db17d7e..c6c2073 100644 (file)
@@ -29,8 +29,8 @@
 
 #include "UnitSPIRV.h"
 
-#include "gmock/gmock.h"
 #include "TestFixture.h"
+#include "gmock/gmock.h"
 
 namespace {
 
@@ -44,12 +44,15 @@ using DimTest =
     spvtest::TextToBinaryTestBase<::testing::TestWithParam<EnumCase<SpvDim>>>;
 
 TEST_P(DimTest, AnyDim) {
-  const std::string input = "%imageType = OpTypeImage %sampledType " +
-                            GetParam().name() + " 2 3 0 4 Rgba8";
+  const std::string input =
+      "%1 = OpTypeImage %2 " + GetParam().name() + " 2 3 0 4 Rgba8\n";
   EXPECT_THAT(
       CompiledInstructions(input),
       Eq(MakeInstruction(SpvOpTypeImage, {1, 2, GetParam().value(), 2, 3, 0, 4,
                                           SpvImageFormatRgba8})));
+
+  // Check the disassembler as well.
+  EXPECT_THAT(EncodeAndDecodeSuccessfully(input), Eq(input));
 }
 
 // clang-format off
@@ -80,10 +83,12 @@ using ImageFormatTest = spvtest::TextToBinaryTestBase<
 
 TEST_P(ImageFormatTest, AnyImageFormat) {
   const std::string input =
-      "%imageType = OpTypeImage %sampledType 1D  2 3 0 4 " + GetParam().name();
+      "%1 = OpTypeImage %2 1D 2 3 0 4 " + GetParam().name() + "\n";
   EXPECT_THAT(CompiledInstructions(input),
               Eq(MakeInstruction(SpvOpTypeImage, {1, 2, SpvDim1D, 2, 3, 0, 4,
                                                   GetParam().value()})));
+  // Check the disassembler as well.
+  EXPECT_THAT(EncodeAndDecodeSuccessfully(input), Eq(input));
 }
 
 // clang-format off