Bug 1038928 - WebGL element array cache: dont try to handle null out_upperBound, and add some test coverage for out_upperBound - r=jgilbert
authorBenoit Jacob <bjacob@mozilla.com>
Fri, 18 Jul 2014 10:59:55 -0400
changeset 216857 fad52f3c9132bc0f16ded3783c454d047cf379bf
parent 216856 cd5e53d2aabd574c78dc6fd24481992b3ad71730
child 216858 a4db87a48b24aa254deb527b1811b347b32a950c
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgilbert
bugs1038928
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1038928 - WebGL element array cache: dont try to handle null out_upperBound, and add some test coverage for out_upperBound - r=jgilbert
content/canvas/compiledtest/TestWebGLElementArrayCache.cpp
content/canvas/src/WebGLContextDraw.cpp
content/canvas/src/WebGLElementArrayCache.cpp
content/canvas/src/WebGLElementArrayCache.h
--- a/content/canvas/compiledtest/TestWebGLElementArrayCache.cpp
+++ b/content/canvas/compiledtest/TestWebGLElementArrayCache.cpp
@@ -54,79 +54,96 @@ GLenum GLType()
     case 2:  return LOCAL_GL_UNSIGNED_SHORT;
     case 1:  return LOCAL_GL_UNSIGNED_BYTE;
     default:
       VERIFY(false);
       return 0;
   }
 }
 
+void CheckValidate(bool expectSuccess,
+                   WebGLElementArrayCache& c,
+                   GLenum type,
+                   uint32_t maxAllowed,
+                   size_t first,
+                   size_t count)
+{
+  uint32_t out_upperBound = 0;
+  const bool success = c.Validate(type, maxAllowed, first, count, &out_upperBound);
+  VERIFY(success == expectSuccess);
+  if (success) {
+    VERIFY(out_upperBound <= maxAllowed);
+  } else {
+    VERIFY(out_upperBound > maxAllowed);
+  }
+}
+
 template<typename T>
-void CheckValidateOneType(WebGLElementArrayCache& c, size_t firstByte, size_t countBytes)
+void CheckValidateOneTypeVariousBounds(WebGLElementArrayCache& c, size_t firstByte, size_t countBytes)
 {
   size_t first = firstByte / sizeof(T);
   size_t count = countBytes / sizeof(T);
 
   GLenum type = GLType<T>();
 
   T max = 0;
   for (size_t i = 0; i < count; i++)
     if (c.Element<T>(first + i) > max)
       max = c.Element<T>(first + i);
 
-  VERIFY(c.Validate(type, max, first, count));
-  VERIFY(c.Validate(type, T(-1), first, count));
-  if (T(max + 1)) VERIFY(c.Validate(type, T(max + 1), first, count));
+  CheckValidate(true, c, type, max, first, count);
+  CheckValidate(true, c, type, T(-1), first, count);
+  if (T(max + 1)) CheckValidate(true, c, type, T(max + 1), first, count);
   if (max > 0) {
-    VERIFY(!c.Validate(type, max - 1, first, count));
-    VERIFY(!c.Validate(type, 0, first, count));
+    CheckValidate(false, c, type, max - 1, first, count);
+    CheckValidate(false, c, type, 0, first, count);
   }
 }
 
