Perform out-of-bounds checks on texel pointers
VK_EXT_image_robustness requires loads and stores using a pointer
produced by OpImageTexelPointer to be bounds checked. Out-of-bounds
reads must result in [0,0,0,0] or [0,0,0,1] values. We always return
all-zero. Out-of-bounds writes must be ignored.
This also includes the read-modify-write operations performed by atomic
instructions.
Bug: b/159329067
Tests: dEQP-VK.robustness.image_robustness.*
Change-Id: I4f5c2e8ae0bc89e61207fd1f7f1c449a35ca621e
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/47069
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.cpp b/src/Pipeline/SpirvShader.cpp
index 848eefe..afcdbac 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -1510,7 +1510,10 @@
: OutOfBoundsBehavior::UndefinedBehavior;
case spv::StorageClassImage:
- return OutOfBoundsBehavior::UndefinedValue; // "The value returned by a read of an invalid texel is undefined"
+ // VK_EXT_image_robustness requires nullifying out-of-bounds accesses.
+ // TODO(b/159329067): Claim VK_EXT_image_robustness
+ // TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
+ return OutOfBoundsBehavior::Nullify;
case spv::StorageClassInput:
if(executionModel == spv::ExecutionModelVertex)
@@ -2238,17 +2241,24 @@
{
auto &resultType = getType(Type::ID(insn.word(1)));
Object::ID resultId = insn.word(2);
+ Object::ID pointerId = insn.word(3);
Object::ID semanticsId = insn.word(5);
auto memorySemantics = static_cast<spv::MemorySemanticsMask>(getObject(semanticsId).constantValue[0]);
auto memoryOrder = MemoryOrder(memorySemantics);
// Where no value is provided (increment/decrement) use an implicit value of 1.
auto value = (insn.wordCount() == 7) ? Operand(this, state, insn.word(6)).UInt(0) : RValue<SIMD::UInt>(1);
auto &dst = state->createIntermediate(resultId, resultType.componentCount);
- auto ptr = state->getPointer(insn.word(3));
+ auto ptr = state->getPointer(pointerId);
auto ptrOffsets = ptr.offsets();
- SIMD::UInt x(0);
- auto mask = state->activeLaneMask() & state->storesAndAtomicsMask();
+ SIMD::Int mask = state->activeLaneMask() & state->storesAndAtomicsMask();
+
+ if(getObject(pointerId).opcode() == spv::OpImageTexelPointer)
+ {
+ mask &= ptr.isInBounds(sizeof(int32_t), OutOfBoundsBehavior::Nullify);
+ }
+
+ SIMD::UInt result(0);
for(int j = 0; j < SIMD::Width; j++)
{
If(Extract(mask, j) != 0)
@@ -2294,11 +2304,11 @@
UNREACHABLE("%s", OpcodeName(insn.opcode()).c_str());
break;
}
- x = Insert(x, v, j);
+ result = Insert(result, v, j);
}
}
- dst.move(0, x);
+ dst.move(0, result);
return EmitResult::Continue;
}
diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index daa2a4b..e291fe8 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -656,6 +656,7 @@
// emulating the glsl function loadImage() requires that this function returns 0 when used with out of bounds
// coordinates, so we have to use OutOfBoundsBehavior::Nullify in that case.
// TODO(b/159329067): Claim VK_EXT_image_robustness
+ // TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
auto robustness = OutOfBoundsBehavior::Nullify;
auto texelSize = vk::Format(vkFormat).bytes();
@@ -1182,6 +1183,7 @@
// Emulating the glsl function imageStore() requires that this function is noop when used with out of bounds
// coordinates, so we have to use OutOfBoundsBehavior::Nullify in that case.
// TODO(b/159329067): Claim VK_EXT_image_robustness
+ // TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
auto robustness = OutOfBoundsBehavior::Nullify;
auto texelPtr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, texelSize, 0, false, robustness);
@@ -1248,7 +1250,12 @@
Pointer<Byte> imageBase = *Pointer<Pointer<Byte>>(binding + OFFSET(vk::StorageImageDescriptor, ptr));
auto imageSizeInBytes = *Pointer<Int>(binding + OFFSET(vk::StorageImageDescriptor, sizeInBytes));
- auto ptr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, sizeof(uint32_t), 0, false, OutOfBoundsBehavior::UndefinedValue);
+ // VK_EXT_image_robustness requires checking for out-of-bounds accesses.
+ // TODO(b/159329067): Claim VK_EXT_image_robustness
+ // TODO(b/162327166): Only perform bounds checks when VK_EXT_image_robustness is enabled.
+ auto robustness = OutOfBoundsBehavior::Nullify;
+
+ auto ptr = GetTexelAddress(state, imageBase, imageSizeInBytes, coordinate, imageType, binding, sizeof(uint32_t), 0, false, robustness);
state->createPointer(resultId, ptr);
diff --git a/src/WSI/WaylandSurfaceKHR.cpp b/src/WSI/WaylandSurfaceKHR.cpp
index 09589c9..2431673 100644
--- a/src/WSI/WaylandSurfaceKHR.cpp
+++ b/src/WSI/WaylandSurfaceKHR.cpp
@@ -18,8 +18,8 @@
#include "Vulkan/VkImage.hpp"
#include <string.h>
-#include <unistd.h>
#include <sys/mman.h>
+#include <unistd.h>
namespace vk {
@@ -28,7 +28,7 @@
struct wl_shm **pshm = (struct wl_shm **)data;
if(!strcmp(interface, "wl_shm"))
{
- *pshm = static_cast<struct wl_shm*>(wl_registry_bind(registry, name, &wl_shm_interface, 1));
+ *pshm = static_cast<struct wl_shm *>(wl_registry_bind(registry, name, &wl_shm_interface, 1));
}
}
