Bug 1647946 Comment 7 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I confirmed that the crash is indeed a 3-pixel-span going past the end of a 2MB buffer (from a 1024x512 RGBA TileCache).
I can't immediately fix it by allocating 16 extra bytes in the IOSurface, but I also can't make it crash right away with an obviously-wrong-size, so I may be doing it wrong.
Or, instead, not every allocation of a colortex is coming from that IOSurface?
Also, what about the depthtex?  I imagine reading past the end of a 4KB page could also crash.

An alternative is for each span < 8 pixels to run the fragment shader on a local rgba[16] and then only transfer the valid pixels?  I'm not sure how much perf. that might cost though.

```
diff --git a/gfx/2d/MacIOSurface.cpp b/gfx/2d/MacIOSurface.cpp
index 90d9a40ec2c8..89bbe4fd9885 100644
--- a/gfx/2d/MacIOSurface.cpp
+++ b/gfx/2d/MacIOSurface.cpp
@@ -36,7 +36,7 @@ already_AddRefed<MacIOSurface> MacIOSurface::CreateIOSurface(
   if (aContentsScaleFactor <= 0) return nullptr;
 
   CFMutableDictionaryRef props = ::CFDictionaryCreateMutable(
-      kCFAllocatorDefault, 4, &kCFTypeDictionaryKeyCallBacks,
+      kCFAllocatorDefault, 5, &kCFTypeDictionaryKeyCallBacks,
       &kCFTypeDictionaryValueCallBacks);
   if (!props) return nullptr;
 
@@ -47,6 +47,9 @@ already_AddRefed<MacIOSurface> MacIOSurface::CreateIOSurface(
   size_t intScaleFactor = ceil(aContentsScaleFactor);
   aWidth *= intScaleFactor;
   aHeight *= intScaleFactor;
+  size_t allocSize = aWidth * aHeight * bytesPerElem + /* SWGL SIMD slop */ 16;
+  // test if this does anything at all
+  allocSize = 1024;
   CFNumberRef cfWidth = ::CFNumberCreate(nullptr, kCFNumberSInt32Type, &aWidth);
   CFNumberRef cfHeight =
       ::CFNumberCreate(nullptr, kCFNumberSInt32Type, &aHeight);
@@ -59,6 +62,10 @@ already_AddRefed<MacIOSurface> MacIOSurface::CreateIOSurface(
   ::CFDictionaryAddValue(props, kIOSurfaceBytesPerElement, cfBytesPerElem);
   ::CFRelease(cfBytesPerElem);
   ::CFDictionaryAddValue(props, kIOSurfaceIsGlobal, kCFBooleanTrue);
+  CFNumberRef cfAllocSize =
+      ::CFNumberCreate(nullptr, kCFNumberSInt32Type, &allocSize);
+  ::CFDictionaryAddValue(props, kIOSurfaceAllocSize, cfAllocSize);
+  ::CFRelease(cfAllocSize);
 
   CFTypeRefPtr<IOSurfaceRef> surfaceRef =
       CFTypeRefPtr<IOSurfaceRef>::WrapUnderCreateRule(::IOSurfaceCreate(props));
```
I confirmed that the crash is indeed a 3-pixel-span going past the end of a 2MB buffer (from a 1024x512 RGBA TileCache).
I can't immediately fix it by allocating 16 extra bytes in the IOSurface, but I also can't make it crash right away with an obviously-wrong-size, so I may be doing it wrong.
Or, instead, not every allocation of a colortex is coming from that IOSurface?
Also, what about the depthtex?  I imagine reading past the end of a 4KB page could also crash.

An alternative is for each span < 8 pixels to run the fragment shader on a local rgba[16] and then only transfer the valid pixels?  I'm not sure how much perf. that might cost though.
Edit: or a variation on this idea is to shift the coordinates until all 4/8 pixels are within range, and adjust the mask accordingly.  Eg. for a span == 3, the mask disables the first pixel and writes the last 3, instead of enabling the first 3 and disabling the last pixel-which-is-out-of-bounds.

```
diff --git a/gfx/2d/MacIOSurface.cpp b/gfx/2d/MacIOSurface.cpp
index 90d9a40ec2c8..89bbe4fd9885 100644
--- a/gfx/2d/MacIOSurface.cpp
+++ b/gfx/2d/MacIOSurface.cpp
@@ -36,7 +36,7 @@ already_AddRefed<MacIOSurface> MacIOSurface::CreateIOSurface(
   if (aContentsScaleFactor <= 0) return nullptr;
 
   CFMutableDictionaryRef props = ::CFDictionaryCreateMutable(
-      kCFAllocatorDefault, 4, &kCFTypeDictionaryKeyCallBacks,
+      kCFAllocatorDefault, 5, &kCFTypeDictionaryKeyCallBacks,
       &kCFTypeDictionaryValueCallBacks);
   if (!props) return nullptr;
 
@@ -47,6 +47,9 @@ already_AddRefed<MacIOSurface> MacIOSurface::CreateIOSurface(
   size_t intScaleFactor = ceil(aContentsScaleFactor);
   aWidth *= intScaleFactor;
   aHeight *= intScaleFactor;
+  size_t allocSize = aWidth * aHeight * bytesPerElem + /* SWGL SIMD slop */ 16;
+  // test if this does anything at all
+  allocSize = 1024;
   CFNumberRef cfWidth = ::CFNumberCreate(nullptr, kCFNumberSInt32Type, &aWidth);
   CFNumberRef cfHeight =
       ::CFNumberCreate(nullptr, kCFNumberSInt32Type, &aHeight);
@@ -59,6 +62,10 @@ already_AddRefed<MacIOSurface> MacIOSurface::CreateIOSurface(
   ::CFDictionaryAddValue(props, kIOSurfaceBytesPerElement, cfBytesPerElem);
   ::CFRelease(cfBytesPerElem);
   ::CFDictionaryAddValue(props, kIOSurfaceIsGlobal, kCFBooleanTrue);
+  CFNumberRef cfAllocSize =
+      ::CFNumberCreate(nullptr, kCFNumberSInt32Type, &allocSize);
+  ::CFDictionaryAddValue(props, kIOSurfaceAllocSize, cfAllocSize);
+  ::CFRelease(cfAllocSize);
 
   CFTypeRefPtr<IOSurfaceRef> surfaceRef =
       CFTypeRefPtr<IOSurfaceRef>::WrapUnderCreateRule(::IOSurfaceCreate(props));
```

Back to Bug 1647946 Comment 7