Bug 1333858 - Intersect should be fallible on overflow. - r=daoshengmu
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 27 Feb 2017 15:24:14 -0800
changeset 401749 a7590be606ab253180606a170ac7517f0116b9c5
parent 401748 bfa8d0189e506ad9130a04cc0290e4c3f357053f
child 401750 20e5a984fc18475b6c44062c6fa49f7c547298a0
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaoshengmu
bugs1333858
milestone55.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 1333858 - Intersect should be fallible on overflow. - r=daoshengmu 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
@@ -2220,35 +2220,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
@@ -2159,20 +2159,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
@@ -1551,28 +1551,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
@@ -1581,22 +1595,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);
 
@@ -1606,17 +1614,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
@@ -2006,34 +2006,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());
                 }