Validate SPIR-V instruction word array accesses
Previously the InsnIterator::wordPointer() method allowed unchecked
access to SPIR-V instruction words. This change eliminates this method
and instead provides a `Span` helper class to represent a contiguous
range of instruction words, where every access is checked against the
bounds of the instruction size as well as the word count of the span.
Bug: b/191426269
Change-Id: Idaef6432e8a0c489f565428edf4d5a347c6c518c
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/64689
Reviewed-by: Alexis Hétu <sugoi@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index ead8189..8d99850 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -574,7 +574,7 @@
if(opcode == spv::OpAccessChain || opcode == spv::OpInBoundsAccessChain)
{
Decorations dd{};
- ApplyDecorationsForAccessChain(&dd, &descriptorDecorations[resultId], pointerId, insn.wordCount() - 4, insn.wordPointer(4));
+ ApplyDecorationsForAccessChain(&dd, &descriptorDecorations[resultId], pointerId, Span(insn, 4, insn.wordCount() - 4));
// Note: offset is the one thing that does *not* propagate, as the access chain accounts for it.
dd.HasOffset = false;
decorations[resultId].Apply(dd);
@@ -1201,14 +1201,14 @@
VisitInterfaceInner(def.word(1), d, f);
}
-void SpirvShader::ApplyDecorationsForAccessChain(Decorations *d, DescriptorDecorations *dd, Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds) const
+void SpirvShader::ApplyDecorationsForAccessChain(Decorations *d, DescriptorDecorations *dd, Object::ID baseId, const Span &indexIds) const
{
ApplyDecorationsForId(d, baseId);
auto &baseObject = getObject(baseId);
ApplyDecorationsForId(d, baseObject.typeId());
auto typeId = getType(baseObject).element;
- for(auto i = 0u; i < numIndexes; i++)
+ for(uint32_t i = 0; i < indexIds.size(); i++)
{
ApplyDecorationsForId(d, typeId);
auto &type = getType(typeId);
@@ -1242,7 +1242,7 @@
}
}
-SIMD::Pointer SpirvShader::WalkExplicitLayoutAccessChain(Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds, EmitState const *state) const
+SIMD::Pointer SpirvShader::WalkExplicitLayoutAccessChain(Object::ID baseId, const Span &indexIds, const EmitState *state) const
{
// Produce a offset into external memory in sizeof(float) units
@@ -1251,6 +1251,7 @@
Decorations d = GetDecorationsForId(baseObject.typeId());
Int arrayIndex = 0;
+ uint32_t start = 0;
if(baseObject.kind == Object::Kind::DescriptorSet)
{
auto type = getType(typeId).definition.opcode();
@@ -1268,8 +1269,7 @@
arrayIndex = Extract(state->getIntermediate(indexIds[0]).Int(0), 0);
}
- numIndexes--;
- indexIds++;
+ start = 1;
typeId = getType(typeId).element;
}
}
@@ -1278,7 +1278,7 @@
int constantOffset = 0;
- for(auto i = 0u; i < numIndexes; i++)
+ for(uint32_t i = start; i < indexIds.size(); i++)
{
auto &type = getType(typeId);
ApplyDecorationsForId(&d, typeId);
@@ -1353,7 +1353,7 @@
return ptr;
}
-SIMD::Pointer SpirvShader::WalkAccessChain(Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds, EmitState const *state) const
+SIMD::Pointer SpirvShader::WalkAccessChain(Object::ID baseId, const Span &indexIds, EmitState const *state) const
{
// TODO: avoid doing per-lane work in some cases if we can?
auto routine = state->routine;
@@ -1364,7 +1364,7 @@
int constantOffset = 0;
- for(auto i = 0u; i < numIndexes; i++)
+ for(uint32_t i = 0; i < indexIds.size(); i++)
{
auto &type = getType(typeId);
switch(type.opcode())
@@ -1437,11 +1437,11 @@
return ptr;
}
-uint32_t SpirvShader::WalkLiteralAccessChain(Type::ID typeId, uint32_t numIndexes, uint32_t const *indexes) const
+uint32_t SpirvShader::WalkLiteralAccessChain(Type::ID typeId, const Span &indexes) const
{
uint32_t componentOffset = 0;
- for(auto i = 0u; i < numIndexes; i++)
+ for(uint32_t i = 0; i < indexes.size(); i++)
{
auto &type = getType(typeId);
switch(type.opcode())
@@ -1824,7 +1824,7 @@
{
auto text = spvtools::spvInstructionBinaryToText(
vk::SPIRV_VERSION,
- insn.wordPointer(0),
+ insn.data(),
insn.wordCount(),
insns.data(),
insns.size(),
@@ -2202,8 +2202,6 @@
Type::ID typeId = insn.word(1);
Object::ID resultId = insn.word(2);
Object::ID baseId = insn.word(3);
- uint32_t numIndexes = insn.wordCount() - 4;
- const uint32_t *indexes = insn.wordPointer(4);
auto &type = getType(typeId);
ASSERT(type.componentCount == 1);
ASSERT(getObject(resultId).kind == Object::Kind::Pointer);
@@ -2212,12 +2210,12 @@
type.storageClass == spv::StorageClassUniform ||
type.storageClass == spv::StorageClassStorageBuffer)
{
- auto ptr = WalkExplicitLayoutAccessChain(baseId, numIndexes, indexes, state);
+ auto ptr = WalkExplicitLayoutAccessChain(baseId, Span(insn, 4, insn.wordCount() - 4), state);
state->createPointer(resultId, ptr);
}
else
{
- auto ptr = WalkAccessChain(baseId, numIndexes, indexes, state);
+ auto ptr = WalkAccessChain(baseId, Span(insn, 4, insn.wordCount() - 4), state);
state->createPointer(resultId, ptr);
}
@@ -2253,7 +2251,7 @@
auto &dst = state->createIntermediate(insn.resultId(), type.componentCount);
auto &newPartObject = getObject(insn.word(3));
auto &newPartObjectTy = getType(newPartObject);
- auto firstNewComponent = WalkLiteralAccessChain(resultTypeId, insn.wordCount() - 5, insn.wordPointer(5));
+ auto firstNewComponent = WalkLiteralAccessChain(resultTypeId, Span(insn, 5, insn.wordCount() - 5));
Operand srcObjectAccess(this, state, insn.word(4));
Operand newPartObjectAccess(this, state, insn.word(3));
@@ -2283,7 +2281,7 @@
auto &dst = state->createIntermediate(insn.resultId(), type.componentCount);
auto &compositeObject = getObject(insn.word(3));
Type::ID compositeTypeId = compositeObject.definition.word(1);
- auto firstComponent = WalkLiteralAccessChain(compositeTypeId, insn.wordCount() - 4, insn.wordPointer(4));
+ auto firstComponent = WalkLiteralAccessChain(compositeTypeId, Span(insn, 4, insn.wordCount() - 4));
Operand compositeObjectAccess(this, state, insn.word(3));
for(auto i = 0u; i < type.componentCount; i++)
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 65ebf69..561032b 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -166,7 +166,7 @@
class Type;
class Object;
- // Pseudo-iterator over SPIRV instructions, designed to support range-based-for.
+ // Pseudo-iterator over SPIR-V instructions, designed to support range-based-for.
class InsnIterator
{
public:
@@ -195,14 +195,14 @@
return iter[n];
}
- uint32_t const *wordPointer(uint32_t n) const
+ const uint32_t *data() const
{
- return &iter[n];
+ return &iter[0];
}
const char *string(uint32_t n) const
{
- return reinterpret_cast<const char *>(wordPointer(n));
+ return reinterpret_cast<const char *>(&iter[n]);
}
// Returns the number of whole-words that a string literal starting at
@@ -214,8 +214,7 @@
uint32_t c = wordCount();
for(uint32_t i = n; n < c; i++)
{
- auto *u32 = wordPointer(i);
- auto *u8 = reinterpret_cast<const uint8_t *>(u32);
+ const char *s = string(i);
// SPIR-V spec 2.2.1. Instructions:
// A string is interpreted as a nul-terminated stream of
// characters. The character set is Unicode in the UTF-8
@@ -225,7 +224,7 @@
// The final word contains the string's nul-termination
// character (0), and all contents past the end of the string in
// the final word are padded with 0.
- if(u8[3] == 0)
+ if(s[3] == 0)
{
return 1 + i - n;
}
@@ -303,6 +302,32 @@
return InsnIterator{ insns.cend() };
}
+ // A range of contiguous instruction words.
+ struct Span
+ {
+ Span(const InsnIterator &insn, uint32_t offset, uint32_t size)
+ : insn(insn)
+ , offset(offset)
+ , wordCount(size)
+ {}
+
+ uint32_t operator[](uint32_t index) const
+ {
+ ASSERT(index < wordCount);
+ return insn.word(offset + index);
+ }
+
+ uint32_t size() const
+ {
+ return wordCount;
+ }
+
+ private:
+ const InsnIterator &insn;
+ const uint32_t offset;
+ const uint32_t wordCount;
+ };
+
class Type
{
public:
@@ -933,7 +958,7 @@
Decorations GetDecorationsForId(TypeOrObjectID id) const;
void ApplyDecorationsForId(Decorations *d, TypeOrObjectID id) const;
void ApplyDecorationsForIdMember(Decorations *d, Type::ID id, uint32_t member) const;
- void ApplyDecorationsForAccessChain(Decorations *d, DescriptorDecorations *dd, Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds) const;
+ void ApplyDecorationsForAccessChain(Decorations *d, DescriptorDecorations *dd, Object::ID baseId, const Span &indexIds) const;
// Creates an Object for the instruction's result in 'defs'.
void DefineResult(const InsnIterator &insn);
@@ -1253,11 +1278,11 @@
OutOfBoundsBehavior getOutOfBoundsBehavior(Object::ID pointerId, EmitState const *state) const;
- SIMD::Pointer WalkExplicitLayoutAccessChain(Object::ID id, uint32_t numIndexes, uint32_t const *indexIds, EmitState const *state) const;
- SIMD::Pointer WalkAccessChain(Object::ID id, uint32_t numIndexes, uint32_t const *indexIds, EmitState const *state) const;
+ SIMD::Pointer WalkExplicitLayoutAccessChain(Object::ID id, const Span &indexIds, const EmitState *state) const;
+ SIMD::Pointer WalkAccessChain(Object::ID id, const Span &indexIds, const EmitState *state) const;
// Returns the *component* offset in the literal for the given access chain.
- uint32_t WalkLiteralAccessChain(Type::ID id, uint32_t numIndexes, uint32_t const *indexes) const;
+ uint32_t WalkLiteralAccessChain(Type::ID id, const Span &indexes) const;
// Lookup the active lane mask for the edge from -> to.
// If from is unreachable, then a mask of all zeros is returned.
diff --git a/src/Pipeline/SpirvShaderDebugger.cpp b/src/Pipeline/SpirvShaderDebugger.cpp
index 0f25a32..b0559af 100644
--- a/src/Pipeline/SpirvShaderDebugger.cpp
+++ b/src/Pipeline/SpirvShaderDebugger.cpp
@@ -2571,13 +2571,13 @@
{
auto instruction = spvtools::spvInstructionBinaryToText(
vk::SPIRV_VERSION,
- insn.wordPointer(0),
+ insn.data(),
insn.wordCount(),
insns.data(),
insns.size(),
SPV_BINARY_TO_TEXT_OPTION_NO_HEADER) +
"\n";
- dbg->spirvLineMappings[insn.wordPointer(0)] = currentLine;
+ dbg->spirvLineMappings[insn.data()] = currentLine;
currentLine += std::count(instruction.begin(), instruction.end(), '\n');
source += instruction;
}
@@ -2670,7 +2670,7 @@
{
auto instruction = spvtools::spvInstructionBinaryToText(
vk::SPIRV_VERSION,
- insn.wordPointer(0),
+ insn.data(),
insn.wordCount(),
insns.data(),
insns.size(),
@@ -2683,7 +2683,7 @@
{
auto instruction = spvtools::spvInstructionBinaryToText(
vk::SPIRV_VERSION,
- insn.wordPointer(0),
+ insn.data(),
insn.wordCount(),
insns.data(),
insns.size(),
@@ -2711,7 +2711,7 @@
// We're emitting debugger logic for SPIR-V.
if(IsStatement(insn.opcode()))
{
- auto line = dbg->spirvLineMappings.at(insn.wordPointer(0));
+ auto line = dbg->spirvLineMappings.at(insn.data());
dbg->setLocation(state, dbg->spirvFile, line);
}
}
@@ -2735,7 +2735,7 @@
break;
default:
{
- auto resIt = dbg->results.find(insn.wordPointer(0));
+ auto resIt = dbg->results.find(insn.data());
if(resIt != dbg->results.end())
{
dbg->shadow.create(this, state, resIt->second);
@@ -2759,7 +2759,7 @@
auto dbg = impl.debugger;
if(!dbg) { return; }
- dbg->results.emplace(insn.wordPointer(0), resultId);
+ dbg->results.emplace(insn.data(), resultId);
}
SpirvShader::EmitResult SpirvShader::EmitLine(InsnIterator insn, EmitState *state) const
@@ -2779,7 +2779,7 @@
{
auto instruction = spvtools::spvInstructionBinaryToText(
vk::SPIRV_VERSION,
- insn.wordPointer(0),
+ insn.data(),
insn.wordCount(),
insns.data(),
insns.size(),
diff --git a/src/Pipeline/SpirvShaderSpec.cpp b/src/Pipeline/SpirvShaderSpec.cpp
index 7da7396..fc4806a 100644
--- a/src/Pipeline/SpirvShaderSpec.cpp
+++ b/src/Pipeline/SpirvShaderSpec.cpp
@@ -85,7 +85,7 @@
{
auto &result = CreateConstant(insn);
auto const &compositeObject = getObject(insn.word(4));
- auto firstComponent = WalkLiteralAccessChain(compositeObject.typeId(), insn.wordCount() - 5, insn.wordPointer(5));
+ auto firstComponent = WalkLiteralAccessChain(compositeObject.typeId(), Span(insn, 5, insn.wordCount() - 5));
for(auto i = 0u; i < getType(result).componentCount; i++)
{
@@ -99,7 +99,7 @@
auto &result = CreateConstant(insn);
auto const &newPart = getObject(insn.word(4));
auto const &oldObject = getObject(insn.word(5));
- auto firstNewComponent = WalkLiteralAccessChain(result.typeId(), insn.wordCount() - 6, insn.wordPointer(6));
+ auto firstNewComponent = WalkLiteralAccessChain(result.typeId(), Span(insn, 6, insn.wordCount() - 6));
// old components before
for(auto i = 0u; i < firstNewComponent; i++)
