Skip to content

Commit 6c15816

Browse files
null77Commit Bot
null77
authored and
Commit Bot
committed
Vulkan: Fix XFB invalid accesses in buffer OOM.
This uses the "null" buffer in the Renderer to bind an empty buffer handle so ANGLE can maintain a consistent state. Bug: chromium:1086532 Change-Id: I1912a1d1cb64433a285fcfced80a675619690a0b Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2219140 Commit-Queue: Jamie Madill <[email protected]> Reviewed-by: Courtney Goeltzenleuchter <[email protected]>
1 parent 3c4d7ab commit 6c15816

File tree

8 files changed

+126
-11
lines changed

8 files changed

+126
-11
lines changed

‎src/libANGLE/renderer/vulkan/BufferVk.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,18 @@ class BufferVk : public BufferImpl
8484

8585
const vk::BufferHelper &getBuffer() const
8686
{
87-
ASSERT(mBuffer && mBuffer->valid());
87+
ASSERT(isBufferValid());
8888
return *mBuffer;
8989
}
9090

9191
vk::BufferHelper &getBuffer()
9292
{
93-
ASSERT(mBuffer && mBuffer->valid());
93+
ASSERT(isBufferValid());
9494
return *mBuffer;
9595
}
9696

97+
bool isBufferValid() const { return mBuffer && mBuffer->valid(); }
98+
9799
angle::Result mapImpl(ContextVk *contextVk, void **mapPtr);
98100
angle::Result mapRangeImpl(ContextVk *contextVk,
99101
VkDeviceSize offset,

‎src/libANGLE/renderer/vulkan/TransformFeedbackVk.cpp‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,25 @@ angle::Result TransformFeedbackVk::begin(const gl::Context *context,
6161
{
6262
const gl::OffsetBindingPointer<gl::Buffer> &binding = mState.getIndexedBuffer(bufferIndex);
6363
ASSERT(binding.get());
64-
mBufferHelpers[bufferIndex] = &vk::GetImpl(binding.get())->getBuffer();
64+
65+
BufferVk *bufferVk = vk::GetImpl(binding.get());
66+
67+
if (bufferVk->isBufferValid())
68+
{
69+
mBufferHelpers[bufferIndex] = &bufferVk->getBuffer();
70+
mBufferOffsets[bufferIndex] = binding.getOffset();
71+
mBufferSizes[bufferIndex] = gl::GetBoundBufferAvailableSize(binding);
72+
}
73+
else
74+
{
75+
// This can happen in error conditions.
76+
vk::BufferHelper &nullBuffer = contextVk->getRenderer()->getNullBuffer();
77+
mBufferHelpers[bufferIndex] = &nullBuffer;
78+
mBufferOffsets[bufferIndex] = 0;
79+
mBufferSizes[bufferIndex] = nullBuffer.getSize();
80+
}
81+
6582
mBufferHandles[bufferIndex] = mBufferHelpers[bufferIndex]->getBuffer().getHandle();
66-
mBufferOffsets[bufferIndex] = binding.getOffset();
67-
mBufferSizes[bufferIndex] = gl::GetBoundBufferAvailableSize(binding);
6883

6984
if (contextVk->getFeatures().supportsTransformFeedbackExtension.enabled)
7085
{

‎src/libANGLE/renderer/vulkan/vk_helpers.cpp‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2152,11 +2152,22 @@ angle::Result BufferHelper::init(Context *context,
21522152
(memoryPropertyFlags & (~VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT));
21532153

21542154
const vk::Allocator &allocator = renderer->getAllocator();
2155-
uint32_t memoryTypeIndex = 0;
2156-
ANGLE_VK_TRY(context,
2157-
allocator.createBuffer(*createInfo, requiredFlags, preferredFlags,
2158-
renderer->getFeatures().persistentlyMappedBuffers.enabled,
2159-
&memoryTypeIndex, &mBuffer, &mAllocation));
2155+
bool persistentlyMapped = renderer->getFeatures().persistentlyMappedBuffers.enabled;
2156+
2157+
// Check that the allocation is not too large.
2158+
uint32_t memoryTypeIndex = 0;
2159+
ANGLE_VK_TRY(context, allocator.findMemoryTypeIndexForBufferInfo(
2160+
*createInfo, requiredFlags, preferredFlags, persistentlyMapped,
2161+
&memoryTypeIndex));
2162+
2163+
VkDeviceSize heapSize =
2164+
renderer->getMemoryProperties().getHeapSizeForMemoryType(memoryTypeIndex);
2165+
2166+
ANGLE_VK_CHECK(context, createInfo->size <= heapSize, VK_ERROR_OUT_OF_DEVICE_MEMORY);
2167+
2168+
ANGLE_VK_TRY(context, allocator.createBuffer(*createInfo, requiredFlags, preferredFlags,
2169+
persistentlyMapped, &memoryTypeIndex, &mBuffer,
2170+
&mAllocation));
21602171

21612172
allocator.getMemoryTypeProperties(memoryTypeIndex, &mMemoryPropertyFlags);
21622173
mCurrentQueueFamilyIndex = renderer->getQueueFamilyIndex();

‎src/libANGLE/renderer/vulkan/vk_mem_alloc_wrapper.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,22 @@ VkResult CreateBuffer(VmaAllocator allocator,
9090
return result;
9191
}
9292

93+
VkResult FindMemoryTypeIndexForBufferInfo(VmaAllocator allocator,
94+
const VkBufferCreateInfo *pBufferCreateInfo,
95+
VkMemoryPropertyFlags requiredFlags,
96+
VkMemoryPropertyFlags preferredFlags,
97+
bool persistentlyMappedBuffers,
98+
uint32_t *pMemoryTypeIndexOut)
99+
{
100+
VmaAllocationCreateInfo allocationCreateInfo = {};
101+
allocationCreateInfo.requiredFlags = requiredFlags;
102+
allocationCreateInfo.preferredFlags = preferredFlags;
103+
allocationCreateInfo.flags = (persistentlyMappedBuffers) ? VMA_ALLOCATION_CREATE_MAPPED_BIT : 0;
104+
105+
return vmaFindMemoryTypeIndexForBufferInfo(allocator, pBufferCreateInfo, &allocationCreateInfo,
106+
pMemoryTypeIndexOut);
107+
}
108+
93109
void GetMemoryTypeProperties(VmaAllocator allocator,
94110
uint32_t memoryTypeIndex,
95111
VkMemoryPropertyFlags *pFlags)

‎src/libANGLE/renderer/vulkan/vk_mem_alloc_wrapper.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ VkResult CreateBuffer(VmaAllocator allocator,
3535
VkBuffer *pBuffer,
3636
VmaAllocation *pAllocation);
3737

38+
VkResult FindMemoryTypeIndexForBufferInfo(VmaAllocator allocator,
39+
const VkBufferCreateInfo *pBufferCreateInfo,
40+
VkMemoryPropertyFlags requiredFlags,
41+
VkMemoryPropertyFlags preferredFlags,
42+
bool persistentlyMappedBuffers,
43+
uint32_t *pMemoryTypeIndexOut);
44+
3845
void GetMemoryTypeProperties(VmaAllocator allocator,
3946
uint32_t memoryTypeIndex,
4047
VkMemoryPropertyFlags *pFlags);

‎src/libANGLE/renderer/vulkan/vk_utils.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,12 @@ class MemoryProperties final : angle::NonCopyable
292292
uint32_t *indexOut) const;
293293
void destroy();
294294

295+
VkDeviceSize getHeapSizeForMemoryType(uint32_t memoryType) const
296+
{
297+
uint32_t heapIndex = mMemoryProperties.memoryTypes[memoryType].heapIndex;
298+
return mMemoryProperties.memoryHeaps[heapIndex].size;
299+
}
300+
295301
private:
296302
VkPhysicalDeviceMemoryProperties mMemoryProperties;
297303
};

‎src/libANGLE/renderer/vulkan/vk_wrapper.h‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,11 @@ class Allocator : public WrappedObject<Allocator, VmaAllocator>
476476
Allocation *allocationOut) const;
477477

478478
void getMemoryTypeProperties(uint32_t memoryTypeIndex, VkMemoryPropertyFlags *flagsOut) const;
479+
VkResult findMemoryTypeIndexForBufferInfo(const VkBufferCreateInfo &bufferCreateInfo,
480+
VkMemoryPropertyFlags requiredFlags,
481+
VkMemoryPropertyFlags preferredFlags,
482+
bool persistentlyMappedBuffers,
483+
uint32_t *memoryTypeIndexOut) const;
479484
};
480485

481486
class Allocation final : public WrappedObject<Allocation, VmaAllocation>
@@ -1401,6 +1406,19 @@ ANGLE_INLINE void Allocator::getMemoryTypeProperties(uint32_t memoryTypeIndex,
14011406
vma::GetMemoryTypeProperties(mHandle, memoryTypeIndex, flagsOut);
14021407
}
14031408

1409+
ANGLE_INLINE VkResult
1410+
Allocator::findMemoryTypeIndexForBufferInfo(const VkBufferCreateInfo &bufferCreateInfo,
1411+
VkMemoryPropertyFlags requiredFlags,
1412+
VkMemoryPropertyFlags preferredFlags,
1413+
bool persistentlyMappedBuffers,
1414+
uint32_t *memoryTypeIndexOut) const
1415+
{
1416+
ASSERT(valid());
1417+
return vma::FindMemoryTypeIndexForBufferInfo(mHandle, &bufferCreateInfo, requiredFlags,
1418+
preferredFlags, persistentlyMappedBuffers,
1419+
memoryTypeIndexOut);
1420+
}
1421+
14041422
// Allocation implementation.
14051423
ANGLE_INLINE void Allocation::destroy(const Allocator &allocator)
14061424
{

‎src/tests/gl_tests/TransformFeedbackTest.cpp‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ TEST_P(TransformFeedbackTest, RecordAndDraw)
219219

220220
const GLfloat vertices[] = {
221221
-1.0f, 1.0f, 0.5f, -1.0f, -1.0f, 0.5f, 1.0f, -1.0f, 0.5f,
222-
223222
-1.0f, 1.0f, 0.5f, 1.0f, -1.0f, 0.5f, 1.0f, 1.0f, 0.5f,
224223
};
225224

@@ -1671,6 +1670,47 @@ TEST_P(TransformFeedbackTest, EndWithDifferentProgramContextSwitch)
16711670
ASSERT_GL_NO_ERROR();
16721671
}
16731672

1673+
// Test an out of memory event.
1674+
TEST_P(TransformFeedbackTest, BufferOutOfMemory)
1675+
{
1676+
// The GL back-end throws an internal error that we can't deal with in this test.
1677+
ANGLE_SKIP_TEST_IF(IsOpenGL());
1678+
1679+
glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
1680+
glClear(GL_COLOR_BUFFER_BIT);
1681+
1682+
// Set the program's transform feedback varyings (just gl_Position)
1683+
std::vector<std::string> tfVaryings;
1684+
tfVaryings.push_back("gl_Position");
1685+
compileDefaultProgram(tfVaryings, GL_INTERLEAVED_ATTRIBS);
1686+
1687+
GLint positionLocation = glGetAttribLocation(mProgram, essl1_shaders::PositionAttrib());
1688+
const GLfloat vertices[] = {-1.0f, -0.5f, 0.0f, 0.5f, 1.0f};
1689+
1690+
glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, vertices);
1691+
glEnableVertexAttribArray(positionLocation);
1692+
1693+
// Draw normally.
1694+
glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
1695+
glUseProgram(mProgram);
1696+
glBeginTransformFeedback(GL_POINTS);
1697+
glDrawArrays(GL_POINTS, 0, 5);
1698+
glEndTransformFeedback();
1699+
ASSERT_GL_NO_ERROR();
1700+
1701+
// Attempt to generate OOM and begin XFB.
1702+
constexpr GLsizeiptr kLargeSize = std::numeric_limits<GLsizeiptr>::max();
1703+
glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, kLargeSize, nullptr, GL_STATIC_DRAW);
1704+
1705+
// It's not spec guaranteed to return OOM here.
1706+
GLenum err = glGetError();
1707+
EXPECT_TRUE(err == GL_NO_ERROR || err == GL_OUT_OF_MEMORY);
1708+
1709+
glBeginTransformFeedback(GL_POINTS);
1710+
glDrawArrays(GL_POINTS, 0, 5);
1711+
glEndTransformFeedback();
1712+
}
1713+
16741714
// Use this to select which configurations (e.g. which renderer, which GLES major version) these
16751715
// tests should be run against.
16761716
ANGLE_INSTANTIATE_TEST_ES3(TransformFeedbackTest);

0 commit comments

Comments
 (0)