D3D11: Fix out-of-range access with robust access.
When using a vertex buffer with DYNAMIC usage, with robust buffer
access enabled, we would sometimes read out-of-bounds when using very
large values for the index range. An unchecked signed addition would
overflow and lead to reading a negative offset.
Fix this problem by keeping the value size_t whenever possible. Also do
clamped casts when converting to a smaller values.
Also adds a regression test.
Bug: chromium:842028
Change-Id: Ie630ac857c6acfc0bace849a03eebfbaa2fbe89a
Reviewed-on: https://chromium-review.googlesource.com/1055928
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/src/libANGLE/params.cpp b/src/libANGLE/params.cpp
index a3e52c7..a77435c 100644
--- a/src/libANGLE/params.cpp
+++ b/src/libANGLE/params.cpp
@@ -100,7 +100,7 @@
return mFirstVertex;
}
-GLsizei DrawCallParams::vertexCount() const
+size_t DrawCallParams::vertexCount() const
{
ASSERT(!isDrawElements() || mIndexRange.valid());
return mVertexCount;
@@ -179,7 +179,7 @@
const IndexRange &indexRange = mIndexRange.value();
mFirstVertex = mBaseVertex + static_cast<GLint>(indexRange.start);
- mVertexCount = static_cast<GLsizei>(indexRange.vertexCount());
+ mVertexCount = indexRange.vertexCount();
return NoError();
}
diff --git a/src/libANGLE/params.h b/src/libANGLE/params.h
index 0544320..3fa023e 100644
--- a/src/libANGLE/params.h
+++ b/src/libANGLE/params.h
@@ -98,7 +98,7 @@
// This value is the sum of 'baseVertex' and the first indexed vertex for DrawElements calls.
GLint firstVertex() const;
- GLsizei vertexCount() const;
+ size_t vertexCount() const;
GLsizei indexCount() const;
GLint baseVertex() const;
GLenum type() const;
@@ -113,6 +113,9 @@
// ensureIndexRangeResolved must be called first.
const IndexRange &getIndexRange() const;
+ template <typename T>
+ T getClampedVertexCount() const;
+
template <EntryPoint EP, typename... ArgsT>
static void Factory(DrawCallParams *objBuffer, ArgsT... args);
@@ -122,7 +125,7 @@
GLenum mMode;
mutable Optional<IndexRange> mIndexRange;
mutable GLint mFirstVertex;
- mutable GLsizei mVertexCount;
+ mutable size_t mVertexCount;
GLint mIndexCount;
GLint mBaseVertex;
GLenum mType;
@@ -131,6 +134,13 @@
const void *mIndirect;
};
+template <typename T>
+T DrawCallParams::getClampedVertexCount() const
+{
+ constexpr size_t kMax = static_cast<size_t>(std::numeric_limits<T>::max());
+ return static_cast<T>(mVertexCount > kMax ? kMax : mVertexCount);
+}
+
// Entry point funcs essentially re-map different entry point parameter arrays into
// the format the parameter type class expects. For example, for HasIndexRange, for the
// various indexed draw calls, they drop parameters that aren't useful and re-arrange
diff --git a/src/libANGLE/renderer/d3d/BufferD3D.cpp b/src/libANGLE/renderer/d3d/BufferD3D.cpp
index 7769ab2..3df7e4f 100644
--- a/src/libANGLE/renderer/d3d/BufferD3D.cpp
+++ b/src/libANGLE/renderer/d3d/BufferD3D.cpp
@@ -160,10 +160,11 @@
}
// Creates static buffers if sufficient used data has been left unmodified
-void BufferD3D::promoteStaticUsage(const gl::Context *context, int dataSize)
+void BufferD3D::promoteStaticUsage(const gl::Context *context, size_t dataSize)
{
if (mUsage == D3DBufferUsage::DYNAMIC)
{
+ // Note: This is not a safe math operation. 'dataSize' can come from the app.
mUnmodifiedDataUse += dataSize;
if (mUnmodifiedDataUse > 3 * getSize())
diff --git a/src/libANGLE/renderer/d3d/BufferD3D.h b/src/libANGLE/renderer/d3d/BufferD3D.h
index 6015374..2f0ff48 100644
--- a/src/libANGLE/renderer/d3d/BufferD3D.h
+++ b/src/libANGLE/renderer/d3d/BufferD3D.h
@@ -55,7 +55,7 @@
virtual void initializeStaticData(const gl::Context *context);
virtual void invalidateStaticData(const gl::Context *context);
- void promoteStaticUsage(const gl::Context *context, int dataSize);
+ void promoteStaticUsage(const gl::Context *context, size_t dataSize);
gl::Error getIndexRange(const gl::Context *context,
GLenum type,
@@ -80,7 +80,7 @@
StaticIndexBufferInterface *mStaticIndexBuffer;
unsigned int mStaticBufferCacheTotalSize;
unsigned int mStaticVertexBufferOutOfDate;
- unsigned int mUnmodifiedDataUse;
+ size_t mUnmodifiedDataUse;
D3DBufferUsage mUsage;
};
diff --git a/src/libANGLE/renderer/d3d/RendererD3D.h b/src/libANGLE/renderer/d3d/RendererD3D.h
index 1ed48ec..86ea932 100644
--- a/src/libANGLE/renderer/d3d/RendererD3D.h
+++ b/src/libANGLE/renderer/d3d/RendererD3D.h
@@ -95,7 +95,7 @@
virtual gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(
const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const = 0;
};
diff --git a/src/libANGLE/renderer/d3d/VertexBuffer.cpp b/src/libANGLE/renderer/d3d/VertexBuffer.cpp
index 7c2d5ae..cb6d26f 100644
--- a/src/libANGLE/renderer/d3d/VertexBuffer.cpp
+++ b/src/libANGLE/renderer/d3d/VertexBuffer.cpp
@@ -155,7 +155,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int *outStreamOffset,
const uint8_t *sourceData)
@@ -190,7 +190,7 @@
gl::Error StreamingVertexBufferInterface::reserveVertexSpace(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances)
{
unsigned int requiredSpace = 0;
diff --git a/src/libANGLE/renderer/d3d/VertexBuffer.h b/src/libANGLE/renderer/d3d/VertexBuffer.h
index df8085d..9ee39ff 100644
--- a/src/libANGLE/renderer/d3d/VertexBuffer.h
+++ b/src/libANGLE/renderer/d3d/VertexBuffer.h
@@ -45,7 +45,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData) = 0;
@@ -110,14 +110,14 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int *outStreamOffset,
const uint8_t *sourceData);
gl::Error reserveVertexSpace(const gl::VertexAttribute &attribute,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances);
private:
diff --git a/src/libANGLE/renderer/d3d/VertexDataManager.cpp b/src/libANGLE/renderer/d3d/VertexDataManager.cpp
index f20386b..183c895 100644
--- a/src/libANGLE/renderer/d3d/VertexDataManager.cpp
+++ b/src/libANGLE/renderer/d3d/VertexDataManager.cpp
@@ -392,7 +392,7 @@
std::vector<TranslatedAttribute> *translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances)
{
// Instantiating this class will ensure the streaming buffer is never left mapped.
@@ -434,7 +434,7 @@
const gl::Context *context,
const std::vector<TranslatedAttribute> &translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
- GLsizei count)
+ size_t count)
{
for (auto attribIndex : dynamicAttribsMask)
{
@@ -445,16 +445,17 @@
gl::Buffer *buffer = binding.getBuffer().get();
if (buffer)
{
+ // Note: this multiplication can overflow. It should not be a security problem.
BufferD3D *bufferD3D = GetImplAs<BufferD3D>(buffer);
size_t typeSize = ComputeVertexAttributeTypeSize(*dynamicAttrib.attribute);
- bufferD3D->promoteStaticUsage(context, count * static_cast<int>(typeSize));
+ bufferD3D->promoteStaticUsage(context, count * typeSize);
}
}
}
gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances) const
{
ASSERT(translatedAttrib.attribute && translatedAttrib.binding);
@@ -467,8 +468,8 @@
BufferD3D *bufferD3D = buffer ? GetImplAs<BufferD3D>(buffer) : nullptr;
ASSERT(!bufferD3D || bufferD3D->getStaticVertexBuffer(attrib, binding) == nullptr);
- size_t totalCount = gl::ComputeVertexBindingElementCount(
- binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances));
+ size_t totalCount = gl::ComputeVertexBindingElementCount(binding.getDivisor(), count,
+ static_cast<size_t>(instances));
// TODO(jiajia.qin@intel.com): force the index buffer to clamp any out of range indices instead
// of invalid operation here.
if (bufferD3D)
@@ -486,15 +487,14 @@
return gl::InvalidOperation() << "Vertex buffer is not big enough for the draw call.";
}
}
- return mStreamingBuffer->reserveVertexSpace(attrib, binding, static_cast<GLsizei>(totalCount),
- instances);
+ return mStreamingBuffer->reserveVertexSpace(attrib, binding, totalCount, instances);
}
gl::Error VertexDataManager::storeDynamicAttrib(const gl::Context *context,
TranslatedAttribute *translated,
GLint start,
- GLsizei count,
- GLsizei instances)
+ size_t count,
+ GLsizei instances) const
{
ASSERT(translated->attribute && translated->binding);
const auto &attrib = *translated->attribute;
@@ -529,8 +529,8 @@
translated->storage = nullptr;
ANGLE_TRY_RESULT(mFactory->getVertexSpaceRequired(attrib, binding, 1, 0), translated->stride);
- size_t totalCount = gl::ComputeVertexBindingElementCount(
- binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances));
+ size_t totalCount = gl::ComputeVertexBindingElementCount(binding.getDivisor(), count,
+ static_cast<size_t>(instances));
ANGLE_TRY(mStreamingBuffer->storeDynamicAttribute(
attrib, binding, translated->currentValueType, firstVertexIndex,
diff --git a/src/libANGLE/renderer/d3d/VertexDataManager.h b/src/libANGLE/renderer/d3d/VertexDataManager.h
index 694366d..2fb4fe6 100644
--- a/src/libANGLE/renderer/d3d/VertexDataManager.h
+++ b/src/libANGLE/renderer/d3d/VertexDataManager.h
@@ -105,14 +105,14 @@
std::vector<TranslatedAttribute> *translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances);
// Promote static usage of dynamic buffers.
static void PromoteDynamicAttribs(const gl::Context *context,
const std::vector<TranslatedAttribute> &translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
- GLsizei count);
+ size_t count);
gl::Error storeCurrentValue(const gl::VertexAttribCurrentValueData ¤tValue,
TranslatedAttribute *translated,
@@ -130,15 +130,15 @@
};
gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
- GLsizei count,
GLint start,
+ size_t count,
GLsizei instances) const;
gl::Error storeDynamicAttrib(const gl::Context *context,
TranslatedAttribute *translated,
GLint start,
- GLsizei count,
- GLsizei instances);
+ size_t count,
+ GLsizei instances) const;
BufferFactoryD3D *const mFactory;
diff --git a/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp b/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp
index 8dd5ad8..f8f8865 100644
--- a/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp
@@ -246,11 +246,12 @@
// As per the spec for ANGLE_instanced_arrays, not all attributes can be instanced
// simultaneously, so a non-instanced element must exist.
- GLsizei numIndicesPerInstance = 0;
+ UINT numIndicesPerInstance = 0;
if (drawCallParams.instances() > 0)
{
// This requires that the index range is resolved.
- numIndicesPerInstance = drawCallParams.vertexCount();
+ // Note: Vertex indexes can be arbitrarily large.
+ numIndicesPerInstance = drawCallParams.getClampedVertexCount<UINT>();
}
for (size_t elementIndex = 0; elementIndex < inputElementCount; ++elementIndex)
diff --git a/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp b/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp
index 2c48668..3d0ad0c 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp
@@ -1403,7 +1403,7 @@
gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallParams ¶ms)
{
- if (params.vertexCount() < mStateManager.getCurrentMinimumDrawCount())
+ if (params.vertexCount() < static_cast<size_t>(mStateManager.getCurrentMinimumDrawCount()))
{
return gl::NoError();
}
@@ -1419,6 +1419,9 @@
GLsizei adjustedInstanceCount = GetAdjustedInstanceCount(program, params.instances());
ProgramD3D *programD3D = GetImplAs<ProgramD3D>(program);
+ // Note: vertex indexes can be arbitrarily large.
+ UINT clampedVertexCount = params.getClampedVertexCount<UINT>();
+
if (programD3D->usesGeometryShader(params.mode()) &&
glState.isTransformFeedbackActiveUnpaused())
{
@@ -1430,11 +1433,11 @@
if (adjustedInstanceCount > 0)
{
- mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0);
+ mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
}
else
{
- mDeviceContext->Draw(params.vertexCount(), 0);
+ mDeviceContext->Draw(clampedVertexCount, 0);
}
rx::ShaderExecutableD3D *pixelExe = nullptr;
@@ -1458,24 +1461,24 @@
if (adjustedInstanceCount > 0)
{
- mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0);
+ mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
}
else
{
- mDeviceContext->Draw(params.vertexCount(), 0);
+ mDeviceContext->Draw(clampedVertexCount, 0);
}
return gl::NoError();
}
if (params.mode() == GL_LINE_LOOP)
{
- return drawLineLoop(context, params.vertexCount(), GL_NONE, nullptr, 0,
+ return drawLineLoop(context, clampedVertexCount, GL_NONE, nullptr, 0,
adjustedInstanceCount);
}
if (params.mode() == GL_TRIANGLE_FAN)
{
- return drawTriangleFan(context, params.vertexCount(), GL_NONE, nullptr, 0,
+ return drawTriangleFan(context, clampedVertexCount, GL_NONE, nullptr, 0,
adjustedInstanceCount);
}
@@ -1486,11 +1489,11 @@
{
if (adjustedInstanceCount == 0)
{
- mDeviceContext->Draw(params.vertexCount(), 0);
+ mDeviceContext->Draw(clampedVertexCount, 0);
}
else
{
- mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0);
+ mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
}
return gl::NoError();
}
@@ -1503,7 +1506,7 @@
// D3D_PRIMITIVE_TOPOLOGY_TRIANGLELIST and DrawIndexedInstanced is called instead.
if (adjustedInstanceCount == 0)
{
- mDeviceContext->DrawIndexedInstanced(6, params.vertexCount(), 0, 0, 0);
+ mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
return gl::NoError();
}
@@ -1516,7 +1519,7 @@
{
ANGLE_TRY(
mStateManager.updateVertexOffsetsForPointSpritesEmulation(params.baseVertex(), i));
- mDeviceContext->DrawIndexedInstanced(6, params.vertexCount(), 0, 0, 0);
+ mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
}
// This required by updateVertexOffsets... above but is outside of the loop for speed.
@@ -1595,13 +1598,13 @@
// efficent code path. Instanced rendering of emulated pointsprites requires a loop to draw each
// batch of points. An offset into the instanced data buffer is calculated and applied on each
// iteration to ensure all instances are rendered correctly.
- GLsizei elementsToRender = params.vertexCount();
+ UINT clampedVertexCount = params.getClampedVertexCount<UINT>();
// Each instance being rendered requires the inputlayout cache to reapply buffers and offsets.
for (GLsizei i = 0; i < params.instances(); i++)
{
ANGLE_TRY(mStateManager.updateVertexOffsetsForPointSpritesEmulation(startVertex, i));
- mDeviceContext->DrawIndexedInstanced(6, elementsToRender, 0, 0, 0);
+ mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
}
mStateManager.invalidateVertexBuffer();
return gl::NoError();
@@ -1653,7 +1656,7 @@
}
gl::Error Renderer11::drawLineLoop(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indexPointer,
int baseVertex,
@@ -1690,8 +1693,6 @@
}
// Checked by Renderer11::applyPrimitiveType
- ASSERT(count >= 0);
-
if (static_cast<unsigned int>(count) + 1 >
(std::numeric_limits<unsigned int>::max() / sizeof(unsigned int)))
{
@@ -1737,7 +1738,7 @@
}
gl::Error Renderer11::drawTriangleFan(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indices,
int baseVertex,
@@ -3570,7 +3571,7 @@
gl::ErrorOrResult<unsigned int> Renderer11::getVertexSpaceRequired(
const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const
{
if (!attrib.enabled)
diff --git a/src/libANGLE/renderer/d3d/d3d11/Renderer11.h b/src/libANGLE/renderer/d3d/d3d11/Renderer11.h
index 4f1a3f4..3e6c4e4 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Renderer11.h
+++ b/src/libANGLE/renderer/d3d/d3d11/Renderer11.h
@@ -342,7 +342,7 @@
// function.
gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const override;
gl::Error readFromAttachment(const gl::Context *context,
@@ -460,13 +460,13 @@
angle::WorkaroundsD3D generateWorkarounds() const override;
gl::Error drawLineLoop(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indices,
int baseVertex,
int instances);
gl::Error drawTriangleFan(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indices,
int baseVertex,
diff --git a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp
index 69e5987..3649cbe 100644
--- a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp
@@ -96,7 +96,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData)
diff --git a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h
index ab619ae..7778c31 100644
--- a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h
+++ b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h
@@ -31,7 +31,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData) override;
diff --git a/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp b/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp
index 1abd244..316de46 100644
--- a/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp
+++ b/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp
@@ -3007,7 +3007,7 @@
gl::ErrorOrResult<unsigned int> Renderer9::getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const
{
if (!attrib.enabled)
diff --git a/src/libANGLE/renderer/d3d/d3d9/Renderer9.h b/src/libANGLE/renderer/d3d/d3d9/Renderer9.h
index 239bfd0..0d5f450 100644
--- a/src/libANGLE/renderer/d3d/d3d9/Renderer9.h
+++ b/src/libANGLE/renderer/d3d/d3d9/Renderer9.h
@@ -346,7 +346,7 @@
// function.
gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const override;
gl::Error copyToRenderTarget(IDirect3DSurface9 *dest,
diff --git a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp
index c0b80a8..51af37a 100644
--- a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp
+++ b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp
@@ -61,7 +61,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData)
@@ -71,8 +71,8 @@
return gl::OutOfMemory() << "Internal vertex buffer is not initialized.";
}
- int inputStride = static_cast<int>(gl::ComputeVertexAttributeStride(attrib, binding));
- int elementSize = static_cast<int>(gl::ComputeVertexAttributeTypeSize(attrib));
+ size_t inputStride = gl::ComputeVertexAttributeStride(attrib, binding);
+ size_t elementSize = gl::ComputeVertexAttributeTypeSize(attrib);
DWORD lockFlags = mDynamicUsage ? D3DLOCK_NOOVERWRITE : 0;
@@ -105,7 +105,7 @@
if (!needsConversion && inputStride == elementSize)
{
- size_t copySize = static_cast<size_t>(count) * static_cast<size_t>(inputStride);
+ size_t copySize = count * inputStride;
memcpy(mapPtr, input, copySize);
}
else
diff --git a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h
index 983616f..90defb3 100644
--- a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h
+++ b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h
@@ -28,7 +28,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData) override;
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
index 5972339..16af443 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
@@ -357,14 +357,17 @@
ANGLE_TRY(onDraw(context, renderer, drawCallParams, drawNode, newCommandBuffer));
+ // Note: Vertex indexes can be arbitrarily large.
+ uint32_t clampedVertexCount = drawCallParams.getClampedVertexCount<uint32_t>();
+
if (drawCallParams.mode() != GL_LINE_LOOP)
{
- commandBuffer->draw(drawCallParams.vertexCount(), 1, drawCallParams.firstVertex(), 0);
+ commandBuffer->draw(clampedVertexCount, 1, drawCallParams.firstVertex(), 0);
return gl::NoError();
}
// Handle GL_LINE_LOOP drawArrays.
- int lastVertex = drawCallParams.firstVertex() + drawCallParams.vertexCount();
+ size_t lastVertex = static_cast<size_t>(drawCallParams.firstVertex() + clampedVertexCount);
if (!mLineLoopBufferFirstIndex.valid() || !mLineLoopBufferLastIndex.valid() ||
mLineLoopBufferFirstIndex != drawCallParams.firstVertex() ||
mLineLoopBufferLastIndex != lastVertex)
@@ -380,7 +383,7 @@
commandBuffer->bindIndexBuffer(mCurrentElementArrayBufferHandle,
mCurrentElementArrayBufferOffset, VK_INDEX_TYPE_UINT32);
- vk::LineLoopHelper::Draw(drawCallParams.vertexCount(), commandBuffer);
+ vk::LineLoopHelper::Draw(clampedVertexCount, commandBuffer);
return gl::NoError();
}
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.h b/src/libANGLE/renderer/vulkan/VertexArrayVk.h
index 4c4cec3..9fe610d 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.h
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.h
@@ -120,8 +120,8 @@
vk::DynamicBuffer mDynamicIndexData;
vk::LineLoopHelper mLineLoopHelper;
- Optional<int> mLineLoopBufferFirstIndex;
- Optional<int> mLineLoopBufferLastIndex;
+ Optional<GLint> mLineLoopBufferFirstIndex;
+ Optional<size_t> mLineLoopBufferLastIndex;
bool mDirtyLineLoopTranslation;
// Cache variable for determining whether or not to store new dependencies in the node.
diff --git a/src/libANGLE/renderer/vulkan/vk_helpers.cpp b/src/libANGLE/renderer/vulkan/vk_helpers.cpp
index 765b283..f8e5cd6 100644
--- a/src/libANGLE/renderer/vulkan/vk_helpers.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_helpers.cpp
@@ -379,8 +379,11 @@
&offset, nullptr));
*offsetOut = static_cast<VkDeviceSize>(offset);
+ uint32_t clampedVertexCount = drawCallParams.getClampedVertexCount<uint32_t>();
+
+ // Note: there could be an overflow in this addition.
uint32_t unsignedFirstVertex = static_cast<uint32_t>(drawCallParams.firstVertex());
- uint32_t vertexCount = (drawCallParams.vertexCount() + unsignedFirstVertex);
+ uint32_t vertexCount = (clampedVertexCount + unsignedFirstVertex);
for (uint32_t vertexIndex = unsignedFirstVertex; vertexIndex < vertexCount; vertexIndex++)
{
*indices++ = vertexIndex;
@@ -469,9 +472,10 @@
}
// static
-void LineLoopHelper::Draw(int count, CommandBuffer *commandBuffer)
+void LineLoopHelper::Draw(uint32_t count, CommandBuffer *commandBuffer)
{
// Our first index is always 0 because that's how we set it up in createIndexBuffer*.
+ // Note: this could theoretically overflow and wrap to zero.
commandBuffer->drawIndexed(count + 1, 1, 0, 0, 0);
}
diff --git a/src/libANGLE/renderer/vulkan/vk_helpers.h b/src/libANGLE/renderer/vulkan/vk_helpers.h
index b83a3b1..75c88e4 100644
--- a/src/libANGLE/renderer/vulkan/vk_helpers.h
+++ b/src/libANGLE/renderer/vulkan/vk_helpers.h
@@ -152,7 +152,7 @@
void destroy(VkDevice device);
- static void Draw(int count, CommandBuffer *commandBuffer);
+ static void Draw(uint32_t count, CommandBuffer *commandBuffer);
private:
DynamicBuffer mDynamicIndexBuffer;
diff --git a/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp b/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp
index 52563ec..9409431 100644
--- a/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp
+++ b/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp
@@ -258,6 +258,46 @@
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
}
+// Covers drawing with a very large vertex range which overflows GLsizei. http://crbug.com/842028
+TEST_P(RobustBufferAccessBehaviorTest, VeryLargeVertexCountWithDynamicVertexData)
+{
+ ANGLE_SKIP_TEST_IF(!initExtension());
+ ANGLE_SKIP_TEST_IF(!extensionEnabled("GL_OES_element_index_uint"));
+
+ constexpr GLsizei kIndexCount = 32;
+ std::array<GLuint, kIndexCount> indices = {{}};
+ for (GLsizei index = 0; index < kIndexCount; ++index)
+ {
+ indices[index] = ((std::numeric_limits<GLuint>::max() - 2) / kIndexCount) * index;
+ }
+
+ GLBuffer indexBuffer;
+ glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuffer);
+ glBufferData(GL_ELEMENT_ARRAY_BUFFER, indices.size() * sizeof(GLuint), indices.data(),
+ GL_STATIC_DRAW);
+
+ std::array<GLfloat, 256> vertexData = {{}};
+
+ GLBuffer vertexBuffer;
+ glBindBuffer(GL_ARRAY_BUFFER, vertexBuffer);
+ glBufferData(GL_ARRAY_BUFFER, vertexData.size() * sizeof(GLfloat), vertexData.data(),
+ GL_DYNAMIC_DRAW);
+
+ ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), essl1_shaders::fs::Red());
+ glUseProgram(program);
+
+ GLint attribLoc = glGetAttribLocation(program, essl1_shaders::PositionAttrib());
+ ASSERT_NE(-1, attribLoc);
+
+ glVertexAttribPointer(attribLoc, 2, GL_FLOAT, GL_FALSE, 0, nullptr);
+ glEnableVertexAttribArray(attribLoc);
+ ASSERT_GL_NO_ERROR();
+
+ glDrawElements(GL_TRIANGLES, kIndexCount, GL_UNSIGNED_INT, nullptr);
+
+ // This may or may not generate an error, but it should not crash.
+}
+
ANGLE_INSTANTIATE_TEST(RobustBufferAccessBehaviorTest,
ES2_D3D9(),
ES2_D3D11_FL9_3(),
diff --git a/src/tests/perf_tests/IndexDataManagerTest.cpp b/src/tests/perf_tests/IndexDataManagerTest.cpp
index 35f6586..80e1b8f 100644
--- a/src/tests/perf_tests/IndexDataManagerTest.cpp
+++ b/src/tests/perf_tests/IndexDataManagerTest.cpp
@@ -58,7 +58,7 @@
MOCK_CONST_METHOD4(getVertexSpaceRequired,
gl::ErrorOrResult<unsigned int>(const gl::VertexAttribute &,
const gl::VertexBinding &,
- GLsizei,
+ size_t,
GLsizei));
// Dependency injection