Support strings with up to 65535 chars, and null.

Move the definition of spv_instruction_t to an internal
header file, since it now depends on C++ and is not
used by the external interface.

Use a std::vector<uint32_t> in spv_instruction_t
instead of a fixed size array.

CMakeLists.txt

@@ -96,9 +96,11 @@ include_directories(
 
 set(SPIRV_SOURCES
   ${CMAKE_CURRENT_SOURCE_DIR}/include/libspirv/libspirv.h
-  ${CMAKE_CURRENT_SOURCE_DIR}/source/bitwisecast.h
   ${CMAKE_CURRENT_SOURCE_DIR}/source/binary.h
+  ${CMAKE_CURRENT_SOURCE_DIR}/source/bitwisecast.h
   ${CMAKE_CURRENT_SOURCE_DIR}/source/diagnostic.h
+  ${CMAKE_CURRENT_SOURCE_DIR}/source/ext_inst.h
+  ${CMAKE_CURRENT_SOURCE_DIR}/source/instruction.h
   ${CMAKE_CURRENT_SOURCE_DIR}/source/opcode.h
   ${CMAKE_CURRENT_SOURCE_DIR}/source/operand.h
   ${CMAKE_CURRENT_SOURCE_DIR}/source/print.h
@@ -164,6 +166,7 @@ if (NOT ${SPIRV_SKIP_EXECUTABLES})
       ${CMAKE_CURRENT_SOURCE_DIR}/test/TestFixture.h
       ${CMAKE_CURRENT_SOURCE_DIR}/test/UnitSPIRV.h
 
+      ${CMAKE_CURRENT_SOURCE_DIR}/test/AssemblyContext.cpp
       ${CMAKE_CURRENT_SOURCE_DIR}/test/AssemblyFormat.cpp
       ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryDestroy.cpp
       ${CMAKE_CURRENT_SOURCE_DIR}/test/BinaryEndianness.cpp

include/libspirv/libspirv.h

@@ -56,10 +56,15 @@ extern "C" {
 
 // Universal limits
 
+// SPIR-V 1.0 limits
+#define SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX 0xffff
+
 // NOTE: These are set to the minimum maximum values
+// TODO(dneto): Check these.
+
+// libspirv limits.
 #define SPV_LIMIT_LITERAL_NAME_MAX 0x00000400
 #define SPV_LIMIT_LITERAL_STRING_MAX 0x00010000
-#define SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX 0x00000108
 #define SPV_LIMIT_RESULT_ID_BOUND 0x00400000
 #define SPV_LIMIT_CONTROL_FLOW_NEST_DEPTH 0x00000400
 #define SPV_LIMIT_GLOBAL_VARIABLES_MAX 0x00010000
@@ -350,22 +355,6 @@ typedef struct spv_text_t {
   uint64_t length;
 } spv_text_t;
 
-// Describes an instruction.
-//
-// The wordCount and words[0..wordCount-1] always contain valid data.
-//
-// Normally, both opcode and extInstType contain valid data.
-// However, when the assembler parses !<number> as the first word in
-// an instruction, then opcode and extInstType are invalid, and
-//   wordCount == 1
-//   words[0] == <number>
-typedef struct spv_instruction_t {
-  uint16_t wordCount;
-  Op opcode;
-  spv_ext_inst_type_t extInstType;
-  uint32_t words[SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX];
-} spv_instruction_t;
-
 typedef struct spv_position_t {
   uint64_t line;
   uint64_t column;

source/binary.cpp

@@ -28,6 +28,7 @@
 #include "binary.h"
 #include "diagnostic.h"
 #include "ext_inst.h"
+#include "instruction.h"
 #include "opcode.h"
 #include "operand.h"
 
@@ -444,7 +445,7 @@ spv_result_t spvBinaryDecodeOpcode(
     }
 
     if (spvBinaryDecodeOperand(
-            opcodeEntry->opcode, type, pInst->words + index, numWords, endian,
+            opcodeEntry->opcode, type, &pInst->words[index], numWords, endian,
             options, operandTable, extInstTable, &expectedOperands,
             &pInst->extInstType,
             (isAssigmentFormat && !currentIsResultId ? no_result_id_stream

source/binary.h

@@ -28,6 +28,7 @@
 #define _LIBSPIRV_UTIL_BINARY_H_
 
 #include <libspirv/libspirv.h>
+#include "instruction.h"
 #include "operand.h"
 #include "print.h"
 

source/instruction.h

@@ -0,0 +1,57 @@
+// Copyright (c) 2015 The Khronos Group Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and/or associated documentation files (the
+// "Materials"), to deal in the Materials without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Materials, and to
+// permit persons to whom the Materials are furnished to do so, subject to
+// the following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Materials.
+//
+// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS
+// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS
+// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT
+//    https://www.khronos.org/registry/
+//
+// THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+// IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS.
+
+#ifndef _LIBSPIRV_UTIL_INSTRUCTION_H_
+#define _LIBSPIRV_UTIL_INSTRUCTION_H_
+
+#include <cstdint>
+#include <vector>
+
+#include <headers/spirv.hpp>
+
+// Describes an instruction.
+struct spv_instruction_t {
+  // Normally, both opcode and extInstType contain valid data.
+  // However, when the assembler parses !<number> as the first word in
+  // an instruction and opcode and extInstType are invalid.
+  Op opcode;
+  spv_ext_inst_type_t extInstType;
+
+  // The instruction, as a sequence of 32-bit words.
+  // For a regular instruction the opcode and word count are combined
+  // in words[0], as described in the SPIR-V spec.
+  // Otherwise, the first token was !<number>, and that number appears
+  // in words[0].  Subsequent elements are the result of parsing
+  // tokens in the alternate parsing mode as described in syntax.md.
+  std::vector<uint32_t> words;
+};
+
+// Appends a word to an instruction, without checking for overflow.
+inline void spvInstructionAddWord(spv_instruction_t* inst, uint32_t value) {
+  inst->words.push_back(value);
+}
+
+#endif  // _LIBSPIRV_UTIL_INSTRUCTION_H_

source/opcode.cpp

@@ -26,6 +26,7 @@
 
 #include <libspirv/libspirv.h>
 #include "binary.h"
+#include "instruction.h"
 #include "opcode.h"
 
 #include <assert.h>
@@ -312,7 +313,7 @@ void spvInstructionCopy(const uint32_t *words, const Op opcode,
                         const uint16_t wordCount, const spv_endianness_t endian,
                         spv_instruction_t *pInst) {
   pInst->opcode = opcode;
-  pInst->wordCount = wordCount;
+  pInst->words.resize(wordCount);
   for (uint16_t wordIndex = 0; wordIndex < wordCount; ++wordIndex) {
     pInst->words[wordIndex] = spvFixWord(words[wordIndex], endian);
     if (!wordIndex) {

source/opcode.h

@@ -27,6 +27,7 @@
 #ifndef _LIBSPIRV_UTIL_OPCODE_H_
 #define _LIBSPIRV_UTIL_OPCODE_H_
 
+#include "instruction.h"
 #include <libspirv/libspirv.h>
 
 // Functions

source/text.cpp

@@ -42,6 +42,7 @@
 #include "bitwisecast.h"
 #include "diagnostic.h"
 #include "ext_inst.h"
+#include "instruction.h"
 #include <libspirv/libspirv.h>
 #include "opcode.h"
 #include "operand.h"
@@ -223,7 +224,7 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
         return SPV_ERROR_INVALID_TEXT;
       }
       const uint32_t id = context->spvNamedIdAssignOrGet(textValue);
-      pInst->words[pInst->wordCount++] = id;
+      spvInstructionAddWord(pInst, id);
     } break;
     case SPV_OPERAND_TYPE_LITERAL_NUMBER: {
       // NOTE: Special case for extension instruction lookup
@@ -234,7 +235,7 @@ spv_result_t spvTextEncodeOperand(const libspirv::AssemblyGrammar& grammar,
                                 << textValue << "'.";
           return SPV_ERROR_INVALID_TEXT;
         }
-        pInst->words[pInst->wordCount++] = extInst->ext_inst;
+        spvInstructionAddWord(pInst, extInst->ext_inst);
 
         // Prepare to parse the operands for the extended instructions.
         spvPrependOperandTypes(extInst->operandTypes, pExpectedOperands);
@@ -546,7 +547,8 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
   }
   pInst->opcode = opcodeEntry->opcode;
   context->setPosition(nextPosition);
-  pInst->wordCount++;
+  // Reserve the first word for the instruction.
+  spvInstructionAddWord(pInst, 0);
 
   // Maintains the ordered list of expected operand types.
   // For many instructions we only need the {numTypes, operandTypes}
@@ -622,7 +624,14 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar,
     }
   }
 
-  pInst->words[0] = spvOpcodeMake(pInst->wordCount, opcodeEntry->opcode);
+  if (pInst->words.size() > SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX) {
+    context->diagnostic() << "Instruction too long: " << pInst->words.size()
+                          << " words, but the limit is "
+                          << SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX;
+    return SPV_ERROR_INVALID_TEXT;
+  }
+
+  pInst->words[0] = spvOpcodeMake(pInst->words.size(), opcodeEntry->opcode);
 
   return SPV_SUCCESS;
 }
@@ -660,7 +669,8 @@ spv_result_t spvTextToBinaryInternal(const libspirv::AssemblyGrammar& grammar,
 
   spv_ext_inst_type_t extInstType = SPV_EXT_INST_TYPE_NONE;
   while (context.hasText()) {
-    spv_instruction_t inst = {};
+    instructions.push_back({});
+    spv_instruction_t& inst = instructions.back();
     inst.extInstType = extInstType;
 
     if (spvTextEncodeOpcode(grammar, &context, format, &inst)) {
@@ -668,22 +678,20 @@ spv_result_t spvTextToBinaryInternal(const libspirv::AssemblyGrammar& grammar,
     }
     extInstType = inst.extInstType;
 
-    instructions.push_back(inst);
-
     if (context.advance()) break;
   }
 
   size_t totalSize = SPV_INDEX_INSTRUCTION;
   for (auto& inst : instructions) {
-    totalSize += inst.wordCount;
+    totalSize += inst.words.size();
   }
 
   uint32_t* data = new uint32_t[totalSize];
   if (!data) return SPV_ERROR_OUT_OF_MEMORY;
   uint64_t currentIndex = SPV_INDEX_INSTRUCTION;
   for (auto& inst : instructions) {
-    memcpy(data + currentIndex, inst.words, sizeof(uint32_t) * inst.wordCount);
+    memcpy(data + currentIndex, inst.words.data(), sizeof(uint32_t) * inst.words.size());
-    currentIndex += inst.wordCount;
+    currentIndex += inst.words.size();
   }
 
   spv_binary binary = new spv_binary_t();

source/text.h

@@ -54,7 +54,8 @@ typedef struct spv_literal_t {
     uint64_t u64;
     float f;
     double d;
-    char str[SPV_LIMIT_LITERAL_STRING_MAX];
+    // Allow room for the null terminator, and two surrounding quotes.
+    char str[SPV_LIMIT_LITERAL_STRING_MAX + 3];
   } value;
 } spv_literal_t;
 

source/text_handler.cpp

@@ -32,6 +32,7 @@
 
 #include "binary.h"
 #include "ext_inst.h"
+#include "instruction.h"
 #include "opcode.h"
 #include "text.h"
 
@@ -317,13 +318,7 @@ void AssemblyContext::seekForward(uint32_t size) {
 
 spv_result_t AssemblyContext::binaryEncodeU32(const uint32_t value,
                                                      spv_instruction_t *pInst) {
-  if (pInst->wordCount + 1 > SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX) {
+  spvInstructionAddWord(pInst, value);
-    diagnostic() << "Instruction word count '"
-             << SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX << "' exceeded.";
-    return SPV_ERROR_INVALID_TEXT;
-  }
-
-  pInst->words[pInst->wordCount++] = (uint32_t)value;
   return SPV_SUCCESS;
 }
 
@@ -340,18 +335,26 @@ spv_result_t AssemblyContext::binaryEncodeU64(const uint64_t value,
 
 spv_result_t AssemblyContext::binaryEncodeString(
     const char *value, spv_instruction_t *pInst) {
-  size_t length = strlen(value);
+  const size_t length = strlen(value);
-  size_t wordCount = (length / 4) + 1;
+  const size_t wordCount = (length / 4) + 1;
-  if ((sizeof(uint32_t) * pInst->wordCount) + length >
+  const size_t oldWordCount = pInst->words.size();
-      sizeof(uint32_t) * SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX) {
+  const size_t newWordCount = oldWordCount + wordCount;
+
+  // TODO(dneto): We can just defer this check until later.
+  if (newWordCount > SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX) {
     diagnostic() << "Instruction word count '"
              << SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX << "'exceeded.";
     return SPV_ERROR_INVALID_TEXT;
   }
 
-  char *dest = (char *)&pInst->words[pInst->wordCount];
+  pInst->words.resize(newWordCount);
+
+  // Make sure all the bytes in the last word are 0, in case we only
+  // write a partial word at the end.
+  pInst->words.back() = 0;
+
+  char *dest = (char *)&pInst->words[oldWordCount];
   strncpy(dest, value, length);
-  pInst->wordCount += (uint16_t)wordCount;
 
   return SPV_SUCCESS;
 }

source/text_handler.h

@@ -31,12 +31,12 @@
 #include <unordered_map>
 
 #include "diagnostic.h"
+#include "instruction.h"
 #include "operand.h"
 
 namespace libspirv {
 // Structures
 
-
 // This is a lattice for tracking types.
 enum class IdTypeClass {
   kBottom, // We have no information yet.

source/validate.cpp

@@ -27,6 +27,7 @@
 #include <libspirv/libspirv.h>
 #include "binary.h"
 #include "diagnostic.h"
+#include "instruction.h"
 #include "opcode.h"
 #include "operand.h"
 #include "validate.h"
@@ -130,7 +131,7 @@ spv_result_t spvValidateBasic(const spv_instruction_t *pInsts,
                               spv_position position,
                               spv_diagnostic *pDiagnostic) {
   for (uint64_t instIndex = 0; instIndex < instCount; ++instIndex) {
-    const uint32_t *words = pInsts[instIndex].words;
+    const uint32_t *words = pInsts[instIndex].words.data();
     uint16_t wordCount;
     Op opcode;
     spvOpcodeSplit(words[0], &wordCount, &opcode);
@@ -150,7 +151,7 @@ spv_result_t spvValidateBasic(const spv_instruction_t *pInsts,
     }
 
     spv_operand_desc operandEntry = nullptr;
-    for (uint16_t index = 1; index < pInsts[instIndex].wordCount;
+    for (uint16_t index = 1; index < pInsts[instIndex].words.size();
          ++index, position->index++) {
       const uint32_t word = words[index];
 
@@ -191,7 +192,7 @@ spv_result_t spvValidateIDs(const spv_instruction_t *pInsts,
   std::vector<spv_id_info_t> idDefs;
 
   for (uint64_t instIndex = 0; instIndex < count; ++instIndex) {
-    const uint32_t *words = pInsts[instIndex].words;
+    const uint32_t *words = pInsts[instIndex].words.data();
     Op opcode;
     spvOpcodeSplit(words[0], nullptr, &opcode);
 
@@ -203,7 +204,7 @@ spv_result_t spvValidateIDs(const spv_instruction_t *pInsts,
 
     spv_operand_desc operandEntry = nullptr;
     position->index++;  // NOTE: Account for Opcode word
-    for (uint16_t index = 1; index < pInsts[instIndex].wordCount;
+    for (uint16_t index = 1; index < pInsts[instIndex].words.size();
          ++index, position->index++) {
       const uint32_t word = words[index];
 

source/validate.h

@@ -27,6 +27,7 @@
 #ifndef _LIBSPIRV_UTIL_VALIDATE_H_
 #define _LIBSPIRV_UTIL_VALIDATE_H_
 
+#include "instruction.h"
 #include <libspirv/libspirv.h>
 
 // Structures

source/validate_id.cpp

@@ -26,6 +26,7 @@
 
 #include <libspirv/libspirv.h>
 #include "diagnostic.h"
+#include "instruction.h"
 #include "opcode.h"
 #include "validate.h"
 
@@ -157,7 +158,7 @@ bool idUsage::isValid<OpMemberName>(const spv_instruction_t *inst,
            return false);
   auto memberIndex = 2;
   auto member = inst->words[memberIndex];
-  auto memberCount = (uint32_t)(type->second.inst->wordCount - 2);
+  auto memberCount = (uint32_t)(type->second.inst->words.size() - 2);
   spvCheck(memberCount <= member, DIAG(memberIndex)
                                       << "OpMemberName Member <id> '"
                                       << inst->words[memberIndex]
@@ -212,7 +213,7 @@ bool idUsage::isValid<OpMemberDecorate>(const spv_instruction_t *inst,
            return false);
   auto memberIndex = 2;
   auto member = inst->words[memberIndex];
-  auto memberCount = (uint32_t)(structType->second.inst->wordCount - 2);
+  auto memberCount = (uint32_t)(structType->second.inst->words.size() - 2);
   spvCheck(memberCount < member, DIAG(memberIndex)
                                      << "OpMemberDecorate Structure type <id> '"
                                      << inst->words[memberIndex]
@@ -237,7 +238,7 @@ bool idUsage::isValid<OpGroupDecorate>(const spv_instruction_t *inst,
                << inst->words[decorationGroupIndex]
                << "' is not a decoration group.";
            return false);
-  for (uint64_t targetIndex = 2; targetIndex < inst->wordCount; ++targetIndex) {
+  for (uint64_t targetIndex = 2; targetIndex < inst->words.size(); ++targetIndex) {
     auto target = find(inst->words[targetIndex]);
     spvCheck(!found(target), DIAG(targetIndex)
                                  << "OpGroupDecorate Target <id> '"
@@ -279,7 +280,7 @@ bool idUsage::isValid<OpEntryPoint>(const spv_instruction_t *inst,
   // to change
   auto entryPointType = find(entryPoint->second.inst->words[4]);
   spvCheck(!found(entryPointType), assert(0 && "Unreachable!"));
-  spvCheck(3 != entryPointType->second.inst->wordCount,
+  spvCheck(3 != entryPointType->second.inst->words.size(),
            DIAG(entryPointIndex) << "OpEntryPoint Entry Point <id> '"
                                  << inst->words[entryPointIndex]
                                  << "'s function parameter count is not zero.";
@@ -406,13 +407,13 @@ bool idUsage::isValid<OpTypeArray>(const spv_instruction_t *inst,
                              << inst->words[lengthIndex]
                              << "' is not a constant integer type.";
            return false);
-  if (4 == constInst->wordCount) {
+  if (4 == constInst->words.size()) {
     spvCheck(1 > constInst->words[3], DIAG(lengthIndex)
                                           << "OpTypeArray Length <id> '"
                                           << inst->words[lengthIndex]
                                           << "' value must be at least 1.";
              return false);
-  } else if (5 == constInst->wordCount) {
+  } else if (5 == constInst->words.size()) {
     uint64_t value =
         constInst->words[3] | ((uint64_t)constInst->words[4]) << 32;
     bool signedness = constResultType->second.inst->words[3];
@@ -453,7 +454,7 @@ bool idUsage::isValid<OpTypeRuntimeArray>(const spv_instruction_t *inst,
 template <>
 bool idUsage::isValid<OpTypeStruct>(const spv_instruction_t *inst,
                                     const spv_opcode_desc) {
-  for (uint64_t memberTypeIndex = 2; memberTypeIndex < inst->wordCount;
+  for (uint64_t memberTypeIndex = 2; memberTypeIndex < inst->words.size();
        ++memberTypeIndex) {
     auto memberType = find(inst->words[memberTypeIndex]);
     spvCheck(!found(memberType), DIAG(memberTypeIndex)
@@ -501,7 +502,7 @@ bool idUsage::isValid<OpTypeFunction>(const spv_instruction_t *inst,
                                  << inst->words[returnTypeIndex]
                                  << "' is not a type.";
            return false);
-  for (uint64_t paramTypeIndex = 3; paramTypeIndex < inst->wordCount;
+  for (uint64_t paramTypeIndex = 3; paramTypeIndex < inst->words.size();
        ++paramTypeIndex) {
     auto paramType = find(inst->words[paramTypeIndex]);
     spvCheck(!found(paramType), DIAG(paramTypeIndex)
@@ -596,21 +597,21 @@ bool idUsage::isValid<OpConstantComposite>(const spv_instruction_t *inst,
                                  << "' is not a composite type.";
            return false);
 
-  uint32_t constituentCount = inst->wordCount - 3;
+  uint32_t constituentCount = inst->words.size() - 3;
   switch (resultType->second.opcode) {
     case OpTypeVector: {
       auto componentCount = resultType->second.inst->words[3];
       spvCheck(
           componentCount != constituentCount,
           // TODO: Output ID's on diagnostic
-          DIAG(inst->wordCount - 1)
+          DIAG(inst->words.size() - 1)
               << "OpConstantComposite Constituent <id> count does not match "
                  "Result Type <id> '"
               << resultType->second.id << "'s vector component count.";
           return false);
       auto componentType = find(resultType->second.inst->words[2]);
       spvCheck(!found(componentType), assert(0 && "Unreachable!"));
-      for (uint64_t constituentIndex = 3; constituentIndex < inst->wordCount;
+      for (uint64_t constituentIndex = 3; constituentIndex < inst->words.size();
            constituentIndex++) {
         auto constituent = find(inst->words[constituentIndex]);
         spvCheck(!found(constituent), assert(0 && "Unreachable!"));
@@ -636,7 +637,7 @@ bool idUsage::isValid<OpConstantComposite>(const spv_instruction_t *inst,
       spvCheck(
           columnCount != constituentCount,
           // TODO: Output ID's on diagnostic
-          DIAG(inst->wordCount - 1)
+          DIAG(inst->words.size() - 1)
               << "OpConstantComposite Constituent <id> count does not match "
                  "Result Type <id> '"
               << resultType->second.id << "'s matrix column count.";
@@ -648,7 +649,7 @@ bool idUsage::isValid<OpConstantComposite>(const spv_instruction_t *inst,
       auto componentType = find(columnType->second.inst->words[2]);
       spvCheck(!found(componentType), assert(0 && "Unreachable!"));
 
-      for (uint64_t constituentIndex = 3; constituentIndex < inst->wordCount;
+      for (uint64_t constituentIndex = 3; constituentIndex < inst->words.size();
            constituentIndex++) {
         auto constituent = find(inst->words[constituentIndex]);
         spvCheck(!found(constituent),
@@ -698,12 +699,12 @@ bool idUsage::isValid<OpConstantComposite>(const spv_instruction_t *inst,
       auto length = find(resultType->second.inst->words[3]);
       spvCheck(!found(length), assert(0 && "Unreachable!"));
       spvCheck(length->second.inst->words[3] != constituentCount,
-               DIAG(inst->wordCount - 1)
+               DIAG(inst->words.size() - 1)
                    << "OpConstantComposite Constituent count does not match "
                       "Result Type <id> '"
                    << resultType->second.id << "'s array length.";
                return false);
-      for (uint64_t constituentIndex = 3; constituentIndex < inst->wordCount;
+      for (uint64_t constituentIndex = 3; constituentIndex < inst->words.size();
            constituentIndex++) {
         auto constituent = find(inst->words[constituentIndex]);
         spvCheck(!found(constituent),
@@ -729,7 +730,7 @@ bool idUsage::isValid<OpConstantComposite>(const spv_instruction_t *inst,
       }
     } break;
     case OpTypeStruct: {
-      uint32_t memberCount = resultType->second.inst->wordCount - 2;
+      uint32_t memberCount = resultType->second.inst->words.size() - 2;
       spvCheck(memberCount != constituentCount,
                DIAG(resultTypeIndex)
                    << "OpConstantComposite Constituent <id> '"
@@ -738,7 +739,7 @@ bool idUsage::isValid<OpConstantComposite>(const spv_instruction_t *inst,
                    << resultType->second.id << "'s struct member count.";
                return false);
       for (uint32_t constituentIndex = 3, memberIndex = 2;
-           constituentIndex < inst->wordCount;
+           constituentIndex < inst->words.size();
            constituentIndex++, memberIndex++) {
         auto constituent = find(inst->words[constituentIndex]);
         spvCheck(!found(constituent),
@@ -841,7 +842,7 @@ bool idUsage::isValid<OpConstantNull>(const spv_instruction_t *inst,
     } break;
     case OpTypeStruct: {
       for (uint64_t elementIndex = 2;
-           elementIndex < resultType->second.inst->wordCount; ++elementIndex) {
+           elementIndex < resultType->second.inst->words.size(); ++elementIndex) {
         auto element = find(resultType->second.inst->words[elementIndex]);
         spvCheck(!found(element), assert(0 && "Unreachable!"));
         spvCheck(!spvOpcodeIsBasicTypeNullable(element->second.inst->opcode),
@@ -936,7 +937,7 @@ bool idUsage::isValid<OpVariable>(const spv_instruction_t *inst,
                                  << inst->words[resultTypeIndex]
                                  << "' is not a pointer type.";
            return false);
-  if (opcodeEntry->numTypes < inst->wordCount) {
+  if (opcodeEntry->numTypes < inst->words.size()) {
     auto initialiserIndex = 4;
     auto initialiser = find(inst->words[initialiserIndex]);
     spvCheck(!found(initialiser), DIAG(initialiserIndex)
@@ -1275,16 +1276,16 @@ bool idUsage::isValid<OpFunctionCall>(const spv_instruction_t *inst,
       return false);
   auto functionType = find(function->second.inst->words[4]);
   spvCheck(!found(functionType), assert(0 && "Unreachable!"));
-  auto functionCallArgCount = inst->wordCount - 4;
+  auto functionCallArgCount = inst->words.size() - 4;
-  auto functionParamCount = functionType->second.inst->wordCount - 3;
+  auto functionParamCount = functionType->second.inst->words.size() - 3;
   spvCheck(
       functionParamCount != functionCallArgCount,
-      DIAG(inst->wordCount - 1)
+      DIAG(inst->words.size() - 1)
           << "OpFunctionCall Function <id>'s parameter count does not match "
              "the argument count.";
       return false);
   for (uint64_t argumentIndex = 4, paramIndex = 3;
-       argumentIndex < inst->wordCount; argumentIndex++, paramIndex++) {
+       argumentIndex < inst->words.size(); argumentIndex++, paramIndex++) {
     auto argument = find(inst->words[argumentIndex]);
     spvCheck(!found(argument), DIAG(argumentIndex)
                                    << "OpFunctionCall Argument <id> '"
@@ -2601,7 +2602,7 @@ spv_result_t spvValidateInstructionIDs(
                   pIdDefs, idDefsCount, pInsts, instCount, position, pDiag);
   for (uint64_t instIndex = 0; instIndex < instCount; ++instIndex) {
     spvCheck(!idUsage.isValid(&pInsts[instIndex]), return SPV_ERROR_INVALID_ID);
-    position->index += pInsts[instIndex].wordCount;
+    position->index += pInsts[instIndex].words.size();
   }
   return SPV_SUCCESS;
 }

test/AssemblyContext.cpp

@@ -0,0 +1,88 @@
+// Copyright (c) 2015 The Khronos Group Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and/or associated documentation files (the
+// "Materials"), to deal in the Materials without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Materials, and to
+// permit persons to whom the Materials are furnished to do so, subject to
+// the following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Materials.
+//
+// MODIFICATIONS TO THIS FILE MAY MEAN IT NO LONGER ACCURATELY REFLECTS
+// KHRONOS STANDARDS. THE UNMODIFIED, NORMATIVE VERSIONS OF KHRONOS
+// SPECIFICATIONS AND HEADER INFORMATION ARE LOCATED AT
+//    https://www.khronos.org/registry/
+//
+// THE MATERIALS ARE PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+// IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+// TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+// MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS.
+
+#include "UnitSPIRV.h"
+
+#include <vector>
+#include <gmock/gmock.h>
+
+#include "../source/instruction.h"
+
+using libspirv::AssemblyContext;
+using spvtest::AutoText;
+using spvtest::Concatenate;
+using ::testing::Eq;
+
+namespace {
+
+struct EncodeStringCase {
+  std::string str;
+  std::vector<uint32_t> initial_contents;
+};
+
+using EncodeStringTest = ::testing::TestWithParam<EncodeStringCase>;
+
+TEST_P(EncodeStringTest, Sample) {
+  AssemblyContext context(AutoText(""), nullptr);
+  spv_instruction_t inst;
+  inst.words = GetParam().initial_contents;
+  ASSERT_EQ(SPV_SUCCESS,
+            context.binaryEncodeString(GetParam().str.c_str(), &inst));
+  // We already trust MakeVector
+  EXPECT_THAT(inst.words,
+              Eq(Concatenate({GetParam().initial_contents,
+                              spvtest::MakeVector(GetParam().str)})));
+}
+
+// clang-format off
+INSTANTIATE_TEST_CASE_P(
+    BinaryEncodeString, EncodeStringTest,
+    ::testing::ValuesIn(std::vector<EncodeStringCase>{
+      // Use cases that exercise at least one to two words,
+      // and both empty and non-empty initial contents.
+      {"", {}},
+      {"", {1,2,3}},
+      {"a", {}},
+      {"a", {4}},
+      {"ab", {4}},
+      {"abc", {}},
+      {"abc", {18}},
+      {"abcd", {}},
+      {"abcd", {22}},
+      {"abcde", {4}},
+      {"abcdef", {}},
+      {"abcdef", {99,42}},
+      {"abcdefg", {}},
+      {"abcdefg", {101}},
+      {"abcdefgh", {}},
+      {"abcdefgh", {102, 103, 104}},
+      // A very long string, encoded after an initial word.
+      // SPIR-V limits strings to 65535 characters.
+      {std::string(65535, 'a'), {1}},
+    }));
+// clang-format on
+
+}  // anonymous namespace

test/TestFixture.h

@@ -115,6 +115,7 @@ class TextToBinaryTestBase : public T {
       spvDiagnosticDestroy(diagnostic);
     }
     EXPECT_EQ(SPV_SUCCESS, error);
+    if (!binary) return "";
 
     spv_text decoded_text;
     error = spvBinaryToText(

test/TextLiteral.cpp

@@ -129,14 +129,14 @@ TEST(TextLiteral, GoodString) {
 TEST(TextLiteral, StringTooLong) {
   spv_literal_t l;
   std::string too_long = std::string("\"") +
-                         std::string(SPV_LIMIT_LITERAL_STRING_MAX - 2, 'a') +
+                         std::string(SPV_LIMIT_LITERAL_STRING_MAX + 1, 'a') +
                          "\"";
   EXPECT_EQ(SPV_ERROR_OUT_OF_MEMORY, spvTextToLiteral(too_long.data(), &l));
 }
 
 TEST(TextLiteral, GoodLongString) {
   spv_literal_t l;
-  std::string unquoted(SPV_LIMIT_LITERAL_STRING_MAX - 3, 'a');
+  std::string unquoted(SPV_LIMIT_LITERAL_STRING_MAX, 'a');
   std::string good_long = std::string("\"") + unquoted + "\"";
   EXPECT_EQ(SPV_SUCCESS, spvTextToLiteral(good_long.data(), &l));
   EXPECT_EQ(SPV_LITERAL_TYPE_STRING, l.type);

test/TextToBinary.Literal.cpp

@@ -50,11 +50,10 @@ TEST_F(TextToBinaryTest, LiteralNumberInPlaceOfLiteralString) {
       CompileFailure(R"(OpSourceExtension 1000)"));
 }
 
-// TODO(antiagainst): libspirv.h defines SPV_LIMIT_INSTRUCTION_WORD_COUNT_MAX
+TEST_F(TextToBinaryTest, LiteralStringTooLong) {
-// to be 0x108. Lift that limit and enable the following test.
+  // SPIR-V allows strings up to 65535 characters.
-TEST_F(TextToBinaryTest, DISABLED_LiteralStringTooLong) {
   const std::string code =
-      "OpSourceExtension \"" + std::string(65534, 'o') + "\"\n";
+      "OpSourceExtension \"" + std::string(65535, 'o') + "\"\n";
   EXPECT_EQ(code, EncodeAndDecodeSuccessfully(code));
 }
 

test/UnitSPIRV.h

@@ -129,6 +129,17 @@ inline std::vector<uint32_t> MakeInstruction(
   return MakeInstruction(opcode, args);
 }
 
+// Returns the vector of words representing the concatenation
+// of all input vectors.
+inline std::vector<uint32_t> Concatenate(
+    const std::vector<std::vector<uint32_t>>& instructions) {
+  std::vector<uint32_t> result;
+  for (const auto& instruction : instructions) {
+    result.insert(result.end(), instruction.begin(), instruction.end());
+  }
+  return result;
+}
+
 // Encodes a string as a sequence of words, using the SPIR-V encoding.
 inline std::vector<uint32_t> MakeVector(std::string input) {
   std::vector<uint32_t> result;