Vulkan: Fix non-aligned buffer offset wrt stride

  It has been observed in some cases that when copying an entire
vertex buffer for conversion using CPU, there is a possibility that
the end of the copy range falls out of the buffer bounds due to the
copy size being calculated due to the max vertex count, which comes
from GetVertexCountForRange(). If the offset for the data (e.g., set
in glVertexAttribPointer()) is not aligned with the stride, the data
range calculated from the vertex count can fall outside the bounds.

This change will limit the source data size to the buffer bounds to
avoid out-of-bound memory accesses in such cases.

* Updated CalculateMaxVertexCountForConversion() so the intended
  source data size does not exceed the source buffer limit.

* Added unit test to demonstrate that a draw using a vertex buffer
  with an offset not aligned with stride results in a correct draw
  without out-of-bound access.
  * VertexAttributeTest.BufferOffsetNonAlignedWithStride

Bug: angleproject:468923885
Change-Id: I5f2e92bd0c923173d5d1191cf189401e34508fe1
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7263864
Reviewed-by: Charlie Lao <cclao@google.com>
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>

diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
index dc3b832..f3727f6 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp

@@ -209,8 +209,8 @@
     // Initialize numVertices to 0
     *maxNumVerticesOut = 0;
 
-    unsigned srcFormatSize = srcFormat.pixelBytes;
-    unsigned dstFormatSize = dstFormat.pixelBytes;
+    uint32_t srcFormatSize = srcFormat.pixelBytes;
+    uint32_t dstFormatSize = dstFormat.pixelBytes;
 
     uint32_t srcStride = conversion->getCacheKey().stride;
     uint32_t dstStride = dstFormatSize;
@@ -218,10 +218,11 @@
     ASSERT(srcStride != 0);
     ASSERT(conversion->dirty());
 
-    // Start the range with the range from the the beginning of the buffer to the end of
-    // buffer. Then scissor it with the dirtyRange.
+    // Start the range from the beginning of the buffer to the end of the buffer. Then scissor it
+    // with the dirtyRange.
+    VkDeviceSize srcBufferSize = srcBuffer->getSize();
     size_t srcOffset  = conversion->getCacheKey().offset;
-    GLint64 srcLength = srcBuffer->getSize() - srcOffset;
+    GLint64 srcLength          = static_cast<GLint64>(srcBufferSize) - srcOffset;
 
     // The max number of vertices from binding to the end of the buffer
     size_t maxNumVertices = GetVertexCountForRange(srcLength, srcFormatSize, srcStride);
@@ -230,6 +231,16 @@
         return angle::Result::Continue;
     }
 
+    // The intended data size does not include the entire stride from the last vertex, but only the
+    // format size. The data size must be within the source buffer's limit.
+    VkDeviceSize intendedSrcDataSize =
+        static_cast<VkDeviceSize>((maxNumVertices - 1) * srcStride) + srcFormatSize;
+    if (intendedSrcDataSize > srcBufferSize)
+    {
+        maxNumVertices = static_cast<size_t>(srcBufferSize / srcStride);
+        ASSERT(maxNumVertices != 0);
+    }
+
     // Allocate buffer for results
     vk::MemoryHostVisibility hostVisible = conversion->getCacheKey().hostVisible
                                                ? vk::MemoryHostVisibility::Visible
@@ -238,7 +249,7 @@
                                                        hostVisible));
 
     // Calculate numVertices to convert
-    *maxNumVerticesOut = GetVertexCountForRange(srcLength, srcFormatSize, srcStride);
+    *maxNumVerticesOut = maxNumVertices;
 
     return angle::Result::Continue;
 }

diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 92f5c62..eece1d6 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt

@@ -60,6 +60,9 @@
 359729255 METAL : VertexAttributeTest.VertexAttribPointerCopyBufferFromInvalid* = SKIP
 359729255 WGPU : VertexAttributeTest.VertexAttribPointerCopyBufferFromInvalid* = SKIP
 
+// Using a non-zero offset non-aligned with the stride.
+468923885 WGPU : VertexAttributeTest.BufferOffsetNonAlignedWithStride/* = SKIP
+
 // Propagate buffer changes to VertexArray fail on some desktop drivers
 433331119 LINUX NVIDIA OPENGL : ValidationStateChangeTest.RebindBufferShouldPickupBufferChange/* = SKIP
 433331119 LINUX NVIDIA OPENGL : ValidationStateChangeTestES31.RebindVertexBufferShouldPickupBufferChange/* = SKIP

diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp
index 37327f4..6874c29 100644
--- a/src/tests/gl_tests/VertexAttributeTest.cpp
+++ b/src/tests/gl_tests/VertexAttributeTest.cpp

@@ -3982,6 +3982,51 @@
     EXPECT_GL_NO_ERROR();
 }
 
+// Test that when the offset for the vertex attribute pointer is not aligned with the stride, the
+// draw is successful without crashes due to out-of-bound access.
+TEST_P(VertexAttributeTest, BufferOffsetNonAlignedWithStride)
+{
+    glClearColor(0.0f, 0.0f, 1.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    constexpr char kVS[] = R"(
+attribute highp vec3 pos;
+void main() {
+    gl_Position = vec4(pos, 1.0);
+})";
+    constexpr char kFS[] = R"(
+precision highp float;
+void main() {
+    gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
+})";
+    ANGLE_GL_PROGRAM(program, kVS, kFS);
+    glBindAttribLocation(program, 0, "pos");
+    glUseProgram(program);
+    ASSERT_GL_NO_ERROR();
+
+    const std::vector<float> kVertexData = {-1.0f, -1.0f, 0.0f, -1.0f, 1.0f,  0.0f,
+                                            1.0f,  1.0f,  0.0f, -1.0f, -1.0f, 0.0f,
+                                            1.0f,  1.0f,  0.0f, 1.0f,  -1.0f, 0.0f};
+    constexpr size_t kOffset             = 1;
+    constexpr size_t kStride             = 12;
+    static_assert(kOffset % kStride != 0, "Offset must not be aligned with stride.");
+
+    constexpr size_t kBufferSize = 1 * 1024 * 1024;
+    std::vector<uint8_t> bufferData(kBufferSize, 0);
+    memcpy(bufferData.data() + kOffset, kVertexData.data(), kVertexData.size() * sizeof(float));
+
+    GLBuffer buffer;
+    glBindBuffer(GL_ARRAY_BUFFER, buffer);
+    glBufferData(GL_ARRAY_BUFFER, kBufferSize, bufferData.data(), GL_DYNAMIC_DRAW);
+
+    glEnableVertexAttribArray(0);
+    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, kStride,
+                          reinterpret_cast<const void *>(kOffset));
+
+    glDrawArrays(GL_TRIANGLES, 0, 6);
+    EXPECT_PIXEL_RECT_EQ(0, 0, getWindowWidth(), getWindowHeight(), GLColor::red);
+}
+
 // Test that default unsigned integer attribute works correctly even if there is a gap in
 // attribute locations.
 TEST_P(VertexAttributeTestES3, DefaultUIntAttribWithGap)