Commit 8afe3f17 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Fix use of uninitialized data in staged clears

When depth/stencil data are staged, only the depthStencil field of VkClearValue is initialized. However, when comparing staged clears, memcmp is used which also compares the extra bytes in the aliasing color field. Bug: chromium:1144491 Change-Id: Ic384ba792e9fd199d8e9c3e534ccdc6ea65ee9b8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2517244 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org>
parent a51b57fa
...@@ -2004,8 +2004,8 @@ void FramebufferVk::mergeClearsWithDeferredClears( ...@@ -2004,8 +2004,8 @@ void FramebufferVk::mergeClearsWithDeferredClears(
// Depth and stencil clears. // Depth and stencil clears.
VkImageAspectFlags dsAspectFlags = 0; VkImageAspectFlags dsAspectFlags = 0;
VkClearValue dsClearValue; VkClearValue dsClearValue = {};
dsClearValue.depthStencil = clearDepthStencilValue; dsClearValue.depthStencil = clearDepthStencilValue;
if (clearDepth) if (clearDepth)
{ {
dsAspectFlags |= VK_IMAGE_ASPECT_DEPTH_BIT; dsAspectFlags |= VK_IMAGE_ASPECT_DEPTH_BIT;
...@@ -2080,8 +2080,8 @@ angle::Result FramebufferVk::clearWithDraw(ContextVk *contextVk, ...@@ -2080,8 +2080,8 @@ angle::Result FramebufferVk::clearWithDraw(ContextVk *contextVk,
VkClearValue FramebufferVk::getCorrectedColorClearValue(size_t colorIndexGL, VkClearValue FramebufferVk::getCorrectedColorClearValue(size_t colorIndexGL,
const VkClearColorValue &clearColor) const const VkClearColorValue &clearColor) const
{ {
VkClearValue clearValue; VkClearValue clearValue = {};
clearValue.color = clearColor; clearValue.color = clearColor;
if (!mEmulatedAlphaAttachmentMask[colorIndexGL]) if (!mEmulatedAlphaAttachmentMask[colorIndexGL])
{ {
...@@ -2116,8 +2116,8 @@ void FramebufferVk::redeferClears(ContextVk *contextVk) ...@@ -2116,8 +2116,8 @@ void FramebufferVk::redeferClears(ContextVk *contextVk)
ASSERT(!contextVk->hasStartedRenderPass()); ASSERT(!contextVk->hasStartedRenderPass());
// Set the appropriate loadOp and clear values for depth and stencil. // Set the appropriate loadOp and clear values for depth and stencil.
VkImageAspectFlags dsAspectFlags = 0; VkImageAspectFlags dsAspectFlags = 0;
VkClearValue dsClearValue; VkClearValue dsClearValue = {};
dsClearValue.depthStencil.depth = mDeferredClears.getDepthValue(); dsClearValue.depthStencil.depth = mDeferredClears.getDepthValue();
dsClearValue.depthStencil.stencil = mDeferredClears.getStencilValue(); dsClearValue.depthStencil.stencil = mDeferredClears.getStencilValue();
...@@ -2174,8 +2174,8 @@ angle::Result FramebufferVk::clearWithCommand(ContextVk *contextVk, ...@@ -2174,8 +2174,8 @@ angle::Result FramebufferVk::clearWithCommand(ContextVk *contextVk,
} }
// Add depth and stencil to list of attachments as needed. // Add depth and stencil to list of attachments as needed.
VkImageAspectFlags dsAspectFlags = 0; VkImageAspectFlags dsAspectFlags = 0;
VkClearValue dsClearValue; VkClearValue dsClearValue = {};
dsClearValue.depthStencil.depth = mDeferredClears.getDepthValue(); dsClearValue.depthStencil.depth = mDeferredClears.getDepthValue();
dsClearValue.depthStencil.stencil = mDeferredClears.getStencilValue(); dsClearValue.depthStencil.stencil = mDeferredClears.getStencilValue();
if (mDeferredClears.testDepth()) if (mDeferredClears.testDepth())
...@@ -2354,7 +2354,7 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk, ...@@ -2354,7 +2354,7 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
if (mDeferredClears.testDepth() || mDeferredClears.testStencil()) if (mDeferredClears.testDepth() || mDeferredClears.testStencil())
{ {
VkClearValue clearValue; VkClearValue clearValue = {};
if (mDeferredClears.testDepth()) if (mDeferredClears.testDepth())
{ {
......
...@@ -556,7 +556,7 @@ const angle::Format &GetDepthStencilImageToBufferFormat(const angle::Format &ima ...@@ -556,7 +556,7 @@ const angle::Format &GetDepthStencilImageToBufferFormat(const angle::Format &ima
VkClearValue GetRobustResourceClearValue(const Format &format) VkClearValue GetRobustResourceClearValue(const Format &format)
{ {
VkClearValue clearValue; VkClearValue clearValue = {};
if (format.intendedFormat().hasDepthOrStencilBits()) if (format.intendedFormat().hasDepthOrStencilBits())
{ {
clearValue.depthStencil = kRobustInitDepthStencilValue; clearValue.depthStencil = kRobustInitDepthStencilValue;
...@@ -4860,9 +4860,11 @@ void ImageHelper::stageClearIfEmulatedFormat(bool isRobustResourceInitEnabled) ...@@ -4860,9 +4860,11 @@ void ImageHelper::stageClearIfEmulatedFormat(bool isRobustResourceInitEnabled)
{ {
// Skip staging extra clears if robust resource init is enabled. // Skip staging extra clears if robust resource init is enabled.
if (!mFormat->hasEmulatedImageChannels() || isRobustResourceInitEnabled) if (!mFormat->hasEmulatedImageChannels() || isRobustResourceInitEnabled)
{
return; return;
}
VkClearValue clearValue; VkClearValue clearValue = {};
if (mFormat->intendedFormat().hasDepthOrStencilBits()) if (mFormat->intendedFormat().hasDepthOrStencilBits())
{ {
clearValue.depthStencil = kRobustInitDepthStencilValue; clearValue.depthStencil = kRobustInitDepthStencilValue;
......
...@@ -2208,6 +2208,95 @@ TEST_P(RobustResourceInitTest, CopyTexImageToOffsetCubeMap) ...@@ -2208,6 +2208,95 @@ TEST_P(RobustResourceInitTest, CopyTexImageToOffsetCubeMap)
EXPECT_PIXEL_COLOR_EQ(1, 1, GLColor::red); EXPECT_PIXEL_COLOR_EQ(1, 1, GLColor::red);
} }
// Test that blit between two depth/stencil buffers after glClearBufferfi works. The blit is done
// once expecting robust resource init value, then clear is called with the same value as the robust
// init, and blit is done again. This triggers an optimization in the Vulkan backend where the
// second clear is no-oped.
TEST_P(RobustResourceInitTestES3, BlitDepthStencilAfterClearBuffer)
{
// http://anglebug.com/5301
ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
// http://anglebug.com/5300
ANGLE_SKIP_TEST_IF(IsD3D11());
// http://anglebug.com/4919
ANGLE_SKIP_TEST_IF(IsIntel() && IsMetal());
constexpr GLsizei kSize = 16;
GLFramebuffer readFbo, drawFbo;
GLRenderbuffer readDepthStencil, drawDepthStencil;
// Create destination framebuffer.
glBindRenderbuffer(GL_RENDERBUFFER, drawDepthStencil);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, kSize, kSize);
glBindFramebuffer(GL_FRAMEBUFFER, drawFbo);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
drawDepthStencil);
ASSERT_GL_NO_ERROR();
// Create source framebuffer
glBindRenderbuffer(GL_RENDERBUFFER, readDepthStencil);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, kSize, kSize);
glBindFramebuffer(GL_FRAMEBUFFER, readFbo);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
readDepthStencil);
ASSERT_GL_NO_ERROR();
// Blit once with the robust resource init clear.
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, drawFbo);
glBlitFramebuffer(0, 0, kSize, kSize, 0, 0, kSize, kSize,
GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT, GL_NEAREST);
ASSERT_GL_NO_ERROR();
// Verify that the blit was successful.
GLRenderbuffer color;
glBindRenderbuffer(GL_RENDERBUFFER, color);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, kSize, kSize);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, color);
ASSERT_GL_NO_ERROR();
ANGLE_GL_PROGRAM(drawRed, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
glEnable(GL_DEPTH_TEST);
glDepthFunc(GL_LESS);
glEnable(GL_STENCIL_TEST);
glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
glStencilFunc(GL_EQUAL, 0, 0xFF);
drawQuad(drawRed, essl1_shaders::PositionAttrib(), 0.95f, 1.0f, true);
ASSERT_GL_NO_ERROR();
glBindFramebuffer(GL_READ_FRAMEBUFFER, drawFbo);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::red);
EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::red);
EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::red);
// Clear to the same value as robust init, and blit again.
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, readFbo);
glClearBufferfi(GL_DEPTH_STENCIL, 0, 1.0f, 0);
glBindFramebuffer(GL_READ_FRAMEBUFFER, readFbo);
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, drawFbo);
glClearBufferfi(GL_DEPTH_STENCIL, 0, 0.5f, 0x3C);
glBlitFramebuffer(0, 0, kSize, kSize, 0, 0, kSize, kSize,
GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT, GL_NEAREST);
ASSERT_GL_NO_ERROR();
// Verify that the blit was successful.
ANGLE_GL_PROGRAM(drawGreen, essl1_shaders::vs::Simple(), essl1_shaders::fs::Green());
drawQuad(drawGreen, essl1_shaders::PositionAttrib(), 0.95f, 1.0f, true);
ASSERT_GL_NO_ERROR();
glBindFramebuffer(GL_READ_FRAMEBUFFER, drawFbo);
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
EXPECT_PIXEL_COLOR_EQ(kSize - 1, 0, GLColor::green);
EXPECT_PIXEL_COLOR_EQ(0, kSize - 1, GLColor::green);
EXPECT_PIXEL_COLOR_EQ(kSize - 1, kSize - 1, GLColor::green);
}
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND(RobustResourceInitTest, ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND(RobustResourceInitTest,
WithAllocateNonZeroMemory(ES2_VULKAN())); WithAllocateNonZeroMemory(ES2_VULKAN()));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment