Fix endianness of string literals (#4622)
* Fix endianness of string literals
To get correct and consistent encoding and decoding of string literals
on big-endian platforms, use spvtools::utils::MakeString and MakeVector
(or wrapper functions) consistently for handling string literals.
- add variant of MakeVector that encodes a string literal into an
existing vector of words
- add variants of MakeString
- add a wrapper spvDecodeLiteralStringOperand in source/
- fix wrapper Operand::AsString to use MakeString (source/opt)
- remove Operand::AsCString as broken and unused
- add a variant of GetOperandAs for string literals (source/val)
... and apply those wrappers throughout the code.
Fixes #149
* Extend round trip test for StringLiterals to flip word order
In the encoding/decoding roundtrip tests for string literals, include
a case that flips byte order in words after encoding and then checks for
successful decoding. That is, on a little-endian host flip to big-endian
byte order and then decode, and vice versa.
* BinaryParseTest.InstructionWithStringOperand: also flip byte order
Test binary parsing of string operands both with the host's and with the
reversed byte order.
diff --git a/source/binary.cpp b/source/binary.cpp
index 93d5da7..48a94f1 100644
--- a/source/binary.cpp
+++ b/source/binary.cpp
@@ -33,6 +33,7 @@
#include "source/operand.h"
#include "source/spirv_constant.h"
#include "source/spirv_endian.h"
+#include "source/util/string_utils.h"
spv_result_t spvBinaryHeaderGet(const spv_const_binary binary,
const spv_endianness_t endian,
@@ -62,6 +63,15 @@
return SPV_SUCCESS;
}
+std::string spvDecodeLiteralStringOperand(const spv_parsed_instruction_t& inst,
+ const uint16_t operand_index) {
+ assert(operand_index < inst.num_operands);
+ const spv_parsed_operand_t& operand = inst.operands[operand_index];
+
+ return spvtools::utils::MakeString(inst.words + operand.offset,
+ operand.num_words);
+}
+
namespace {
// A SPIR-V binary parser. A parser instance communicates detailed parse
@@ -577,27 +587,18 @@
case SPV_OPERAND_TYPE_LITERAL_STRING:
case SPV_OPERAND_TYPE_OPTIONAL_LITERAL_STRING: {
- convert_operand_endianness = false;
- const char* string =
- reinterpret_cast<const char*>(_.words + _.word_index);
- // 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 =
- spv_strnlen_s(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)
+ const size_t max_words = _.num_words - _.word_index;
+ std::string string =
+ spvtools::utils::MakeString(_.words + _.word_index, max_words, false);
+
+ if (string.length() == max_words * 4)
return exhaustedInputDiagnostic(inst_offset, 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.
//
// This error can't currently be triggered because of validity
// checks elsewhere.
+ const size_t string_num_words = string.length() / 4 + 1;
if (string_num_words > std::numeric_limits<uint16_t>::max()) {
return diagnostic() << "Literal string is longer than "
<< std::numeric_limits<uint16_t>::max()
@@ -611,7 +612,7 @@
// There is only one string literal argument to OpExtInstImport,
// so it's sufficient to guard this just on the opcode.
const spv_ext_inst_type_t ext_inst_type =
- spvExtInstImportTypeGet(string);
+ spvExtInstImportTypeGet(string.c_str());
if (SPV_EXT_INST_TYPE_NONE == ext_inst_type) {
return diagnostic()
<< "Invalid extended instruction import '" << string << "'";