Added type tracking to the disassembler.

TODO: Actually use the tracked types to make sure that we print out
values correctly.

diff --git a/source/binary.cpp b/source/binary.cpp
index e7fc61d..9df80d3 100644 (file)
--- a/source/binary.cpp
+++ b/source/binary.cpp
@@ -37,6 +37,7 @@
 #include <string.h>
 
 #include <sstream>
+#include <unordered_map>
 
 // Binary API
 
@@ -50,6 +51,9 @@ static const union {
   uint32_t value;
 } o32_host_order = {{0, 1, 2, 3}};
 
+using id_to_type_id_map = std::unordered_map<uint32_t, uint32_t>;
+using type_id_to_type_map = std::unordered_map<uint32_t, libspirv::IdType>;
+
 #define I32_ENDIAN_HOST (o32_host_order.value)
 
 spv_result_t spvBinaryEndianness(const spv_binary binary,
@@ -363,6 +367,62 @@ spv_result_t spvBinaryDecodeOperand(
   return SPV_SUCCESS;
 }
 
+
+
+/// @brief Regsiters the given instruction with the type and id tracking
+///   tables.
+///
+/// @param[in] pInst the Opcode instruction stream
+/// @param[in] pOpcodeEntry the Opcode Entry describing the instruction
+/// @param[in, out] type_map the map of Ids to Types to be filled in
+/// @param[in, out] id_map the map of Ids to type Ids to be filled in
+/// @param[in, out] position position in the stream
+/// @param[out] pDiag return diagnostic on error
+///
+/// @return result code
+spv_result_t spvRegisterIdForOpcode(const spv_instruction_t* pInst,
+                                    const spv_opcode_desc_t* pOpcodeEntry,
+                                    type_id_to_type_map* type_map,
+                                    id_to_type_id_map* id_map,
+                                    spv_position position,
+                                    spv_diagnostic* pDiagnostic) {
+  libspirv::IdType detected_type = libspirv::kUnknownType;
+  if (spvOpcodeIsType(pOpcodeEntry->opcode)) {
+    if (spv::OpTypeInt == pOpcodeEntry->opcode) {
+      detected_type.type_class = libspirv::IdTypeClass::kScalarIntegerType;
+      detected_type.bitwidth = pInst->words[2];
+      detected_type.isSigned = (pInst->words[3] != 0);
+    } else if (spv::OpTypeFloat == pOpcodeEntry->opcode) {
+      detected_type.type_class = libspirv::IdTypeClass::kScalarIntegerType;
+      detected_type.bitwidth = pInst->words[2];
+      detected_type.isSigned = true;
+    } else {
+      detected_type.type_class = libspirv::IdTypeClass::kOtherType;
+    }
+  }
+
+  // We do not use else-if here so that we can still catch the case where an
+  // OpType* instruction shares the same ID as a non OpType* instruction.
+  if (pOpcodeEntry->hasResult) {
+    uint32_t value_id =
+        pOpcodeEntry->hasType ? pInst->words[2] : pInst->words[1];
+    if (id_map->find(value_id) != id_map->end()) {
+      DIAGNOSTIC << "Id " << value_id << " is defined more than once";
+      return SPV_ERROR_INVALID_BINARY;
+    }
+
+    (*id_map)[value_id] = pOpcodeEntry->hasType ? pInst->words[1] : 0;
+  }
+
+  if (detected_type != libspirv::kUnknownType) {
+    // This defines a new type.
+    uint32_t id = pInst->words[1];
+    (*type_map)[id] = detected_type;
+  }
+
+  return SPV_SUCCESS;
+}
+
 /// @brief Translate binary Opcode stream to textual form
 ///
 /// @param[in] pInst the Opcode instruction stream
@@ -379,6 +439,8 @@ spv_result_t spvBinaryDecodeOpcode(spv_instruction_t* pInst,
                                    const spv_endianness_t endian,
                                    const uint32_t options,
                                    const libspirv::AssemblyGrammar& grammar,
+                                   type_id_to_type_map* type_map,
+                                   id_to_type_id_map* id_map,
                                    spv_assembly_syntax_format_t format,
                                    out_stream &stream, spv_position position,
                                    spv_diagnostic *pDiagnostic) {
@@ -499,7 +561,10 @@ spv_result_t spvBinaryDecodeOpcode(spv_instruction_t* pInst,
   }
 
   stream.get() << no_result_id_strstream.str();
-
+  if (spv_result_t error = spvRegisterIdForOpcode(
+          pInst, opcodeEntry, type_map, id_map, position, pDiagnostic)) {
+    return error;
+  }
   return SPV_SUCCESS;
 }
 
@@ -574,6 +639,10 @@ spv_result_t spvBinaryToTextWithFormat(
   const uint32_t *words = binary.code;
   position.index = SPV_INDEX_INSTRUCTION;
   spv_ext_inst_type_t extInstType = SPV_EXT_INST_TYPE_NONE;
+
+  id_to_type_id_map id_map;
+  type_id_to_type_map type_map;
+
   while (position.index < binary.wordCount) {
     uint64_t index = position.index;
     uint16_t wordCount;
@@ -586,8 +655,8 @@ spv_result_t spvBinaryToTextWithFormat(
     spvInstructionCopy(&words[position.index], opcode, wordCount, endian,
                        &inst);
 
-    if (spvBinaryDecodeOpcode(&inst, endian, options, grammar, format, stream,
-                              &position, pDiagnostic))
+    if (spvBinaryDecodeOpcode(&inst, endian, options, grammar, &type_map,
+                              &id_map, format, stream, &position, pDiagnostic))
       return SPV_ERROR_INVALID_BINARY;
     extInstType = inst.extInstType;
 

diff --git a/source/opcode.cpp b/source/opcode.cpp
index 8946cde..f1a2200 100644 (file)
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -587,6 +587,7 @@ int32_t spvOpcodeIsType(const Op opcode) {
     case OpTypeVector:
     case OpTypeMatrix:
     case OpTypeSampler:
+    case OpTypeSampledImage:
     case OpTypeArray:
     case OpTypeRuntimeArray:
     case OpTypeStruct:

diff --git a/source/text_handler.h b/source/text_handler.h
index ce391a3..adbde16 100644 (file)
--- a/source/text_handler.h
+++ b/source/text_handler.h
@@ -60,6 +60,18 @@ struct IdType {
   IdTypeClass type_class;
 };
 
+// Default equality operator for IdType. Tests if all members are the same.
+inline bool operator==(const IdType &first, const IdType &second) {
+  return (first.bitwidth == second.bitwidth) &&
+         (first.isSigned == second.isSigned) &&
+         (first.type_class == second.type_class);
+}
+
+// Tests whether any member of the IdTypes do not match.
+inline bool operator!=(const IdType &first, const IdType &second) {
+  return !(first == second);
+}
+
 // A value representing an unknown type.
 extern const IdType kUnknownType;
 

diff --git a/test/BinaryToText.cpp b/test/BinaryToText.cpp
index 8ca9faf..c621019 100644 (file)
--- a/test/BinaryToText.cpp
+++ b/test/BinaryToText.cpp
@@ -138,6 +138,37 @@ TEST_F(BinaryToText, InvalidDiagnostic) {
                             operandTable, extInstTable, &text, nullptr));
 }
 
+struct FailedDecodeCase {
+  std::string source_text;
+  std::vector<uint32_t> appended_instruction;
+  std::string expected_error_message;
+};
+
+using BinaryToTextFail =
+    spvtest::TextToBinaryTestBase <
+    ::testing::TestWithParam<FailedDecodeCase>>;
+
+TEST_P(BinaryToTextFail, EncodeSuccessfullyDecodeFailed) {
+  EXPECT_THAT(EncodeSuccessfullyDecodeFailed(GetParam().source_text,
+                                             GetParam().appended_instruction),
+              Eq(GetParam().expected_error_message));
+}
+
+INSTANTIATE_TEST_CASE_P(InvalidIds, BinaryToTextFail,
+                        ::testing::ValuesIn(std::vector<FailedDecodeCase>{
+                            {"%1 = OpTypeVoid",
+                             spvtest::MakeInstruction(spv::OpTypeVoid, {1}),
+                             "Id 1 is defined more than once"},
+                            {"%1 = OpTypeVoid\n"
+                             "%2 = OpNot %1 %foo",
+                             spvtest::MakeInstruction(spv::OpNot, {1, 2, 3}),
+                             "Id 2 is defined more than once"},
+                            {"%1 = OpTypeVoid\n"
+                             "%2 = OpNot %1 %foo",
+                             spvtest::MakeInstruction(spv::OpNot, {1, 1, 3}),
+                             "Id 1 is defined more than once"},
+                        }));
+
 TEST(BinaryToTextSmall, OneInstruction) {
   // TODO(dneto): This test could/should be refactored.
   spv_opcode_table opcodeTable;

diff --git a/test/TestFixture.h b/test/TestFixture.h
index 8711e6e..ffb0c1b 100644 (file)
--- a/test/TestFixture.h
+++ b/test/TestFixture.h
@@ -131,6 +131,32 @@ class TextToBinaryTestBase : public T {
     return decoded_string.substr(preamble_end + schema0.size());
   }
 
+  // Encodes SPIR-V text into binary. This is expected to succeed.
+  // The given words are then appended to the binary, and the result
+  // is then decoded. This is expected to fail.
+  // Returns the error message.
+  std::string EncodeSuccessfullyDecodeFailed(
+      const std::string& text,
+      const SpirvVector& words_to_append) {
+    SpirvVector code = spvtest::Concatenate(
+        {CompileSuccessfully(text, SPV_ASSEMBLY_SYNTAX_FORMAT_DEFAULT),
+         words_to_append});
+
+    spv_text decoded_text;
+    EXPECT_NE(SPV_SUCCESS,
+              spvBinaryToText(code.data(), code.size(),
+                              SPV_BINARY_TO_TEXT_OPTION_NONE, opcodeTable,
+                              operandTable, extInstTable, &decoded_text,
+                              &diagnostic));
+    if (diagnostic) {
+      std::string error_message = diagnostic->error;
+      spvDiagnosticDestroy(diagnostic);
+      diagnostic = nullptr;
+      return error_message;
+    }
+    return "";
+  }
+
   // Compiles SPIR-V text, asserts success, and returns the words representing
   // the instructions.  In particular, skip the words in the SPIR-V header.
   SpirvVector CompiledInstructions(const std::string& text,