Bug 1333858 - Intersect should be fallible on overflow. r=daoshengmu, a=gchang
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 27 Feb 2017 15:24:14 -0800
changeset 379472 9e3e0759726b21e37afe2e55acf53f13a94f3918
parent 379471 3abcaea76fa30d9b568cf8b664d7ecf8f55f4abd
child 379473 35bf6122572ace2295ed0d0a13c3ba6edd0a3e40
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaoshengmu, gchang
bugs1333858
milestone53.0
Bug 1333858 - Intersect should be fallible on overflow. r=daoshengmu, a=gchang MozReview-Commit-ID: 6lmIKKyXXah
dom/canvas/WebGLContext.cpp
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextGL.cpp
dom/canvas/WebGLTextureUpload.cpp
--- a/dom/canvas/WebGLContext.cpp
+++ b/dom/canvas/WebGLContext.cpp
@@ -2202,35 +2202,57 @@ ScopedLazyBind::UnwrapImpl()
 {
     if (mTarget) {
         mGL->fBindBuffer(mTarget, 0);
     }
 }
 
 ////////////////////////////////////////
 
-void
-Intersect(uint32_t srcSize, int32_t dstStartInSrc, uint32_t dstSize,
-          uint32_t* const out_intStartInSrc, uint32_t* const out_intStartInDst,
-          uint32_t* const out_intSize)
+bool
+Intersect(const int32_t srcSize, const int32_t read0, const int32_t readSize,
+          int32_t* const out_intRead0, int32_t* const out_intWrite0,
+          int32_t* const out_intSize)
 {
-    // Only >0 if dstStartInSrc is >0:
-    // 0  3          // src coords
-    // |  [========] // dst box
-    // ^--^
-    *out_intStartInSrc = std::max<int32_t>(0, dstStartInSrc);
-
-    // Only >0 if dstStartInSrc is <0:
-    //-6     0       // src coords
-    // [=====|==]    // dst box
-    // ^-----^
-    *out_intStartInDst = std::max<int32_t>(0, 0 - dstStartInSrc);
-
-    int32_t intEndInSrc = std::min<int32_t>(srcSize, dstStartInSrc + dstSize);
-    *out_intSize = std::max<int32_t>(0, intEndInSrc - *out_intStartInSrc);
+    MOZ_ASSERT(srcSize >= 0);
+    MOZ_ASSERT(readSize >= 0);
+    const auto read1 = int64_t(read0) + readSize;
+
+    int32_t intRead0 = read0; // Clearly doesn't need validation.
+    int64_t intWrite0 = 0;
+    int64_t intSize = readSize;
+
+    if (read1 <= 0 || read0 >= srcSize) {
+        // Disjoint ranges.
+        intSize = 0;
+    } else {
+        if (read0 < 0) {
+            const auto diff = int64_t(0) - read0;
+            MOZ_ASSERT(diff >= 0);
+            intRead0 = 0;
+            intWrite0 = diff;
+            intSize -= diff;
+        }
+        if (read1 > srcSize) {
+            const auto diff = int64_t(read1) - srcSize;
+            MOZ_ASSERT(diff >= 0);
+            intSize -= diff;
+        }
+
+        if (!CheckedInt<int32_t>(intWrite0).isValid() ||
+            !CheckedInt<int32_t>(intSize).isValid())
+        {
+            return false;
+        }
+    }
+
+    *out_intRead0 = intRead0;
+    *out_intWrite0 = intWrite0;
+    *out_intSize = intSize;
+    return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 CheckedUint32
 WebGLContext::GetUnpackSize(bool isFunc3D, uint32_t width, uint32_t height,
                             uint32_t depth, uint8_t bytesPerPixel)
 {
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -2188,20 +2188,19 @@ public:
     ScopedLazyBind(gl::GLContext* gl, GLenum target, const WebGLBuffer* buf);
 
 private:
     void UnwrapImpl();
 };
 
 ////
 
-void
-Intersect(uint32_t srcSize, int32_t dstStartInSrc, uint32_t dstSize,
-          uint32_t* const out_intStartInSrc, uint32_t* const out_intStartInDst,
-          uint32_t* const out_intSize);
+bool
+Intersect(int32_t srcSize, int32_t read0, int32_t readSize, int32_t* out_intRead0,
+          int32_t* out_intWrite0, int32_t* out_intSize);
 
 ////
 
 void
 ImplCycleCollectionTraverse(nsCycleCollectionTraversalCallback& callback,
                             const std::vector<IndexedBufferBinding>& field,
                             const char* name, uint32_t flags = 0);
 
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -1548,28 +1548,42 @@ WebGLContext::ReadPixelsImpl(GLint x, GL
         return;
     }
 
     if (bytesNeeded > dataLen) {
         ErrorInvalidOperation("readPixels: buffer too small");
         return;
     }
 
+    ////
+
+    int32_t readX, readY;
+    int32_t writeX, writeY;
+    int32_t rwWidth, rwHeight;
+    if (!Intersect(srcWidth, x, width, &readX, &writeX, &rwWidth) ||
+        !Intersect(srcHeight, y, height, &readY, &writeY, &rwHeight))
+    {
+        ErrorOutOfMemory("readPixels: Bad subrect selection.");
+        return;
+    }
+
     ////////////////
     // Now that the errors are out of the way, on to actually reading!
 
     OnBeforeReadCall();
 
-    uint32_t readX, readY;
-    uint32_t writeX, writeY;
-    uint32_t rwWidth, rwHeight;
-    Intersect(srcWidth, x, width, &readX, &writeX, &rwWidth);
-    Intersect(srcHeight, y, height, &readY, &writeY, &rwHeight);
-
-    if (rwWidth == uint32_t(width) && rwHeight == uint32_t(height)) {
+    if (!rwWidth || !rwHeight) {
+        // Disjoint rects, so we're done already.
+        DummyReadFramebufferOperation("readPixels");
+        return;
+    }
+
+    if (uint32_t(rwWidth) == width &&
+        uint32_t(rwHeight) == height)
+    {
         DoReadPixelsAndConvert(srcFormat->format, x, y, width, height, packFormat,
                                packType, dest, dataLen, rowStride);
         return;
     }
 
     // Read request contains out-of-bounds pixels. Unfortunately:
     // GLES 3.0.4 p194 "Obtaining Pixels from the Framebuffer":
     // "If any of these pixels lies outside of the window allocated to the current GL
@@ -1578,22 +1592,16 @@ WebGLContext::ReadPixelsImpl(GLint x, GL
 
     // This is a slow-path, so warn people away!
     GenerateWarning("readPixels: Out-of-bounds reads with readPixels are deprecated, and"
                     " may be slow.");
 
     ////////////////////////////////////
     // Read only the in-bounds pixels.
 
-    if (!rwWidth || !rwHeight) {
-        // There aren't any, so we're 'done'.
-        DummyReadFramebufferOperation("readPixels");
-        return;
-    }
-
     if (IsWebGL2()) {
         if (!mPixelStore_PackRowLength) {
             gl->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH,
                              mPixelStore_PackSkipPixels + width);
         }
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_PIXELS, mPixelStore_PackSkipPixels + writeX);
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_ROWS, mPixelStore_PackSkipRows + writeY);
 
@@ -1603,17 +1611,17 @@ WebGLContext::ReadPixelsImpl(GLint x, GL
         gl->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH, mPixelStore_PackRowLength);
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_PIXELS, mPixelStore_PackSkipPixels);
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_ROWS, mPixelStore_PackSkipRows);
     } else {
         // I *did* say "hilariously slow".
 
         uint8_t* row = (uint8_t*)dest + writeX * bytesPerPixel;
         row += writeY * rowStride;
-        for (uint32_t j = 0; j < rwHeight; j++) {
+        for (uint32_t j = 0; j < uint32_t(rwHeight); j++) {
             DoReadPixelsAndConvert(srcFormat->format, readX, readY+j, rwWidth, 1,
                                    packFormat, packType, row, dataLen, rowStride);
             row += rowStride;
         }
     }
 }
 
 void
