Commit 6c158164 by Jamie Madill Committed by Commit Bot

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 <jmadill@chromium.org> Reviewed-by: 's avatarCourtney Goeltzenleuchter <courtneygo@google.com>
parent 3c4d7ab0
...@@ -84,16 +84,18 @@ class BufferVk : public BufferImpl ...@@ -84,16 +84,18 @@ class BufferVk : public BufferImpl
const vk::BufferHelper &getBuffer() const const vk::BufferHelper &getBuffer() const
{ {
ASSERT(mBuffer && mBuffer->valid()); ASSERT(isBufferValid());
return *mBuffer; return *mBuffer;
} }
vk::BufferHelper &getBuffer() vk::BufferHelper &getBuffer()
{ {
ASSERT(mBuffer && mBuffer->valid()); ASSERT(isBufferValid());
return *mBuffer; return *mBuffer;
} }
bool isBufferValid() const { return mBuffer && mBuffer->valid(); }
angle::Result mapImpl(ContextVk *contextVk, void **mapPtr); angle::Result mapImpl(ContextVk *contextVk, void **mapPtr);
angle::Result mapRangeImpl(ContextVk *contextVk, angle::Result mapRangeImpl(ContextVk *contextVk,
VkDeviceSize offset, VkDeviceSize offset,
......
...@@ -61,10 +61,25 @@ angle::Result TransformFeedbackVk::begin(const gl::Context *context, ...@@ -61,10 +61,25 @@ angle::Result TransformFeedbackVk::begin(const gl::Context *context,
{ {
const gl::OffsetBindingPointer<gl::Buffer> &binding = mState.getIndexedBuffer(bufferIndex); const gl::OffsetBindingPointer<gl::Buffer> &binding = mState.getIndexedBuffer(bufferIndex);
ASSERT(binding.get()); ASSERT(binding.get());
mBufferHelpers[bufferIndex] = &vk::GetImpl(binding.get())->getBuffer();
BufferVk *bufferVk = vk::GetImpl(binding.get());
if (bufferVk->isBufferValid())
{
mBufferHelpers[bufferIndex] = &bufferVk->getBuffer();
mBufferOffsets[bufferIndex] = binding.getOffset();
mBufferSizes[bufferIndex] = gl::GetBoundBufferAvailableSize(binding);
}
else
{
// This can happen in error conditions.
vk::BufferHelper &nullBuffer = contextVk->getRenderer()->getNullBuffer();
mBufferHelpers[bufferIndex] = &nullBuffer;
mBufferOffsets[bufferIndex] = 0;
mBufferSizes[bufferIndex] = nullBuffer.getSize();
}
mBufferHandles[bufferIndex] = mBufferHelpers[bufferIndex]->getBuffer().getHandle(); mBufferHandles[bufferIndex] = mBufferHelpers[bufferIndex]->getBuffer().getHandle();
mBufferOffsets[bufferIndex] = binding.getOffset();
mBufferSizes[bufferIndex] = gl::GetBoundBufferAvailableSize(binding);
if (contextVk->getFeatures().supportsTransformFeedbackExtension.enabled) if (contextVk->getFeatures().supportsTransformFeedbackExtension.enabled)
{ {
......
...@@ -2152,11 +2152,22 @@ angle::Result BufferHelper::init(Context *context, ...@@ -2152,11 +2152,22 @@ angle::Result BufferHelper::init(Context *context,
(memoryPropertyFlags & (~VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT)); (memoryPropertyFlags & (~VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT));
const vk::Allocator &allocator = renderer->getAllocator(); const vk::Allocator &allocator = renderer->getAllocator();
uint32_t memoryTypeIndex = 0; bool persistentlyMapped = renderer->getFeatures().persistentlyMappedBuffers.enabled;
ANGLE_VK_TRY(context,
allocator.createBuffer(*createInfo, requiredFlags, preferredFlags, // Check that the allocation is not too large.
renderer->getFeatures().persistentlyMappedBuffers.enabled, uint32_t memoryTypeIndex = 0;
&memoryTypeIndex, &mBuffer, &mAllocation)); ANGLE_VK_TRY(context, allocator.findMemoryTypeIndexForBufferInfo(
*createInfo, requiredFlags, preferredFlags, persistentlyMapped,
&memoryTypeIndex));
VkDeviceSize heapSize =
renderer->getMemoryProperties().getHeapSizeForMemoryType(memoryTypeIndex);
ANGLE_VK_CHECK(context, createInfo->size <= heapSize, VK_ERROR_OUT_OF_DEVICE_MEMORY);
ANGLE_VK_TRY(context, allocator.createBuffer(*createInfo, requiredFlags, preferredFlags,
persistentlyMapped, &memoryTypeIndex, &mBuffer,
&mAllocation));
allocator.getMemoryTypeProperties(memoryTypeIndex, &mMemoryPropertyFlags); allocator.getMemoryTypeProperties(memoryTypeIndex, &mMemoryPropertyFlags);
mCurrentQueueFamilyIndex = renderer->getQueueFamilyIndex(); mCurrentQueueFamilyIndex = renderer->getQueueFamilyIndex();
......
...@@ -90,6 +90,22 @@ VkResult CreateBuffer(VmaAllocator allocator, ...@@ -90,6 +90,22 @@ VkResult CreateBuffer(VmaAllocator allocator,
return result; return result;
} }
VkResult FindMemoryTypeIndexForBufferInfo(VmaAllocator allocator,
const VkBufferCreateInfo *pBufferCreateInfo,
VkMemoryPropertyFlags requiredFlags,
VkMemoryPropertyFlags preferredFlags,
bool persistentlyMappedBuffers,
uint32_t *pMemoryTypeIndexOut)
{
VmaAllocationCreateInfo allocationCreateInfo = {};
allocationCreateInfo.requiredFlags = requiredFlags;
allocationCreateInfo.preferredFlags = preferredFlags;
allocationCreateInfo.flags = (persistentlyMappedBuffers) ? VMA_ALLOCATION_CREATE_MAPPED_BIT : 0;
return vmaFindMemoryTypeIndexForBufferInfo(allocator, pBufferCreateInfo, &allocationCreateInfo,
pMemoryTypeIndexOut);
}
void GetMemoryTypeProperties(VmaAllocator allocator, void GetMemoryTypeProperties(VmaAllocator allocator,
uint32_t memoryTypeIndex, uint32_t memoryTypeIndex,
VkMemoryPropertyFlags *pFlags) VkMemoryPropertyFlags *pFlags)
......
...@@ -35,6 +35,13 @@ VkResult CreateBuffer(VmaAllocator allocator, ...@@ -35,6 +35,13 @@ VkResult CreateBuffer(VmaAllocator allocator,
VkBuffer *pBuffer, VkBuffer *pBuffer,
VmaAllocation *pAllocation); VmaAllocation *pAllocation);
VkResult FindMemoryTypeIndexForBufferInfo(VmaAllocator allocator,
const VkBufferCreateInfo *pBufferCreateInfo,
VkMemoryPropertyFlags requiredFlags,
VkMemoryPropertyFlags preferredFlags,
bool persistentlyMappedBuffers,
uint32_t *pMemoryTypeIndexOut);
void GetMemoryTypeProperties(VmaAllocator allocator, void GetMemoryTypeProperties(VmaAllocator allocator,
uint32_t memoryTypeIndex, uint32_t memoryTypeIndex,
VkMemoryPropertyFlags *pFlags); VkMemoryPropertyFlags *pFlags);
......
...@@ -292,6 +292,12 @@ class MemoryProperties final : angle::NonCopyable ...@@ -292,6 +292,12 @@ class MemoryProperties final : angle::NonCopyable
uint32_t *indexOut) const; uint32_t *indexOut) const;
void destroy(); void destroy();
VkDeviceSize getHeapSizeForMemoryType(uint32_t memoryType) const
{
uint32_t heapIndex = mMemoryProperties.memoryTypes[memoryType].heapIndex;
return mMemoryProperties.memoryHeaps[heapIndex].size;
}
private: private:
VkPhysicalDeviceMemoryProperties mMemoryProperties; VkPhysicalDeviceMemoryProperties mMemoryProperties;
}; };
......
...@@ -476,6 +476,11 @@ class Allocator : public WrappedObject<Allocator, VmaAllocator> ...@@ -476,6 +476,11 @@ class Allocator : public WrappedObject<Allocator, VmaAllocator>
Allocation *allocationOut) const; Allocation *allocationOut) const;
void getMemoryTypeProperties(uint32_t memoryTypeIndex, VkMemoryPropertyFlags *flagsOut) const; void getMemoryTypeProperties(uint32_t memoryTypeIndex, VkMemoryPropertyFlags *flagsOut) const;
VkResult findMemoryTypeIndexForBufferInfo(const VkBufferCreateInfo &bufferCreateInfo,
VkMemoryPropertyFlags requiredFlags,
VkMemoryPropertyFlags preferredFlags,
bool persistentlyMappedBuffers,
uint32_t *memoryTypeIndexOut) const;
}; };
class Allocation final : public WrappedObject<Allocation, VmaAllocation> class Allocation final : public WrappedObject<Allocation, VmaAllocation>
...@@ -1401,6 +1406,19 @@ ANGLE_INLINE void Allocator::getMemoryTypeProperties(uint32_t memoryTypeIndex, ...@@ -1401,6 +1406,19 @@ ANGLE_INLINE void Allocator::getMemoryTypeProperties(uint32_t memoryTypeIndex,
vma::GetMemoryTypeProperties(mHandle, memoryTypeIndex, flagsOut); vma::GetMemoryTypeProperties(mHandle, memoryTypeIndex, flagsOut);
} }
ANGLE_INLINE VkResult
Allocator::findMemoryTypeIndexForBufferInfo(const VkBufferCreateInfo &bufferCreateInfo,
VkMemoryPropertyFlags requiredFlags,
VkMemoryPropertyFlags preferredFlags,
bool persistentlyMappedBuffers,
uint32_t *memoryTypeIndexOut) const
{
ASSERT(valid());
return vma::FindMemoryTypeIndexForBufferInfo(mHandle, &bufferCreateInfo, requiredFlags,
preferredFlags, persistentlyMappedBuffers,
memoryTypeIndexOut);
}
// Allocation implementation. // Allocation implementation.
ANGLE_INLINE void Allocation::destroy(const Allocator &allocator) ANGLE_INLINE void Allocation::destroy(const Allocator &allocator)
{ {
......
...@@ -219,7 +219,6 @@ TEST_P(TransformFeedbackTest, RecordAndDraw) ...@@ -219,7 +219,6 @@ TEST_P(TransformFeedbackTest, RecordAndDraw)
const GLfloat vertices[] = { const GLfloat vertices[] = {
-1.0f, 1.0f, 0.5f, -1.0f, -1.0f, 0.5f, 1.0f, -1.0f, 0.5f, -1.0f, 1.0f, 0.5f, -1.0f, -1.0f, 0.5f, 1.0f, -1.0f, 0.5f,
-1.0f, 1.0f, 0.5f, 1.0f, -1.0f, 0.5f, 1.0f, 1.0f, 0.5f, -1.0f, 1.0f, 0.5f, 1.0f, -1.0f, 0.5f, 1.0f, 1.0f, 0.5f,
}; };
...@@ -1671,6 +1670,47 @@ TEST_P(TransformFeedbackTest, EndWithDifferentProgramContextSwitch) ...@@ -1671,6 +1670,47 @@ TEST_P(TransformFeedbackTest, EndWithDifferentProgramContextSwitch)
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR();
} }
// Test an out of memory event.
TEST_P(TransformFeedbackTest, BufferOutOfMemory)
{
// The GL back-end throws an internal error that we can't deal with in this test.
ANGLE_SKIP_TEST_IF(IsOpenGL());
glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
glClear(GL_COLOR_BUFFER_BIT);
// Set the program's transform feedback varyings (just gl_Position)
std::vector<std::string> tfVaryings;
tfVaryings.push_back("gl_Position");
compileDefaultProgram(tfVaryings, GL_INTERLEAVED_ATTRIBS);
GLint positionLocation = glGetAttribLocation(mProgram, essl1_shaders::PositionAttrib());
const GLfloat vertices[] = {-1.0f, -0.5f, 0.0f, 0.5f, 1.0f};
glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, vertices);
glEnableVertexAttribArray(positionLocation);
// Draw normally.
glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
glUseProgram(mProgram);
glBeginTransformFeedback(GL_POINTS);
glDrawArrays(GL_POINTS, 0, 5);
glEndTransformFeedback();
ASSERT_GL_NO_ERROR();
// Attempt to generate OOM and begin XFB.
constexpr GLsizeiptr kLargeSize = std::numeric_limits<GLsizeiptr>::max();
glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, kLargeSize, nullptr, GL_STATIC_DRAW);
// It's not spec guaranteed to return OOM here.
GLenum err = glGetError();
EXPECT_TRUE(err == GL_NO_ERROR || err == GL_OUT_OF_MEMORY);
glBeginTransformFeedback(GL_POINTS);
glDrawArrays(GL_POINTS, 0, 5);
glEndTransformFeedback();
}
// Use this to select which configurations (e.g. which renderer, which GLES major version) these // Use this to select which configurations (e.g. which renderer, which GLES major version) these
// tests should be run against. // tests should be run against.
ANGLE_INSTANTIATE_TEST_ES3(TransformFeedbackTest); ANGLE_INSTANTIATE_TEST_ES3(TransformFeedbackTest);
......
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