Commit 702006f4 by Jamie Madill

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. Also combined with 26b0bfb46: Fix warnings from size_t conversions. Bug: chromium:842028 Change-Id: Ie1a8f476f3e97149362eb9855f08450c067ff807 Reviewed-on: https://chromium-review.googlesource.com/1064721Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org>
parent f7d4f25e
...@@ -100,7 +100,7 @@ GLint DrawCallParams::firstVertex() const ...@@ -100,7 +100,7 @@ GLint DrawCallParams::firstVertex() const
return mFirstVertex; return mFirstVertex;
} }
GLsizei DrawCallParams::vertexCount() const size_t DrawCallParams::vertexCount() const
{ {
ASSERT(!isDrawElements() || mIndexRange.valid()); ASSERT(!isDrawElements() || mIndexRange.valid());
return mVertexCount; return mVertexCount;
...@@ -179,7 +179,7 @@ Error DrawCallParams::ensureIndexRangeResolved(const Context *context) const ...@@ -179,7 +179,7 @@ Error DrawCallParams::ensureIndexRangeResolved(const Context *context) const
const IndexRange &indexRange = mIndexRange.value(); const IndexRange &indexRange = mIndexRange.value();
mFirstVertex = mBaseVertex + static_cast<GLint>(indexRange.start); mFirstVertex = mBaseVertex + static_cast<GLint>(indexRange.start);
mVertexCount = static_cast<GLsizei>(indexRange.vertexCount()); mVertexCount = indexRange.vertexCount();
return NoError(); return NoError();
} }
......
...@@ -98,7 +98,7 @@ class DrawCallParams final : angle::NonCopyable ...@@ -98,7 +98,7 @@ class DrawCallParams final : angle::NonCopyable
// This value is the sum of 'baseVertex' and the first indexed vertex for DrawElements calls. // This value is the sum of 'baseVertex' and the first indexed vertex for DrawElements calls.
GLint firstVertex() const; GLint firstVertex() const;
GLsizei vertexCount() const; size_t vertexCount() const;
GLsizei indexCount() const; GLsizei indexCount() const;
GLint baseVertex() const; GLint baseVertex() const;
GLenum type() const; GLenum type() const;
...@@ -113,6 +113,9 @@ class DrawCallParams final : angle::NonCopyable ...@@ -113,6 +113,9 @@ class DrawCallParams final : angle::NonCopyable
// ensureIndexRangeResolved must be called first. // ensureIndexRangeResolved must be called first.
const IndexRange &getIndexRange() const; const IndexRange &getIndexRange() const;
template <typename T>
T getClampedVertexCount() const;
template <EntryPoint EP, typename... ArgsT> template <EntryPoint EP, typename... ArgsT>
static void Factory(DrawCallParams *objBuffer, ArgsT... args); static void Factory(DrawCallParams *objBuffer, ArgsT... args);
...@@ -122,7 +125,7 @@ class DrawCallParams final : angle::NonCopyable ...@@ -122,7 +125,7 @@ class DrawCallParams final : angle::NonCopyable
GLenum mMode; GLenum mMode;
mutable Optional<IndexRange> mIndexRange; mutable Optional<IndexRange> mIndexRange;
mutable GLint mFirstVertex; mutable GLint mFirstVertex;
mutable GLsizei mVertexCount; mutable size_t mVertexCount;
GLint mIndexCount; GLint mIndexCount;
GLint mBaseVertex; GLint mBaseVertex;
GLenum mType; GLenum mType;
...@@ -131,6 +134,13 @@ class DrawCallParams final : angle::NonCopyable ...@@ -131,6 +134,13 @@ class DrawCallParams final : angle::NonCopyable
const void *mIndirect; 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 // 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 // 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 // various indexed draw calls, they drop parameters that aren't useful and re-arrange
......
...@@ -160,10 +160,11 @@ void BufferD3D::invalidateStaticData(const gl::Context *context) ...@@ -160,10 +160,11 @@ void BufferD3D::invalidateStaticData(const gl::Context *context)
} }
// Creates static buffers if sufficient used data has been left unmodified // 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) if (mUsage == D3DBufferUsage::DYNAMIC)
{ {
// Note: This is not a safe math operation. 'dataSize' can come from the app.
mUnmodifiedDataUse += dataSize; mUnmodifiedDataUse += dataSize;
if (mUnmodifiedDataUse > 3 * getSize()) if (mUnmodifiedDataUse > 3 * getSize())
......
...@@ -55,7 +55,7 @@ class BufferD3D : public BufferImpl ...@@ -55,7 +55,7 @@ class BufferD3D : public BufferImpl
virtual void initializeStaticData(const gl::Context *context); virtual void initializeStaticData(const gl::Context *context);
virtual void invalidateStaticData(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, gl::Error getIndexRange(const gl::Context *context,
GLenum type, GLenum type,
...@@ -80,7 +80,7 @@ class BufferD3D : public BufferImpl ...@@ -80,7 +80,7 @@ class BufferD3D : public BufferImpl
StaticIndexBufferInterface *mStaticIndexBuffer; StaticIndexBufferInterface *mStaticIndexBuffer;
unsigned int mStaticBufferCacheTotalSize; unsigned int mStaticBufferCacheTotalSize;
unsigned int mStaticVertexBufferOutOfDate; unsigned int mStaticVertexBufferOutOfDate;
unsigned int mUnmodifiedDataUse; size_t mUnmodifiedDataUse;
D3DBufferUsage mUsage; D3DBufferUsage mUsage;
}; };
......
...@@ -92,7 +92,7 @@ class BufferFactoryD3D : angle::NonCopyable ...@@ -92,7 +92,7 @@ class BufferFactoryD3D : angle::NonCopyable
virtual gl::ErrorOrResult<unsigned int> getVertexSpaceRequired( virtual gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(
const gl::VertexAttribute &attrib, const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) const = 0; GLsizei instances) const = 0;
}; };
......
...@@ -92,7 +92,7 @@ gl::Error VertexBufferInterface::setBufferSize(unsigned int size) ...@@ -92,7 +92,7 @@ gl::Error VertexBufferInterface::setBufferSize(unsigned int size)
gl::ErrorOrResult<unsigned int> VertexBufferInterface::getSpaceRequired( gl::ErrorOrResult<unsigned int> VertexBufferInterface::getSpaceRequired(
const gl::VertexAttribute &attrib, const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) const GLsizei instances) const
{ {
unsigned int spaceRequired = 0; unsigned int spaceRequired = 0;
...@@ -155,7 +155,7 @@ gl::Error StreamingVertexBufferInterface::storeDynamicAttribute(const gl::Vertex ...@@ -155,7 +155,7 @@ gl::Error StreamingVertexBufferInterface::storeDynamicAttribute(const gl::Vertex
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLenum currentValueType, GLenum currentValueType,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances, GLsizei instances,
unsigned int *outStreamOffset, unsigned int *outStreamOffset,
const uint8_t *sourceData) const uint8_t *sourceData)
...@@ -190,7 +190,7 @@ gl::Error StreamingVertexBufferInterface::storeDynamicAttribute(const gl::Vertex ...@@ -190,7 +190,7 @@ gl::Error StreamingVertexBufferInterface::storeDynamicAttribute(const gl::Vertex
gl::Error StreamingVertexBufferInterface::reserveVertexSpace(const gl::VertexAttribute &attrib, gl::Error StreamingVertexBufferInterface::reserveVertexSpace(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) GLsizei instances)
{ {
unsigned int requiredSpace = 0; unsigned int requiredSpace = 0;
......
...@@ -45,7 +45,7 @@ class VertexBuffer : angle::NonCopyable ...@@ -45,7 +45,7 @@ class VertexBuffer : angle::NonCopyable
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLenum currentValueType, GLenum currentValueType,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances, GLsizei instances,
unsigned int offset, unsigned int offset,
const uint8_t *sourceData) = 0; const uint8_t *sourceData) = 0;
...@@ -93,7 +93,7 @@ class VertexBufferInterface : angle::NonCopyable ...@@ -93,7 +93,7 @@ class VertexBufferInterface : angle::NonCopyable
gl::ErrorOrResult<unsigned int> getSpaceRequired(const gl::VertexAttribute &attrib, gl::ErrorOrResult<unsigned int> getSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) const; GLsizei instances) const;
BufferFactoryD3D *const mFactory; BufferFactoryD3D *const mFactory;
VertexBuffer *mVertexBuffer; VertexBuffer *mVertexBuffer;
...@@ -110,14 +110,14 @@ class StreamingVertexBufferInterface : public VertexBufferInterface ...@@ -110,14 +110,14 @@ class StreamingVertexBufferInterface : public VertexBufferInterface
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLenum currentValueType, GLenum currentValueType,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances, GLsizei instances,
unsigned int *outStreamOffset, unsigned int *outStreamOffset,
const uint8_t *sourceData); const uint8_t *sourceData);
gl::Error reserveVertexSpace(const gl::VertexAttribute &attribute, gl::Error reserveVertexSpace(const gl::VertexAttribute &attribute,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances); GLsizei instances);
private: private:
......
...@@ -392,7 +392,7 @@ gl::Error VertexDataManager::storeDynamicAttribs( ...@@ -392,7 +392,7 @@ gl::Error VertexDataManager::storeDynamicAttribs(
std::vector<TranslatedAttribute> *translatedAttribs, std::vector<TranslatedAttribute> *translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask, const gl::AttributesMask &dynamicAttribsMask,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances) GLsizei instances)
{ {
// Instantiating this class will ensure the streaming buffer is never left mapped. // Instantiating this class will ensure the streaming buffer is never left mapped.
...@@ -434,7 +434,7 @@ void VertexDataManager::PromoteDynamicAttribs( ...@@ -434,7 +434,7 @@ void VertexDataManager::PromoteDynamicAttribs(
const gl::Context *context, const gl::Context *context,
const std::vector<TranslatedAttribute> &translatedAttribs, const std::vector<TranslatedAttribute> &translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask, const gl::AttributesMask &dynamicAttribsMask,
GLsizei count) size_t count)
{ {
for (auto attribIndex : dynamicAttribsMask) for (auto attribIndex : dynamicAttribsMask)
{ {
...@@ -445,16 +445,17 @@ void VertexDataManager::PromoteDynamicAttribs( ...@@ -445,16 +445,17 @@ void VertexDataManager::PromoteDynamicAttribs(
gl::Buffer *buffer = binding.getBuffer().get(); gl::Buffer *buffer = binding.getBuffer().get();
if (buffer) if (buffer)
{ {
// Note: this multiplication can overflow. It should not be a security problem.
BufferD3D *bufferD3D = GetImplAs<BufferD3D>(buffer); BufferD3D *bufferD3D = GetImplAs<BufferD3D>(buffer);
size_t typeSize = ComputeVertexAttributeTypeSize(*dynamicAttrib.attribute); 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, gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances) const GLsizei instances) const
{ {
ASSERT(translatedAttrib.attribute && translatedAttrib.binding); ASSERT(translatedAttrib.attribute && translatedAttrib.binding);
...@@ -467,8 +468,8 @@ gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &tr ...@@ -467,8 +468,8 @@ gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &tr
BufferD3D *bufferD3D = buffer ? GetImplAs<BufferD3D>(buffer) : nullptr; BufferD3D *bufferD3D = buffer ? GetImplAs<BufferD3D>(buffer) : nullptr;
ASSERT(!bufferD3D || bufferD3D->getStaticVertexBuffer(attrib, binding) == nullptr); ASSERT(!bufferD3D || bufferD3D->getStaticVertexBuffer(attrib, binding) == nullptr);
size_t totalCount = gl::ComputeVertexBindingElementCount( size_t totalCount = gl::ComputeVertexBindingElementCount(binding.getDivisor(), count,
binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances)); static_cast<size_t>(instances));
// TODO(jiajia.qin@intel.com): force the index buffer to clamp any out of range indices instead // TODO(jiajia.qin@intel.com): force the index buffer to clamp any out of range indices instead
// of invalid operation here. // of invalid operation here.
if (bufferD3D) if (bufferD3D)
...@@ -486,15 +487,14 @@ gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &tr ...@@ -486,15 +487,14 @@ gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &tr
return gl::InvalidOperation() << "Vertex buffer is not big enough for the draw call."; return gl::InvalidOperation() << "Vertex buffer is not big enough for the draw call.";
} }
} }
return mStreamingBuffer->reserveVertexSpace(attrib, binding, static_cast<GLsizei>(totalCount), return mStreamingBuffer->reserveVertexSpace(attrib, binding, totalCount, instances);
instances);
} }
gl::Error VertexDataManager::storeDynamicAttrib(const gl::Context *context, gl::Error VertexDataManager::storeDynamicAttrib(const gl::Context *context,
TranslatedAttribute *translated, TranslatedAttribute *translated,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances) GLsizei instances) const
{ {
ASSERT(translated->attribute && translated->binding); ASSERT(translated->attribute && translated->binding);
const auto &attrib = *translated->attribute; const auto &attrib = *translated->attribute;
...@@ -529,8 +529,8 @@ gl::Error VertexDataManager::storeDynamicAttrib(const gl::Context *context, ...@@ -529,8 +529,8 @@ gl::Error VertexDataManager::storeDynamicAttrib(const gl::Context *context,
translated->storage = nullptr; translated->storage = nullptr;
ANGLE_TRY_RESULT(mFactory->getVertexSpaceRequired(attrib, binding, 1, 0), translated->stride); ANGLE_TRY_RESULT(mFactory->getVertexSpaceRequired(attrib, binding, 1, 0), translated->stride);
size_t totalCount = gl::ComputeVertexBindingElementCount( size_t totalCount = gl::ComputeVertexBindingElementCount(binding.getDivisor(), count,
binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances)); static_cast<size_t>(instances));
ANGLE_TRY(mStreamingBuffer->storeDynamicAttribute( ANGLE_TRY(mStreamingBuffer->storeDynamicAttribute(
attrib, binding, translated->currentValueType, firstVertexIndex, attrib, binding, translated->currentValueType, firstVertexIndex,
......
...@@ -105,14 +105,14 @@ class VertexDataManager : angle::NonCopyable ...@@ -105,14 +105,14 @@ class VertexDataManager : angle::NonCopyable
std::vector<TranslatedAttribute> *translatedAttribs, std::vector<TranslatedAttribute> *translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask, const gl::AttributesMask &dynamicAttribsMask,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances); GLsizei instances);
// Promote static usage of dynamic buffers. // Promote static usage of dynamic buffers.
static void PromoteDynamicAttribs(const gl::Context *context, static void PromoteDynamicAttribs(const gl::Context *context,
const std::vector<TranslatedAttribute> &translatedAttribs, const std::vector<TranslatedAttribute> &translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask, const gl::AttributesMask &dynamicAttribsMask,
GLsizei count); size_t count);
gl::Error storeCurrentValue(const gl::VertexAttribCurrentValueData &currentValue, gl::Error storeCurrentValue(const gl::VertexAttribCurrentValueData &currentValue,
TranslatedAttribute *translated, TranslatedAttribute *translated,
...@@ -130,15 +130,15 @@ class VertexDataManager : angle::NonCopyable ...@@ -130,15 +130,15 @@ class VertexDataManager : angle::NonCopyable
}; };
gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib, gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
GLsizei count,
GLint start, GLint start,
size_t count,
GLsizei instances) const; GLsizei instances) const;
gl::Error storeDynamicAttrib(const gl::Context *context, gl::Error storeDynamicAttrib(const gl::Context *context,
TranslatedAttribute *translated, TranslatedAttribute *translated,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances); GLsizei instances) const;
BufferFactoryD3D *const mFactory; BufferFactoryD3D *const mFactory;
......
...@@ -246,11 +246,12 @@ gl::Error InputLayoutCache::createInputLayout( ...@@ -246,11 +246,12 @@ gl::Error InputLayoutCache::createInputLayout(
// As per the spec for ANGLE_instanced_arrays, not all attributes can be instanced // As per the spec for ANGLE_instanced_arrays, not all attributes can be instanced
// simultaneously, so a non-instanced element must exist. // simultaneously, so a non-instanced element must exist.
GLsizei numIndicesPerInstance = 0; UINT numIndicesPerInstance = 0;
if (drawCallParams.instances() > 0) if (drawCallParams.instances() > 0)
{ {
// This requires that the index range is resolved. // 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) for (size_t elementIndex = 0; elementIndex < inputElementCount; ++elementIndex)
......
...@@ -1400,7 +1400,7 @@ void *Renderer11::getD3DDevice() ...@@ -1400,7 +1400,7 @@ void *Renderer11::getD3DDevice()
gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallParams &params) gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallParams &params)
{ {
if (params.vertexCount() < mStateManager.getCurrentMinimumDrawCount()) if (params.vertexCount() < static_cast<size_t>(mStateManager.getCurrentMinimumDrawCount()))
{ {
return gl::NoError(); return gl::NoError();
} }
...@@ -1416,6 +1416,9 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1416,6 +1416,9 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP
GLsizei adjustedInstanceCount = GetAdjustedInstanceCount(program, params.instances()); GLsizei adjustedInstanceCount = GetAdjustedInstanceCount(program, params.instances());
ProgramD3D *programD3D = GetImplAs<ProgramD3D>(program); ProgramD3D *programD3D = GetImplAs<ProgramD3D>(program);
// Note: vertex indexes can be arbitrarily large.
UINT clampedVertexCount = params.getClampedVertexCount<UINT>();
if (programD3D->usesGeometryShader(params.mode()) && if (programD3D->usesGeometryShader(params.mode()) &&
glState.isTransformFeedbackActiveUnpaused()) glState.isTransformFeedbackActiveUnpaused())
{ {
...@@ -1427,11 +1430,11 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1427,11 +1430,11 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP
if (adjustedInstanceCount > 0) if (adjustedInstanceCount > 0)
{ {
mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0); mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
} }
else else
{ {
mDeviceContext->Draw(params.vertexCount(), 0); mDeviceContext->Draw(clampedVertexCount, 0);
} }
rx::ShaderExecutableD3D *pixelExe = nullptr; rx::ShaderExecutableD3D *pixelExe = nullptr;
...@@ -1455,24 +1458,24 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1455,24 +1458,24 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP
if (adjustedInstanceCount > 0) if (adjustedInstanceCount > 0)
{ {
mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0); mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
} }
else else
{ {
mDeviceContext->Draw(params.vertexCount(), 0); mDeviceContext->Draw(clampedVertexCount, 0);
} }
return gl::NoError(); return gl::NoError();
} }
if (params.mode() == GL_LINE_LOOP) if (params.mode() == GL_LINE_LOOP)
{ {
return drawLineLoop(context, params.vertexCount(), GL_NONE, nullptr, 0, return drawLineLoop(context, clampedVertexCount, GL_NONE, nullptr, 0,
adjustedInstanceCount); adjustedInstanceCount);
} }
if (params.mode() == GL_TRIANGLE_FAN) if (params.mode() == GL_TRIANGLE_FAN)
{ {
return drawTriangleFan(context, params.vertexCount(), GL_NONE, nullptr, 0, return drawTriangleFan(context, clampedVertexCount, GL_NONE, nullptr, 0,
adjustedInstanceCount); adjustedInstanceCount);
} }
...@@ -1483,11 +1486,11 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1483,11 +1486,11 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP
{ {
if (adjustedInstanceCount == 0) if (adjustedInstanceCount == 0)
{ {
mDeviceContext->Draw(params.vertexCount(), 0); mDeviceContext->Draw(clampedVertexCount, 0);
} }
else else
{ {
mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0); mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
} }
return gl::NoError(); return gl::NoError();
} }
...@@ -1500,7 +1503,7 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1500,7 +1503,7 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP
// D3D_PRIMITIVE_TOPOLOGY_TRIANGLELIST and DrawIndexedInstanced is called instead. // D3D_PRIMITIVE_TOPOLOGY_TRIANGLELIST and DrawIndexedInstanced is called instead.
if (adjustedInstanceCount == 0) if (adjustedInstanceCount == 0)
{ {
mDeviceContext->DrawIndexedInstanced(6, params.vertexCount(), 0, 0, 0); mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
return gl::NoError(); return gl::NoError();
} }
...@@ -1513,7 +1516,7 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1513,7 +1516,7 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP
{ {
ANGLE_TRY( ANGLE_TRY(
mStateManager.updateVertexOffsetsForPointSpritesEmulation(params.baseVertex(), i)); 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. // This required by updateVertexOffsets... above but is outside of the loop for speed.
...@@ -1592,13 +1595,13 @@ gl::Error Renderer11::drawElements(const gl::Context *context, const gl::DrawCal ...@@ -1592,13 +1595,13 @@ gl::Error Renderer11::drawElements(const gl::Context *context, const gl::DrawCal
// efficent code path. Instanced rendering of emulated pointsprites requires a loop to draw each // 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 // batch of points. An offset into the instanced data buffer is calculated and applied on each
// iteration to ensure all instances are rendered correctly. // 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. // Each instance being rendered requires the inputlayout cache to reapply buffers and offsets.
for (GLsizei i = 0; i < params.instances(); i++) for (GLsizei i = 0; i < params.instances(); i++)
{ {
ANGLE_TRY(mStateManager.updateVertexOffsetsForPointSpritesEmulation(startVertex, i)); ANGLE_TRY(mStateManager.updateVertexOffsetsForPointSpritesEmulation(startVertex, i));
mDeviceContext->DrawIndexedInstanced(6, elementsToRender, 0, 0, 0); mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
} }
mStateManager.invalidateVertexBuffer(); mStateManager.invalidateVertexBuffer();
return gl::NoError(); return gl::NoError();
...@@ -1650,7 +1653,7 @@ gl::Error Renderer11::drawElementsIndirect(const gl::Context *context, ...@@ -1650,7 +1653,7 @@ gl::Error Renderer11::drawElementsIndirect(const gl::Context *context,
} }
gl::Error Renderer11::drawLineLoop(const gl::Context *context, gl::Error Renderer11::drawLineLoop(const gl::Context *context,
GLsizei count, GLuint count,
GLenum type, GLenum type,
const void *indexPointer, const void *indexPointer,
int baseVertex, int baseVertex,
...@@ -1687,8 +1690,6 @@ gl::Error Renderer11::drawLineLoop(const gl::Context *context, ...@@ -1687,8 +1690,6 @@ gl::Error Renderer11::drawLineLoop(const gl::Context *context,
} }
// Checked by Renderer11::applyPrimitiveType // Checked by Renderer11::applyPrimitiveType
ASSERT(count >= 0);
if (static_cast<unsigned int>(count) + 1 > if (static_cast<unsigned int>(count) + 1 >
(std::numeric_limits<unsigned int>::max() / sizeof(unsigned int))) (std::numeric_limits<unsigned int>::max() / sizeof(unsigned int)))
{ {
...@@ -1734,7 +1735,7 @@ gl::Error Renderer11::drawLineLoop(const gl::Context *context, ...@@ -1734,7 +1735,7 @@ gl::Error Renderer11::drawLineLoop(const gl::Context *context,
} }
gl::Error Renderer11::drawTriangleFan(const gl::Context *context, gl::Error Renderer11::drawTriangleFan(const gl::Context *context,
GLsizei count, GLuint count,
GLenum type, GLenum type,
const void *indices, const void *indices,
int baseVertex, int baseVertex,
...@@ -3598,7 +3599,7 @@ GLenum Renderer11::getVertexComponentType(gl::VertexFormatType vertexFormatType) ...@@ -3598,7 +3599,7 @@ GLenum Renderer11::getVertexComponentType(gl::VertexFormatType vertexFormatType)
gl::ErrorOrResult<unsigned int> Renderer11::getVertexSpaceRequired( gl::ErrorOrResult<unsigned int> Renderer11::getVertexSpaceRequired(
const gl::VertexAttribute &attrib, const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) const GLsizei instances) const
{ {
if (!attrib.enabled) if (!attrib.enabled)
...@@ -3610,7 +3611,8 @@ gl::ErrorOrResult<unsigned int> Renderer11::getVertexSpaceRequired( ...@@ -3610,7 +3611,8 @@ gl::ErrorOrResult<unsigned int> Renderer11::getVertexSpaceRequired(
const unsigned int divisor = binding.getDivisor(); const unsigned int divisor = binding.getDivisor();
if (instances == 0 || divisor == 0) if (instances == 0 || divisor == 0)
{ {
elementCount = count; // This could be a clipped cast.
elementCount = gl::clampCast<unsigned int>(count);
} }
else else
{ {
......
...@@ -343,7 +343,7 @@ class Renderer11 : public RendererD3D ...@@ -343,7 +343,7 @@ class Renderer11 : public RendererD3D
// function. // function.
gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib, gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) const override; GLsizei instances) const override;
gl::Error readFromAttachment(const gl::Context *context, gl::Error readFromAttachment(const gl::Context *context,
...@@ -461,13 +461,13 @@ class Renderer11 : public RendererD3D ...@@ -461,13 +461,13 @@ class Renderer11 : public RendererD3D
angle::WorkaroundsD3D generateWorkarounds() const override; angle::WorkaroundsD3D generateWorkarounds() const override;
gl::Error drawLineLoop(const gl::Context *context, gl::Error drawLineLoop(const gl::Context *context,
GLsizei count, GLuint count,
GLenum type, GLenum type,
const void *indices, const void *indices,
int baseVertex, int baseVertex,
int instances); int instances);
gl::Error drawTriangleFan(const gl::Context *context, gl::Error drawTriangleFan(const gl::Context *context,
GLsizei count, GLuint count,
GLenum type, GLenum type,
const void *indices, const void *indices,
int baseVertex, int baseVertex,
......
...@@ -96,7 +96,7 @@ gl::Error VertexBuffer11::storeVertexAttributes(const gl::VertexAttribute &attri ...@@ -96,7 +96,7 @@ gl::Error VertexBuffer11::storeVertexAttributes(const gl::VertexAttribute &attri
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLenum currentValueType, GLenum currentValueType,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances, GLsizei instances,
unsigned int offset, unsigned int offset,
const uint8_t *sourceData) const uint8_t *sourceData)
......
...@@ -31,7 +31,7 @@ class VertexBuffer11 : public VertexBuffer ...@@ -31,7 +31,7 @@ class VertexBuffer11 : public VertexBuffer
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLenum currentValueType, GLenum currentValueType,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances, GLsizei instances,
unsigned int offset, unsigned int offset,
const uint8_t *sourceData) override; const uint8_t *sourceData) override;
......
...@@ -2998,7 +2998,7 @@ GLenum Renderer9::getVertexComponentType(gl::VertexFormatType vertexFormatType) ...@@ -2998,7 +2998,7 @@ GLenum Renderer9::getVertexComponentType(gl::VertexFormatType vertexFormatType)
gl::ErrorOrResult<unsigned int> Renderer9::getVertexSpaceRequired(const gl::VertexAttribute &attrib, gl::ErrorOrResult<unsigned int> Renderer9::getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) const GLsizei instances) const
{ {
if (!attrib.enabled) if (!attrib.enabled)
......
...@@ -346,7 +346,7 @@ class Renderer9 : public RendererD3D ...@@ -346,7 +346,7 @@ class Renderer9 : public RendererD3D
// function. // function.
gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib, gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLsizei count, size_t count,
GLsizei instances) const override; GLsizei instances) const override;
gl::Error copyToRenderTarget(IDirect3DSurface9 *dest, gl::Error copyToRenderTarget(IDirect3DSurface9 *dest,
......
...@@ -61,7 +61,7 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib ...@@ -61,7 +61,7 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLenum currentValueType, GLenum currentValueType,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances, GLsizei instances,
unsigned int offset, unsigned int offset,
const uint8_t *sourceData) const uint8_t *sourceData)
...@@ -71,8 +71,8 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib ...@@ -71,8 +71,8 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib
return gl::OutOfMemory() << "Internal vertex buffer is not initialized."; return gl::OutOfMemory() << "Internal vertex buffer is not initialized.";
} }
int inputStride = static_cast<int>(gl::ComputeVertexAttributeStride(attrib, binding)); size_t inputStride = gl::ComputeVertexAttributeStride(attrib, binding);
int elementSize = static_cast<int>(gl::ComputeVertexAttributeTypeSize(attrib)); size_t elementSize = gl::ComputeVertexAttributeTypeSize(attrib);
DWORD lockFlags = mDynamicUsage ? D3DLOCK_NOOVERWRITE : 0; DWORD lockFlags = mDynamicUsage ? D3DLOCK_NOOVERWRITE : 0;
...@@ -105,7 +105,7 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib ...@@ -105,7 +105,7 @@ gl::Error VertexBuffer9::storeVertexAttributes(const gl::VertexAttribute &attrib
if (!needsConversion && inputStride == elementSize) 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); memcpy(mapPtr, input, copySize);
} }
else else
......
...@@ -28,7 +28,7 @@ class VertexBuffer9 : public VertexBuffer ...@@ -28,7 +28,7 @@ class VertexBuffer9 : public VertexBuffer
const gl::VertexBinding &binding, const gl::VertexBinding &binding,
GLenum currentValueType, GLenum currentValueType,
GLint start, GLint start,
GLsizei count, size_t count,
GLsizei instances, GLsizei instances,
unsigned int offset, unsigned int offset,
const uint8_t *sourceData) override; const uint8_t *sourceData) override;
......
...@@ -343,15 +343,17 @@ gl::Error VertexArrayVk::drawArrays(const gl::Context *context, ...@@ -343,15 +343,17 @@ gl::Error VertexArrayVk::drawArrays(const gl::Context *context,
ANGLE_TRY(onDraw(context, renderer, drawCallParams, drawNode, newCommandBuffer)); 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) if (drawCallParams.mode() != GL_LINE_LOOP)
{ {
commandBuffer->draw(drawCallParams.vertexCount(), 1, drawCallParams.firstVertex(), 0); commandBuffer->draw(clampedVertexCount, 1, drawCallParams.firstVertex(), 0);
return gl::NoError(); return gl::NoError();
} }
// Handle GL_LINE_LOOP drawArrays. // Handle GL_LINE_LOOP drawArrays.
// This test may be incorrect if the draw call switches from DrawArrays/DrawElements. size_t lastVertex = static_cast<size_t>(drawCallParams.firstVertex() + clampedVertexCount);
int lastVertex = drawCallParams.firstVertex() + drawCallParams.vertexCount();
if (!mLineLoopBufferFirstIndex.valid() || !mLineLoopBufferLastIndex.valid() || if (!mLineLoopBufferFirstIndex.valid() || !mLineLoopBufferLastIndex.valid() ||
mLineLoopBufferFirstIndex != drawCallParams.firstVertex() || mLineLoopBufferFirstIndex != drawCallParams.firstVertex() ||
mLineLoopBufferLastIndex != lastVertex) mLineLoopBufferLastIndex != lastVertex)
...@@ -367,7 +369,7 @@ gl::Error VertexArrayVk::drawArrays(const gl::Context *context, ...@@ -367,7 +369,7 @@ gl::Error VertexArrayVk::drawArrays(const gl::Context *context,
commandBuffer->bindIndexBuffer(mCurrentElementArrayBufferHandle, commandBuffer->bindIndexBuffer(mCurrentElementArrayBufferHandle,
mCurrentElementArrayBufferOffset, VK_INDEX_TYPE_UINT32); mCurrentElementArrayBufferOffset, VK_INDEX_TYPE_UINT32);
vk::LineLoopHelper::Draw(drawCallParams.vertexCount(), commandBuffer); vk::LineLoopHelper::Draw(clampedVertexCount, commandBuffer);
return gl::NoError(); return gl::NoError();
} }
......
...@@ -120,8 +120,8 @@ class VertexArrayVk : public VertexArrayImpl ...@@ -120,8 +120,8 @@ class VertexArrayVk : public VertexArrayImpl
vk::DynamicBuffer mDynamicIndexData; vk::DynamicBuffer mDynamicIndexData;
vk::LineLoopHelper mLineLoopHelper; vk::LineLoopHelper mLineLoopHelper;
Optional<int> mLineLoopBufferFirstIndex; Optional<GLint> mLineLoopBufferFirstIndex;
Optional<int> mLineLoopBufferLastIndex; Optional<size_t> mLineLoopBufferLastIndex;
bool mDirtyLineLoopTranslation; bool mDirtyLineLoopTranslation;
// Cache variable for determining whether or not to store new dependencies in the node. // Cache variable for determining whether or not to store new dependencies in the node.
......
...@@ -321,8 +321,11 @@ gl::Error LineLoopHelper::getIndexBufferForDrawArrays(RendererVk *renderer, ...@@ -321,8 +321,11 @@ gl::Error LineLoopHelper::getIndexBufferForDrawArrays(RendererVk *renderer,
&offset, nullptr)); &offset, nullptr));
*offsetOut = static_cast<VkDeviceSize>(offset); *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 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++) for (uint32_t vertexIndex = unsignedFirstVertex; vertexIndex < vertexCount; vertexIndex++)
{ {
*indices++ = vertexIndex; *indices++ = vertexIndex;
...@@ -378,9 +381,10 @@ void LineLoopHelper::destroy(VkDevice device) ...@@ -378,9 +381,10 @@ void LineLoopHelper::destroy(VkDevice device)
} }
// static // 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*. // 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); commandBuffer->drawIndexed(count + 1, 1, 0, 0, 0);
} }
......
...@@ -130,7 +130,7 @@ class LineLoopHelper final : public vk::CommandGraphResource ...@@ -130,7 +130,7 @@ class LineLoopHelper final : public vk::CommandGraphResource
VkDeviceSize *bufferOffsetOut); VkDeviceSize *bufferOffsetOut);
void destroy(VkDevice device); void destroy(VkDevice device);
static void Draw(int count, CommandBuffer *commandBuffer); static void Draw(uint32_t count, CommandBuffer *commandBuffer);
private: private:
DynamicBuffer mDynamicIndexBuffer; DynamicBuffer mDynamicIndexBuffer;
......
...@@ -165,6 +165,49 @@ TEST_P(RobustBufferAccessBehaviorTest, DrawElementsIndexOutOfRangeWithDynamicDra ...@@ -165,6 +165,49 @@ TEST_P(RobustBufferAccessBehaviorTest, DrawElementsIndexOutOfRangeWithDynamicDra
runIndexOutOfRangeTests(GL_DYNAMIC_DRAW); runIndexOutOfRangeTests(GL_DYNAMIC_DRAW);
} }
// 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);
constexpr char simpleVS[] = "attribute vec4 position; void main() { gl_Position = position; }";
constexpr char redFS[] = "void main() { gl_FragColor = vec4(1, 0, 0, 1); }";
ANGLE_GL_PROGRAM(program, simpleVS, redFS);
glUseProgram(program);
GLint attribLoc = glGetAttribLocation(program, "position");
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, ANGLE_INSTANTIATE_TEST(RobustBufferAccessBehaviorTest,
ES2_D3D9(), ES2_D3D9(),
ES2_D3D11_FL9_3(), ES2_D3D11_FL9_3(),
......
...@@ -58,7 +58,7 @@ class MockBufferFactoryD3D : public rx::BufferFactoryD3D ...@@ -58,7 +58,7 @@ class MockBufferFactoryD3D : public rx::BufferFactoryD3D
MOCK_CONST_METHOD4(getVertexSpaceRequired, MOCK_CONST_METHOD4(getVertexSpaceRequired,
gl::ErrorOrResult<unsigned int>(const gl::VertexAttribute &, gl::ErrorOrResult<unsigned int>(const gl::VertexAttribute &,
const gl::VertexBinding &, const gl::VertexBinding &,
GLsizei, size_t,
GLsizei)); GLsizei));
// Dependency injection // Dependency injection
......
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