-void CheckValidate(WebGLElementArrayCache& c, size_t firstByte, size_t countBytes)
+void CheckValidateAllTypes(WebGLElementArrayCache& c, size_t firstByte, size_t countBytes)
 {
-  CheckValidateOneType<uint8_t>(c, firstByte, countBytes);
-  CheckValidateOneType<uint16_t>(c, firstByte, countBytes);
-  CheckValidateOneType<uint32_t>(c, firstByte, countBytes);
+  CheckValidateOneTypeVariousBounds<uint8_t>(c, firstByte, countBytes);
+  CheckValidateOneTypeVariousBounds<uint16_t>(c, firstByte, countBytes);
+  CheckValidateOneTypeVariousBounds<uint32_t>(c, firstByte, countBytes);
 }
 
 template<typename T>
 void CheckSanity()
 {
   const size_t numElems = 64; // should be significantly larger than tree leaf size to
                         // ensure we exercise some nontrivial tree-walking
   T data[numElems] = {1,0,3,1,2,6,5,4}; // intentionally specify only 8 elements for now
   size_t numBytes = numElems * sizeof(T);
   MOZ_ASSERT(numBytes == sizeof(data));
 
   GLenum type = GLType<T>();
 
   WebGLElementArrayCache c;
   c.BufferData(data, numBytes);
-  VERIFY( c.Validate(type, 6, 0, 8));
-  VERIFY(!c.Validate(type, 5, 0, 8));
-  VERIFY( c.Validate(type, 3, 0, 3));
-  VERIFY(!c.Validate(type, 2, 0, 3));
-  VERIFY( c.Validate(type, 6, 2, 4));
-  VERIFY(!c.Validate(type, 5, 2, 4));
+  CheckValidate(true,  c, type, 6, 0, 8);
+  CheckValidate(false, c, type, 5, 0, 8);
+  CheckValidate(true,  c, type, 3, 0, 3);
+  CheckValidate(false, c, type, 2, 0, 3);
+  CheckValidate(true,  c, type, 6, 2, 4);
+  CheckValidate(false, c, type, 5, 2, 4);
 
   c.BufferSubData(5*sizeof(T), data, sizeof(T));
-  VERIFY( c.Validate(type, 5, 0, 8));
-  VERIFY(!c.Validate(type, 4, 0, 8));
+  CheckValidate(true,  c, type, 5, 0, 8);
+  CheckValidate(false, c, type, 4, 0, 8);
 
   // now test a somewhat larger size to ensure we exceed the size of a tree leaf
   for(size_t i = 0; i < numElems; i++)
     data[i] = numElems - i;
   c.BufferData(data, numBytes);
-  VERIFY( c.Validate(type, numElems,     0, numElems));
-  VERIFY(!c.Validate(type, numElems - 1, 0, numElems));
+  CheckValidate(true,  c, type, numElems,     0, numElems);
+  CheckValidate(false, c, type, numElems - 1, 0, numElems);
 
   MOZ_ASSERT(numElems > 10);
-  VERIFY( c.Validate(type, numElems - 10, 10, numElems - 10));
-  VERIFY(!c.Validate(type, numElems - 11, 10, numElems - 10));
+  CheckValidate(true,  c, type, numElems - 10, 10, numElems - 10);
+  CheckValidate(false, c, type, numElems - 11, 10, numElems - 10);
 }
 
 template<typename T>
 void CheckUintOverflow()
 {
   // This test is only for integer types smaller than uint32_t
   static_assert(sizeof(T) < sizeof(uint32_t), "This test is only for integer types \
                 smaller than uint32_t");
@@ -142,19 +159,19 @@ void CheckUintOverflow()
   WebGLElementArrayCache c;
 
   for(size_t i = 0; i < numElems; i++)
     data[i] = numElems - i;
   c.BufferData(data, numBytes);
 
   // bug 825205
   uint32_t bigValWrappingToZero = uint32_t(T(-1)) + 1;
-  VERIFY( c.Validate(type, bigValWrappingToZero,     0, numElems));
-  VERIFY( c.Validate(type, bigValWrappingToZero - 1, 0, numElems));
-  VERIFY(!c.Validate(type,                        0, 0, numElems));
+  CheckValidate(true,  c, type, bigValWrappingToZero,     0, numElems);
+  CheckValidate(true,  c, type, bigValWrappingToZero - 1, 0, numElems);
+  CheckValidate(false, c, type,                        0, 0, numElems);
 }
 
 int main(int argc, char *argv[])
 {
   srand(0); // do not want a random seed here.
 
   CheckSanity<uint8_t>();
   CheckSanity<uint16_t>();
@@ -172,17 +189,17 @@ int main(int argc, char *argv[])
     // producing different values. In that case, the minimum value by which to replace
     // this 20 to reproduce the bug on Linux, was 25. Replacing it with 64 should give
     // us some comfort margin.
     int repeat = std::min(maxBufferSize, 64);
     for (int i = 0; i < repeat; i++) {
       size_t size = RandomInteger<size_t>(0, maxBufferSize);
       MakeRandomVector(v, size);
       b.BufferData(v.Elements(), size);
-      CheckValidate(b, 0, size);
+      CheckValidateAllTypes(b, 0, size);
 
       for (int j = 0; j < 16; j++) {
         for (int bufferSubDataCalls = 1; bufferSubDataCalls <= 8; bufferSubDataCalls *= 2) {
           for (int validateCalls = 1; validateCalls <= 8; validateCalls *= 2) {
 
             size_t offset = 0, subsize = 0;
 
             for (int k = 0; k < bufferSubDataCalls; k++) {
@@ -190,17 +207,17 @@ int main(int argc, char *argv[])
               subsize = RandomInteger<size_t>(0, size - offset);
               MakeRandomVector(vsub, subsize);
               b.BufferSubData(offset, vsub.Elements(), subsize);
             }
 
             for (int k = 0; k < validateCalls; k++) {
               offset = RandomInteger<size_t>(0, size);
               subsize = RandomInteger<size_t>(0, size - offset);
-              CheckValidate(b, offset, subsize);
+              CheckValidateAllTypes(b, offset, subsize);
             }
           } // validateCalls
         } // bufferSubDataCalls
       } // j
     } // i
   } // maxBufferSize
 
   std::cerr << argv[0] << ": all " << gTestsPassed << " tests passed" << std::endl;
