Bug 1333858 - Intersect should be fallible on overflow. r=daoshengmu, a=jcristau FIREFOX_45_9_0esr_BUILD3 FIREFOX_45_9_0esr_RELEASE
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 27 Feb 2017 15:24:14 -0800
changeset 312884 413dc18f25c816c615c323182b6847b1cca5d898
parent 312883 c6ef3921ef6070d1bdd41c4c63d95158d8ae0de9
child 312885 2988a3b486c5156bc6394a7d33ef3796caacfe03
push id526
push userryanvm@gmail.com
push dateTue, 11 Apr 2017 18:52:23 +0000
treeherdermozilla-esr45@413dc18f25c8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaoshengmu, jcristau
bugs1333858
milestone45.9.0
Bug 1333858 - Intersect should be fallible on overflow. r=daoshengmu, a=jcristau 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
@@ -1869,35 +1869,57 @@ ScopedUnpackReset::UnwrapImpl()
         }
 
         mGL->fBindBuffer(LOCAL_GL_PIXEL_UNPACK_BUFFER, pbo);
     }
 }
 
 ////////////////////////////////////////
 
-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);
+    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;
 
-    // Only >0 if dstStartInSrc is <0:
-    //-6     0       // src coords
-    // [=====|==]    // dst box
-    // ^-----^
-    *out_intStartInDst = std::max<int32_t>(0, 0 - dstStartInSrc);
+    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;
+        }
 
-    int32_t intEndInSrc = std::min<int32_t>(srcSize, dstStartInSrc + dstSize);
-    *out_intSize = std::max<int32_t>(0, intEndInSrc - *out_intStartInSrc);
+        if (!CheckedInt<int32_t>(intWrite0).isValid() ||
+            !CheckedInt<int32_t>(intSize).isValid())
+        {
+            return false;
+        }
+    }
+
+    *out_intRead0 = intRead0;
+    *out_intWrite0 = intWrite0;
+    *out_intSize = intSize;
+    return true;
 }
 
 static bool
 ZeroTexImageWithClear(WebGLContext* webgl, GLContext* gl, TexImageTarget target,
                       GLuint tex, uint32_t level, const webgl::FormatUsageInfo* usage,
                       uint32_t width, uint32_t height)
 {
     MOZ_ASSERT(gl->IsCurrent());
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -1744,20 +1744,19 @@ protected:
     void UnwrapImpl();
 };
 
 void
 ComputeLengthAndData(const dom::ArrayBufferViewOrSharedArrayBufferView& view,
                      void** const out_data, size_t* const out_length,
                      js::Scalar::Type* const out_type);
 
-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);
 
 bool
 ZeroTextureData(WebGLContext* webgl, const char* funcName, bool respecifyTexture,
                 GLuint tex, TexImageTarget target, uint32_t level,
                 const webgl::FormatUsageInfo* usage, uint32_t xOffset, uint32_t yOffset,
                 uint32_t zOffset, uint32_t width, uint32_t height, uint32_t depth);
 
 } // namespace mozilla
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -1510,26 +1510,37 @@ WebGLContext::ReadPixels(GLint x, GLint 
     }
 
     const bool mainMatches = (format == mainReadFormat && type == mainReadType);
     const bool auxMatches = (format == auxReadFormat && type == auxReadType);
     const bool isValid = mainMatches || auxMatches;
     if (!isValid)
         return ErrorInvalidOperation("readPixels: Invalid format/type pair");
 
+    ////
+
+    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!
 
-    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)) {
-        // Warning: Possibly shared memory.  See bug 1225033.
+    if (!rwWidth || !rwHeight) {
+        // Disjoint rects, so we're done already.
+        DummyReadFramebufferOperation("readPixels");
+        return;
+    }
+
+    if (rwWidth == width && rwHeight == height) {
         DoReadPixelsAndConvert(x, y, width, height, format, type, data, auxReadFormat,
                                auxReadType);
         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
@@ -1566,22 +1577,16 @@ WebGLContext::ReadPixels(GLint x, GLint 
         }
     } else {
         std::memset(data, 0, bytesNeeded.value());
     }
 
     ////////////////////////////////////
     // 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, width);
         }
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_PIXELS, mPixelStore_PackSkipPixels + writeX);
         gl->fPixelStorei(LOCAL_GL_PACK_SKIP_ROWS, mPixelStore_PackSkipRows + writeY);
 
         DoReadPixelsAndConvert(readX, readY, rwWidth, rwHeight, format, type, data,
@@ -1590,17 +1595,17 @@ WebGLContext::ReadPixels(GLint x, GLint 
         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*)data + startOffset.value() + writeX * bytesPerPixel;
         row += writeY * rowStride.value();
-        for (uint32_t j = 0; j < rwHeight; j++) {
+        for (uint32_t j = 0; j < uint32_t(rwHeight); j++) {
             DoReadPixelsAndConvert(readX, readY+j, rwWidth, 1, format, type, row,
                                    auxReadFormat, auxReadType);
             row += rowStride.value();
         }
     }
 
     // if we're reading alpha, we may need to do fixup.  Note that we don't allow
     // GL_ALPHA to readpixels currently, but we had the code written for it already.
--- a/dom/canvas/WebGLTextureUpload.cpp
+++ b/dom/canvas/WebGLTextureUpload.cpp
@@ -1757,24 +1757,32 @@ WebGLTexture::CopyTexImage2D(TexImageTar
     // Do the thing!
 
     gl::GLContext* gl = mContext->gl;
     gl->MakeCurrent();
 
     ScopedCopyTexImageSource maybeSwizzle(mContext, funcName, srcWidth, srcHeight,
                                           srcFormat, dstUsage);
 
-    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);
+    ////
+
+    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))
+    {
+        mContext->ErrorOutOfMemory("%s: Bad subrect selection.", funcName);
+        return;
+    }
+
+    ////
 
     GLenum error;
-    if (rwWidth == uint32_t(width) && rwHeight == uint32_t(height)) {
+    if (rwWidth == width && rwHeight == height) {
         error = DoCopyTexImage2D(gl, target, level, internalFormat, x, y, width, height,
                                  border);
     } else {
         // 1. Zero the texture data.
         // 2. CopyTexSubImage the subrect.
 
         const bool respecifyTexture = true;
         const uint8_t zOffset = 0;
@@ -1866,21 +1874,29 @@ WebGLTexture::CopyTexSubImage(const char
     ////////////////////////////////////
     // Do the thing!
 
     mContext->gl->MakeCurrent();
 
     ScopedCopyTexImageSource maybeSwizzle(mContext, funcName, srcWidth, srcHeight,
                                           srcFormat, dstUsage);
 
-    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);
+    ////
+
+    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))
+    {
+        mContext->ErrorOutOfMemory("%s: Bad subrect selection.", funcName);
+        return;
+    }
+
+    ////
 
     if (!rwWidth || !rwHeight) {
         // There aren't any, so we're 'done'.
         mContext->DummyReadFramebufferOperation(funcName);
         return;
     }
 
     bool uploadWillInitialize;