Commit 71168a07 by Corentin Wallez Committed by Commit Bot

Manually write overflow checks for glDraw*

The usage of checked numerics showed has a big hotspot in ValidateDrawAttribs. BUG=angleproject:1671 BUG=chromium:674143 Change-Id: I96392e099b2257e465fb47e1c96f9aa1f54a89ba Reviewed-on: https://chromium-review.googlesource.com/422428 Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent 5cdbda17
...@@ -56,40 +56,56 @@ bool ValidateDrawAttribs(ValidationContext *context, ...@@ -56,40 +56,56 @@ bool ValidateDrawAttribs(ValidationContext *context,
if (buffer) if (buffer)
{ {
CheckedNumeric<GLint64> maxVertexElement = 0; GLint maxVertexElement = 0;
bool readsData = false; bool readsData = false;
if (attrib.divisor == 0) if (attrib.divisor == 0)
{ {
readsData = vertexCount > 0; readsData = vertexCount > 0;
maxVertexElement = static_cast<GLint64>(maxVertex); maxVertexElement = maxVertex;
} }
else if (primcount > 0) else if (primcount > 0)
{ {
readsData = true; readsData = true;
maxVertexElement = maxVertexElement = (primcount - 1) / attrib.divisor;
static_cast<GLint64>(primcount - 1) / static_cast<GLint64>(attrib.divisor);
} }
// If we're drawing zero vertices, we have enough data. // If we're drawing zero vertices, we have enough data.
if (readsData) if (readsData)
{ {
// Note: Last vertex element does not take the full stride! // We do manual overflow checks here instead of using safe_math.h because it was
CheckedNumeric<GLint64> attribStride = ComputeVertexAttributeStride(attrib); // a bottleneck. Thanks to some properties of GL we know inequalities that can
CheckedNumeric<GLint64> attribSize = ComputeVertexAttributeTypeSize(attrib); // help us make the overflow checks faster.
CheckedNumeric<GLint64> attribDataSize =
maxVertexElement * attribStride + attribSize; // The max possible attribSize is 16 for a vector of 4 32 bit values.
CheckedNumeric<GLint64> maxOffset = attribDataSize + attrib.offset; constexpr uint64_t kMaxAttribSize = 16;
constexpr uint64_t kIntMax = std::numeric_limits<int>::max();
if (!maxOffset.IsValid()) constexpr uint64_t kUint64Max = std::numeric_limits<uint64_t>::max();
// We know attribStride is given as a GLsizei which is typedefed to int.
// We also know an upper bound for attribSize.
static_assert(std::is_same<int, GLsizei>::value, "");
uint64_t attribStride = ComputeVertexAttributeStride(attrib);
uint64_t attribSize = ComputeVertexAttributeTypeSize(attrib);
ASSERT(attribStride <= kIntMax && attribSize <= kMaxAttribSize);
// Computing the max offset using uint64_t without attrib.offset is overflow
// safe. Note: Last vertex element does not take the full stride!
static_assert(kIntMax * kIntMax < kUint64Max - kMaxAttribSize, "");
uint64_t attribDataSizeNoOffset = maxVertexElement * attribStride + attribSize;
// An overflow can happen when adding the offset, check for it.
uint64_t attribOffset = attrib.offset;
if (attribDataSizeNoOffset > kUint64Max - attrib.offset)
{ {
context->handleError(Error(GL_INVALID_OPERATION, "Integer overflow.")); context->handleError(Error(GL_INVALID_OPERATION, "Integer overflow."));
return false; return false;
} }
uint64_t attribDataSizeWithOffset = attribDataSizeNoOffset + attribOffset;
// [OpenGL ES 3.0.2] section 2.9.4 page 40: // [OpenGL ES 3.0.2] section 2.9.4 page 40:
// We can return INVALID_OPERATION if our vertex attribute does not have // We can return INVALID_OPERATION if our vertex attribute does not have
// enough backing data. // enough backing data.
if (maxOffset.ValueOrDie() > buffer->getSize()) if (attribDataSizeWithOffset > static_cast<uint64_t>(buffer->getSize()))
{ {
context->handleError( context->handleError(
Error(GL_INVALID_OPERATION, Error(GL_INVALID_OPERATION,
...@@ -3207,17 +3223,19 @@ bool ValidateDrawArrays(ValidationContext *context, ...@@ -3207,17 +3223,19 @@ bool ValidateDrawArrays(ValidationContext *context,
return false; return false;
} }
CheckedNumeric<GLint> maxVertex = first; // Check the computation of maxVertex doesn't overflow.
maxVertex += count; // - first < 0 or count < 0 have been checked as an error condition
maxVertex -= 1; // - count > 0 has been checked in ValidateDrawBase as it makes the call a noop
// From this we know maxVertex will be positive, and only need to check if it overflows GLint.
if (!maxVertex.IsValid()) ASSERT(count > 0 && first >= 0);
int64_t maxVertex = static_cast<int64_t>(first) + static_cast<int64_t>(count) - 1;
if (maxVertex > static_cast<int64_t>(std::numeric_limits<GLint>::max()))
{ {
context->handleError(Error(GL_INVALID_OPERATION, "Integer overflow.")); context->handleError(Error(GL_INVALID_OPERATION, "Integer overflow."));
return false; return false;
} }
if (!ValidateDrawAttribs(context, primcount, maxVertex.ValueOrDie(), count)) if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(maxVertex), count))
{ {
return false; return false;
} }
......
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