Commit 18e323ab by Jamie Madill Committed by Commit Bot

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: 's avatarGeoff Lang <geofflang@chromium.org>
parent 422f2ce2
...@@ -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;
}; };
......
...@@ -95,7 +95,7 @@ class BufferFactoryD3D : angle::NonCopyable ...@@ -95,7 +95,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;
}; };
......
...@@ -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;
...@@ -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)
......
...@@ -1403,7 +1403,7 @@ void *Renderer11::getD3DDevice() ...@@ -1403,7 +1403,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();
} }
...@@ -1419,6 +1419,9 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1419,6 +1419,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())
{ {
...@@ -1430,11 +1433,11 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1430,11 +1433,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;
...@@ -1458,24 +1461,24 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1458,24 +1461,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);
} }
...@@ -1486,11 +1489,11 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1486,11 +1489,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();
} }
...@@ -1503,7 +1506,7 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1503,7 +1506,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();
} }
...@@ -1516,7 +1519,7 @@ gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallP ...@@ -1516,7 +1519,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.
...@@ -1595,13 +1598,13 @@ gl::Error Renderer11::drawElements(const gl::Context *context, const gl::DrawCal ...@@ -1595,13 +1598,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();
...@@ -1653,7 +1656,7 @@ gl::Error Renderer11::drawElementsIndirect(const gl::Context *context, ...@@ -1653,7 +1656,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,
...@@ -1690,8 +1693,6 @@ gl::Error Renderer11::drawLineLoop(const gl::Context *context, ...@@ -1690,8 +1693,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)))
{ {
...@@ -1737,7 +1738,7 @@ gl::Error Renderer11::drawLineLoop(const gl::Context *context, ...@@ -1737,7 +1738,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,
...@@ -3570,7 +3571,7 @@ GLenum Renderer11::getVertexComponentType(gl::VertexFormatType vertexFormatType) ...@@ -3570,7 +3571,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)
......
...@@ -342,7 +342,7 @@ class Renderer11 : public RendererD3D ...@@ -342,7 +342,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,
...@@ -460,13 +460,13 @@ class Renderer11 : public RendererD3D ...@@ -460,13 +460,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;
......
...@@ -3007,7 +3007,7 @@ GLenum Renderer9::getVertexComponentType(gl::VertexFormatType vertexFormatType) ...@@ -3007,7 +3007,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;
......
...@@ -357,14 +357,17 @@ gl::Error VertexArrayVk::drawArrays(const gl::Context *context, ...@@ -357,14 +357,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.
int lastVertex = drawCallParams.firstVertex() + drawCallParams.vertexCount(); size_t lastVertex = static_cast<size_t>(drawCallParams.firstVertex() + clampedVertexCount);
if (!mLineLoopBufferFirstIndex.valid() || !mLineLoopBufferLastIndex.valid() || if (!mLineLoopBufferFirstIndex.valid() || !mLineLoopBufferLastIndex.valid() ||
mLineLoopBufferFirstIndex != drawCallParams.firstVertex() || mLineLoopBufferFirstIndex != drawCallParams.firstVertex() ||
mLineLoopBufferLastIndex != lastVertex) mLineLoopBufferLastIndex != lastVertex)
...@@ -380,7 +383,7 @@ gl::Error VertexArrayVk::drawArrays(const gl::Context *context, ...@@ -380,7 +383,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.
......
...@@ -379,8 +379,11 @@ gl::Error LineLoopHelper::getIndexBufferForDrawArrays(RendererVk *renderer, ...@@ -379,8 +379,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;
...@@ -469,9 +472,10 @@ void LineLoopHelper::destroy(VkDevice device) ...@@ -469,9 +472,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);
} }
......
...@@ -152,7 +152,7 @@ class LineLoopHelper final : public vk::CommandGraphResource ...@@ -152,7 +152,7 @@ class LineLoopHelper final : public vk::CommandGraphResource
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;
......
...@@ -258,6 +258,46 @@ void main() ...@@ -258,6 +258,46 @@ void main()
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green); 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, 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