--- a/dom/canvas/WebGLTextureUpload.cpp
+++ b/dom/canvas/WebGLTextureUpload.cpp
@@ -2005,34 +2005,38 @@ DoCopyTexOrSubImage(WebGLContext* webgl,
                     GLint xOffset, GLint yOffset, GLint zOffset,
                     uint32_t dstWidth, uint32_t dstHeight,
                     const webgl::FormatUsageInfo* dstUsage)
 {
     const auto& gl = webgl->gl;
 
     ////
 
-    uint32_t readX, readY;
-    uint32_t writeX, writeY;
-    uint32_t rwWidth, rwHeight;
-    Intersect(srcTotalWidth, xWithinSrc, dstWidth, &readX, &writeX, &rwWidth);
-    Intersect(srcTotalHeight, yWithinSrc, dstHeight, &readY, &writeY, &rwHeight);
+    int32_t readX, readY;
+    int32_t writeX, writeY;
+    int32_t rwWidth, rwHeight;
+    if (!Intersect(srcTotalWidth, xWithinSrc, dstWidth, &readX, &writeX, &rwWidth) ||
+        !Intersect(srcTotalHeight, yWithinSrc, dstHeight, &readY, &writeY, &rwHeight))
+    {
+        webgl->ErrorOutOfMemory("%s: Bad subrect selection.", funcName);
+        return false;
+    }
 
     writeX += xOffset;
     writeY += yOffset;
 
     ////
 
     GLenum error = 0;
     do {
         const auto& idealUnpack = dstUsage->idealUnpack;
         if (!isSubImage) {
             UniqueBuffer buffer;
 
-            if (rwWidth != dstWidth || rwHeight != dstHeight) {
+            if (uint32_t(rwWidth) != dstWidth || uint32_t(rwHeight) != dstHeight) {
                 const auto& pi = idealUnpack->ToPacking();
                 CheckedUint32 byteCount = BytesPerPixel(pi);
                 byteCount *= dstWidth;
                 byteCount *= dstHeight;
 
                 if (byteCount.isValid()) {
                     buffer = calloc(1, byteCount.value());
                 }