--- a/content/canvas/src/WebGLContextDraw.cpp
+++ b/content/canvas/src/WebGLContextDraw.cpp
@@ -299,17 +299,17 @@ WebGLContext::DrawElements(GLenum mode, 
                            WebGLintptr byteOffset)
 {
     if (IsContextLost())
         return;
 
     if (!ValidateDrawModeEnum(mode, "drawElements: mode"))
         return;
 
-    GLuint upperBound = UINT_MAX;
+    GLuint upperBound = 0;
     if (!DrawElements_check(count, type, byteOffset, 1, "drawElements",
                             &upperBound))
     {
         return;
     }
 
     RunContextLossTimer();
 
--- a/content/canvas/src/WebGLElementArrayCache.cpp
+++ b/content/canvas/src/WebGLElementArrayCache.cpp
@@ -12,30 +12,19 @@
 #include <cstdlib>
 #include <cstring>
 #include <limits>
 #include <algorithm>
 
 namespace mozilla {
 
 static void
-SetUpperBound(uint32_t* out_upperBound, uint32_t newBound)
-{
-  if (!out_upperBound)
-      return;
-
-  *out_upperBound = newBound;
-}
-
-static void
 UpdateUpperBound(uint32_t* out_upperBound, uint32_t newBound)
 {
-  if (!out_upperBound)
-      return;
-
+  MOZ_ASSERT(out_upperBound);
   *out_upperBound = std::max(*out_upperBound, newBound);
 }
 
 /*
  * WebGLElementArrayCacheTree contains most of the implementation of WebGLElementArrayCache,
  * which performs WebGL element array buffer validation for drawElements.
  *
  * Attention: Here lie nontrivial data structures, bug-prone algorithms, and non-canonical tweaks!
@@ -484,22 +473,22 @@ bool WebGLElementArrayCache::UpdateTrees
   return result;
 }
 
 template<typename T>
 bool
 WebGLElementArrayCache::Validate(uint32_t maxAllowed, size_t firstElement,
                                  size_t countElements, uint32_t* out_upperBound)
 {
-  SetUpperBound(out_upperBound, 0);
+  *out_upperBound = 0;
 
   // if maxAllowed is >= the max T value, then there is no way that a T index could be invalid
   uint32_t maxTSize = std::numeric_limits<T>::max();
   if (maxAllowed >= maxTSize) {
-    SetUpperBound(out_upperBound, maxTSize);
+    UpdateUpperBound(out_upperBound, maxTSize);
     return true;
   }
 
   T maxAllowedT(maxAllowed);
 
   // integer overflow must have been handled earlier, so we assert that maxAllowedT
   // is exactly the max allowed value.
   MOZ_ASSERT(uint32_t(maxAllowedT) == maxAllowed);
@@ -523,17 +512,17 @@ WebGLElementArrayCache::Validate(uint32_
 
   size_t lastElement = firstElement + countElements - 1;
 
   // fast exit path when the global maximum for the whole element array buffer
   // falls in the allowed range
   T globalMax = tree->GlobalMaximum();
   if (globalMax <= maxAllowedT)
   {
-    SetUpperBound(out_upperBound, globalMax);
+    UpdateUpperBound(out_upperBound, globalMax);
     return true;
   }
 
   const T* elements = Elements<T>();
 
   // before calling tree->Validate, we have to validate ourselves the boundaries of the elements span,
   // to round them to the nearest multiple of sElementsPerLeaf.
   size_t firstElementAdjustmentEnd = std::min(lastElement,
--- a/content/canvas/src/WebGLElementArrayCache.h
+++ b/content/canvas/src/WebGLElementArrayCache.h
@@ -31,31 +31,52 @@ struct WebGLElementArrayCacheTree;
  */
 class WebGLElementArrayCache {
 
 public:
   bool BufferData(const void* ptr, size_t byteLength);
   bool BufferSubData(size_t pos, const void* ptr, size_t updateByteSize);
 
   bool Validate(GLenum type, uint32_t maxAllowed, size_t first, size_t count,
-                uint32_t* out_upperBound = nullptr);
+                uint32_t* out_upperBound);
 
   template<typename T>
   T Element(size_t i) const { return Elements<T>()[i]; }
 
   WebGLElementArrayCache();
 
   ~WebGLElementArrayCache();
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   bool BeenUsedWithMultipleTypes() const;
 
 private:
 
+  /*
+   * Returns true if a drawElements call with the given parameters should succeed,
+   * false otherwise.
+   *
+   * In other words, this returns true if all entries in the element array at positions
+   *
+   *    first .. first+count-1
+   *
+   * are less than or equal to maxAllowed.
+   *
+   * Input parameters:
+   *   maxAllowed: maximum value to be allowed in the specificied portion of the element array.
+   *   first: start of the portion of the element array to consume.
+   *   count: number of entries from the element array to consume.
+   *
+   * Output parameter:
+   *   out_upperBound: upon success, is set to the actual maximum value in the specified range,
+   *                   which is then guaranteed to be less than or equal to maxAllowed.
+   *                   upon failure, is set to the first value in the specified range, that
+   *                   is greater than maxAllowed.
+   */
   template<typename T>
   bool Validate(uint32_t maxAllowed, size_t first, size_t count,
                 uint32_t* out_upperBound);
 
   template<typename T>
   const T* Elements() const { return reinterpret_cast<const T*>(mBytes.Elements()); }
   template<typename T>
   T* Elements() { return reinterpret_cast<T*>(mBytes.Elements()); }