Vulkan: Make sure the default uniform info arrays match indices.
When inserting uniform layout info into the default uniform block, the array
sizes need to match but insertion was skipped when the uniform was a sampler.
This caused heap corruption when uniform data was written to the wrong offset
in the buffer.
BUG=angleproject:2859
Change-Id: Ia58a68870e2a4805391266dfe73fe8c8d238931d
Reviewed-on: https://chromium-review.googlesource.com/1249562
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
diff --git a/src/libANGLE/renderer/vulkan/ProgramVk.cpp b/src/libANGLE/renderer/vulkan/ProgramVk.cpp
index 5175b2b..70c0cca 100644
--- a/src/libANGLE/renderer/vulkan/ProgramVk.cpp
+++ b/src/libANGLE/renderer/vulkan/ProgramVk.cpp
@@ -68,6 +68,7 @@
{
uint32_t arrayOffset = arrayIndex * layoutInfo.arrayStride;
uint8_t *writePtr = dst + arrayOffset;
+ ASSERT(writePtr + (elementSize * count) <= uniformData->data() + uniformData->size());
memcpy(writePtr, v, elementSize * count);
}
else
@@ -80,6 +81,7 @@
const int arrayOffset = writeIndex * layoutInfo.arrayStride;
uint8_t *writePtr = dst + arrayOffset;
const T *readPtr = v + (readIndex * componentCount);
+ ASSERT(writePtr + elementSize <= uniformData->data() + uniformData->size());
memcpy(writePtr, readPtr, elementSize);
}
}
@@ -372,30 +374,29 @@
if (location.used() && !location.ignored)
{
const auto &uniform = uniforms[location.index];
-
- if (uniform.isSampler())
- continue;
-
- std::string uniformName = uniform.name;
- if (uniform.isArray())
+ if (!uniform.isSampler())
{
- // Gets the uniform name without the [0] at the end.
- uniformName = gl::ParseResourceName(uniformName, nullptr);
- }
-
- bool found = false;
-
- for (vk::ShaderType shaderType : vk::AllShaderTypes())
- {
- auto it = layoutMap[shaderType].find(uniformName);
- if (it != layoutMap[shaderType].end())
+ std::string uniformName = uniform.name;
+ if (uniform.isArray())
{
- found = true;
- layoutInfo[shaderType] = it->second;
+ // Gets the uniform name without the [0] at the end.
+ uniformName = gl::ParseResourceName(uniformName, nullptr);
}
- }
- ASSERT(found);
+ bool found = false;
+
+ for (vk::ShaderType shaderType : vk::AllShaderTypes())
+ {
+ auto it = layoutMap[shaderType].find(uniformName);
+ if (it != layoutMap[shaderType].end())
+ {
+ found = true;
+ layoutInfo[shaderType] = it->second;
+ }
+ }
+
+ ASSERT(found);
+ }
}
for (vk::ShaderType shaderType : vk::AllShaderTypes())
diff --git a/src/tests/gl_tests/BindUniformLocationTest.cpp b/src/tests/gl_tests/BindUniformLocationTest.cpp
index 695282f..1674f70 100644
--- a/src/tests/gl_tests/BindUniformLocationTest.cpp
+++ b/src/tests/gl_tests/BindUniformLocationTest.cpp
@@ -7,6 +7,7 @@
// BindUniformLocationTest.cpp : Tests of the GL_CHROMIUM_bind_uniform_location extension.
#include "test_utils/ANGLETest.h"
+#include "test_utils/gl_raii.h"
#include <cmath>
@@ -111,6 +112,73 @@
EXPECT_PIXEL_NEAR(0, 0, 64, 128, 192, 255, 1.0);
}
+// Force a sampler location and make sure it samples the correct texture
+TEST_P(BindUniformLocationTest, SamplerLocation)
+{
+ ANGLE_SKIP_TEST_IF(!extensionEnabled("GL_CHROMIUM_bind_uniform_location"));
+
+ ASSERT_NE(mBindUniformLocation, nullptr);
+
+ const std::string fsSource =
+ R"(precision mediump float;
+ uniform vec4 u_colorA;
+ uniform vec4 u_colorB[2];
+ uniform sampler2D u_sampler;
+ void main()
+ {
+ gl_FragColor = u_colorA + u_colorB[0] + u_colorB[1] + texture2D(u_sampler, vec2(0, 0));
+ })";
+
+ GLint colorALocation = 3;
+ GLint colorBLocation = 10;
+ GLint samplerLocation = 1;
+
+ GLuint vs = CompileShader(GL_VERTEX_SHADER, essl1_shaders::vs::Simple());
+ GLuint fs = CompileShader(GL_FRAGMENT_SHADER, fsSource);
+
+ mProgram = glCreateProgram();
+
+ mBindUniformLocation(mProgram, colorALocation, "u_colorA");
+ mBindUniformLocation(mProgram, colorBLocation, "u_colorB[0]");
+ mBindUniformLocation(mProgram, samplerLocation, "u_sampler");
+
+ glAttachShader(mProgram, vs);
+ glDeleteShader(vs);
+
+ glAttachShader(mProgram, fs);
+ glDeleteShader(fs);
+
+ // Link the mProgram
+ glLinkProgram(mProgram);
+ // Check the link status
+ GLint linked = 0;
+ glGetProgramiv(mProgram, GL_LINK_STATUS, &linked);
+ ASSERT_EQ(1, linked);
+
+ glUseProgram(mProgram);
+
+ static const float colorB[] = {
+ 0.0f, 0.50f, 0.0f, 0.0f, 0.0f, 0.0f, 0.75f, 0.0f,
+ };
+
+ glUniform4f(colorALocation, 0.25f, 0.0f, 0.0f, 0.0f);
+ glUniform4fv(colorBLocation, 2, colorB);
+
+ // Point the texture at texture unit 2
+ glUniform1i(samplerLocation, 2);
+
+ GLTexture texture;
+ glActiveTexture(GL_TEXTURE2);
+ glBindTexture(GL_TEXTURE_2D, texture);
+ constexpr GLubyte kTextureData[] = {32, 32, 32, 255};
+ glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, kTextureData);
+
+ drawQuad(mProgram, essl1_shaders::PositionAttrib(), 0.5f);
+
+ EXPECT_GL_NO_ERROR();
+ EXPECT_PIXEL_NEAR(0, 0, 96, 160, 224, 255, 1.0);
+}
+
// Test that conflicts are detected when two uniforms are bound to the same location
TEST_P(BindUniformLocationTest, ConflictsDetection)
{