Vulkan: Fix a bug releasing DynamicBuffer-owned buffer

src/libANGLE/renderer/vulkan/BufferVk.cpp

@@ -606,8 +606,6 @@ angle::Result BufferVk::mapRangeImpl(ContextVk *contextVk,
         {
             // We try to map buffer, but buffer is busy. Caller has told us it doesn't care about
             // previous content. Instead of wait for GPU to finish, we just allocate a new buffer.
-            RendererVk *renderer = contextVk->getRenderer();
-            mBuffer->release(renderer);
             ANGLE_TRY(acquireBufferHelper(contextVk, static_cast<size_t>(mState.getSize())));
         }
         else if ((access & GL_MAP_UNSYNCHRONIZED_BIT) == 0)

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -2047,6 +2047,9 @@ angle::Result DynamicBuffer::allocateWithAlignment(ContextVk *contextVk,
     {
         if (mBuffer)
         {
+            // Make sure the buffer is not released externally.
+            ASSERT(mBuffer->valid());
+
             ANGLE_TRY(flush(contextVk));
 
             mInFlightBuffers.push_back(std::move(mBuffer));

src/tests/gl_tests/BufferDataTest.cpp

@@ -824,6 +824,95 @@ TEST_P(BufferDataTestES3, BufferDataUnmap)
     ASSERT_GL_NO_ERROR();
 }
 
+// Ensures that mapping buffer with GL_MAP_INVALIDATE_BUFFER_BIT followed by glBufferSubData calls
+// works.  Regression test for the Vulkan backend where that flag caused use after free.
+TEST_P(BufferDataTestES3, MapInvalidateThenBufferSubData)
+{
+    // http://anglebug.com/5984
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsOpenGL() && IsIntel());
+
+    // http://anglebug.com/5985
+    ANGLE_SKIP_TEST_IF(IsNexus5X() && IsOpenGLES());
+
+    const std::array<GLColor, 4> kInitialData = {GLColor::red, GLColor::red, GLColor::red,
+                                                 GLColor::red};
+    const std::array<GLColor, 4> kUpdateData1 = {GLColor::white, GLColor::white, GLColor::white,
+                                                 GLColor::white};
+    const std::array<GLColor, 4> kUpdateData2 = {GLColor::blue, GLColor::blue, GLColor::blue,
+                                                 GLColor::blue};
+
+    GLBuffer buffer;
+    glBindBuffer(GL_UNIFORM_BUFFER, buffer);
+    glBufferData(GL_UNIFORM_BUFFER, sizeof(kInitialData), kInitialData.data(), GL_DYNAMIC_DRAW);
+    glBindBufferBase(GL_UNIFORM_BUFFER, 0, buffer);
+    EXPECT_GL_NO_ERROR();
+
+    // Draw
+    constexpr char kVerifyUBO[] = R"(#version 300 es
+precision mediump float;
+uniform block {
+    uvec4 data;
+} ubo;
+uniform uint expect;
+uniform vec4 successOutput;
+out vec4 colorOut;
+void main()
+{
+    if (all(equal(ubo.data, uvec4(expect))))
+        colorOut = successOutput;
+    else
+        colorOut = vec4(1.0, 0, 0, 1.0);
+})";
+
+    ANGLE_GL_PROGRAM(verifyUbo, essl3_shaders::vs::Simple(), kVerifyUBO);
+    glUseProgram(verifyUbo);
+
+    GLint expectLoc = glGetUniformLocation(verifyUbo, "expect");
+    EXPECT_NE(-1, expectLoc);
+    GLint successLoc = glGetUniformLocation(verifyUbo, "successOutput");
+    EXPECT_NE(-1, successLoc);
+
+    glUniform1ui(expectLoc, kInitialData[0].asUint());
+    glUniform4f(successLoc, 0, 1, 0, 1);
+
+    drawQuad(verifyUbo, essl3_shaders::PositionAttrib(), 0.5);
+    EXPECT_GL_NO_ERROR();
+
+    // Dont't verify the buffer.  This is testing GL_MAP_INVALIDATE_BUFFER_BIT while the buffer is
+    // in use by the GPU.
+
+    // Map the buffer and update it.
+    void *mappedBuffer = glMapBufferRange(GL_UNIFORM_BUFFER, 0, sizeof(kInitialData),
+                                          GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT);
+
+    memcpy(mappedBuffer, kUpdateData1.data(), sizeof(kInitialData));
+
+    glUnmapBuffer(GL_UNIFORM_BUFFER);
+    EXPECT_GL_NO_ERROR();
+
+    // Verify that the buffer has the updated value.
+    glUniform1ui(expectLoc, kUpdateData1[0].asUint());
+    glUniform4f(successLoc, 0, 0, 1, 1);
+
+    drawQuad(verifyUbo, essl3_shaders::PositionAttrib(), 0.5);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::blue);
+
+    // Update the buffer with glBufferSubData
+    glBufferSubData(GL_UNIFORM_BUFFER, 0, sizeof(kUpdateData2), kUpdateData2.data());
+    EXPECT_GL_NO_ERROR();
+
+    // Verify that the buffer has the updated value.
+    glUniform1ui(expectLoc, kUpdateData2[0].asUint());
+    glUniform4f(successLoc, 0, 1, 1, 1);
+
+    drawQuad(verifyUbo, essl3_shaders::PositionAttrib(), 0.5);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::cyan);
+}
+
 class BufferStorageTestES3 : public BufferDataTest
 {};
 

src/tests/test_utils/ANGLETest.cpp

@@ -231,6 +231,13 @@ GLColor::GLColor(GLuint colorValue) : R(0), G(0), B(0), A(0)
     memcpy(&R, &colorValue, sizeof(GLuint));
 }
 
+GLuint GLColor::asUint() const
+{
+    GLuint uint = 0;
+    memcpy(&uint, &R, sizeof(GLuint));
+    return uint;
+}
+
 testing::AssertionResult GLColor::ExpectNear(const GLColor &expected, const GLColor &err) const
 {
     testing::AssertionResult result(

src/tests/test_utils/ANGLETest.h

@@ -134,6 +134,8 @@ struct GLColor
     const GLubyte *data() const { return &R; }
     GLubyte *data() { return &R; }
 
+    GLuint asUint() const;
+
     testing::AssertionResult ExpectNear(const GLColor &expected, const GLColor &err) const;
 
     GLubyte R, G, B, A;