Replace instead of add pointer offsets when out of bounds
Adding large values can overflow the already out-of-bounds offsets to
become negative. Negative offsets are not detected as out-of-bounds, so
it would result in an out-of-bounds access. This can also happen when
they were already large negative values and remain negative after
the addition. Or, when they're small negative values, they can become
in bounds and thus not trigger the desired out-of-bounds behavior.
Just replace the offsets with the out-of-bounds values.
Bug: b/155862459
Change-Id: I38a45832bfea3a13a66f133ee7c2162855f02c61
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/45091
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 4aaaa61..793a1ef 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -1213,7 +1213,7 @@
void EmitImageSampleUnconditional(Array<SIMD::Float> &out, ImageInstruction instruction, InsnIterator insn, EmitState *state) const;
void GetImageDimensions(EmitState const *state, Type const &resultTy, Object::ID imageId, Object::ID lodId, Intermediate &dst) const;
- SIMD::Pointer GetTexelAddress(EmitState const *state, SIMD::Pointer base, Operand const &coordinate, Type const &imageType, Pointer<Byte> descriptor, int texelSize, Object::ID sampleId, bool useStencilAspect, OutOfBoundsBehavior outOfBoundsBehavior) const;
+ SIMD::Pointer GetTexelAddress(EmitState const *state, Pointer<Byte> imageBase, Int imageSizeInBytes, Operand const &coordinate, Type const &imageType, Pointer<Byte> descriptor, int texelSize, Object::ID sampleId, bool useStencilAspect, OutOfBoundsBehavior outOfBoundsBehavior) const;
uint32_t GetConstScalarInt(Object::ID id) const;
void EvalSpecConstantOp(InsnIterator insn);
void EvalSpecConstantUnaryOp(InsnIterator insn);
diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index 145b8a9..d8c76dd 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -476,7 +476,7 @@
return EmitResult::Continue;
}
-SIMD::Pointer SpirvShader::GetTexelAddress(EmitState const *state, SIMD::Pointer basePtr, Operand const &coordinate, Type const &imageType, Pointer<Byte> descriptor, int texelSize, Object::ID sampleId, bool useStencilAspect, OutOfBoundsBehavior outOfBoundsBehavior) const
+SIMD::Pointer SpirvShader::GetTexelAddress(EmitState const *state, Pointer<Byte> imageBase, Int imageSizeInBytes, Operand const &coordinate, Type const &imageType, Pointer<Byte> descriptor, int texelSize, Object::ID sampleId, bool useStencilAspect, OutOfBoundsBehavior outOfBoundsBehavior) const
{
auto routine = state->routine;
bool isArrayed = imageType.definition.word(5) != 0;
@@ -564,14 +564,13 @@
if(nullifyOutOfBounds)
{
- // Because pointers can be 32b, using 0xFFFFFFFF would wrap around to about the same point,
- // so use a smaller number
- static constexpr int FORCE_OOB = 0x3FFFFFFF; // ~1 Gb, should be larger than any allocated image
- // Push pointers out of bounds if they are oob in any dimension
- ptrOffset += (oobMask & SIMD::Int(FORCE_OOB));
+ constexpr int32_t OOB_OFFSET = 0x7FFFFFFF - 16; // SIMD pointer offsets are signed 32-bit, so this is the largest offset (for 16-byte texels).
+ static_assert(OOB_OFFSET >= MAX_MEMORY_ALLOCATION_SIZE, "the largest offset must be guaranteed to be out-of-bounds");
+
+ ptrOffset = (ptrOffset & ~oobMask) | (oobMask & SIMD::Int(OOB_OFFSET)); // oob ? OOB_OFFSET : ptrOffset // TODO: IfThenElse()
}
- return basePtr + ptrOffset;
+ return SIMD::Pointer(imageBase, imageSizeInBytes, ptrOffset);
}
SpirvShader::EmitResult SpirvShader::EmitImageRead(InsnIterator insn, EmitState *state) const
@@ -639,8 +638,7 @@
auto robustness = OutOfBoundsBehavior::Nullify;
auto texelSize = vk::Format(vkFormat).bytes();
- auto basePtr = SIMD::Pointer(imageBase, imageSizeInBytes);
- auto texelPtr = GetTexelAddress(state, basePtr, coordinate, imageType, binding, texelSize, sampleId, useStencilAspect, robustness);
+ auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, sampleId, useStencilAspect, robustness);
SIMD::Int packed[4];
// Round up texel size: for formats smaller than 32 bits per texel, we will emit a bunch
@@ -1022,8 +1020,7 @@
// coordinates, so we have to use OutOfBoundsBehavior::Nullify in that case.
auto robustness = OutOfBoundsBehavior::Nullify;
- auto basePtr = SIMD::Pointer(imageBase, imageSizeInBytes);
- auto texelPtr = GetTexelAddress(state, basePtr, coordinate, imageType, binding, texelSize, 0, false, robustness);
+ auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, 0, false, robustness);
for(auto i = 0u; i < numPackedElements; i++)
{
@@ -1054,8 +1051,7 @@
Pointer<Byte> imageBase = *Pointer<Pointer<Byte>>(binding + OFFSET(vk::StorageImageDescriptor, ptr));
auto imageSizeInBytes = *Pointer<Int>(binding + OFFSET(vk::StorageImageDescriptor, sizeInBytes));
- auto basePtr = SIMD::Pointer(imageBase, imageSizeInBytes);
- auto ptr = GetTexelAddress(state, basePtr, coordinate, imageType, binding, sizeof(uint32_t), 0, false, OutOfBoundsBehavior::UndefinedValue);
+ auto ptr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, sizeof(uint32_t), 0, false, OutOfBoundsBehavior::UndefinedValue);
state->createPointer(resultId, ptr);
