[Vm-dev] There might be a bug in Bitblt copyloop

[John M McIntosh]

Ah memories
I found this a decade back
Maybe I can find the email that talked about it.  Never was able to propose a solution the bitblt loop that looks ahead 4 bytes is tricky. 

I think for that case it was letting QuickDraw allocate the buffer for QuickTime to draw in.  

Sent from my iPhone

On Apr 4, 2012, at 10:24 AM, Igor Stasenko <siguctua at gmail.com> wrote:

> 
> The way how i discovered  it is a bit complicated, so i need to
> explain it in detail.
> 
> I am using a SurfacePlugin for gluing the cairo image surface (which
> is holds a bitmap bits somewhere in C heap) with bitblt.
> 
> For that, you need to register a surface with surface plugin and
> provide a callbacks by filling following structure:
> 
> typedef struct sqSurfaceDispatch {
>    /* Version information. Must be provided by the client
>       so the surface manager can check if certain operations
>       are supported. */
>    int majorVersion;
>    int minorVersion;
> 
>    /* Version 1.0 */
>    fn_getSurfaceFormat getSurfaceFormat;
>    fn_lockSurface lockSurface;
>    fn_unlockSurface unlockSurface;
>    fn_showSurface showSurface;
> } sqSurfaceDispatch;
> 
> 
> The function to register a surface is provided by a surface plugin is following:
> 
>    int ioRegisterSurface(int surfaceHandle, sqSurfaceDispatch *fn, int
> *surfaceID);
>        Register a new surface with the given handle and
>        the set of surface functions. The new ID is returned
>        in surfaceID. Returns true if successful, false
>        otherwise.
> 
> you passing a handle (which in my case is cairo surface handle, the
> filled struct and as result you get a surfaceID.
> 
> A surface ID is one which is used directly in language side, by
> setting a Form's bits instance variable to smallinteger with id value.
> 
> I.e. imagine that i call ioRegisterSurface:
> 
> int id = 0;
> ioRegisterSurface(myHandle, myDispatch, &id);
> 
> and then in language side i creating a Form with same id:
> 
> externalForm := Form extent: (self width at self height) depth: 32 bits: id .
> 
> Now you can use this form for any bitblt operations.. and when it
> happens, a bitblt detects that bits are not an instance of Bitmap,
> but a smallinteger.. and so instead of working with bitmap bits
> directly it using a sqSurfaceDispatch structure, which we provided
> before
> to access surface bits.
> 
> So, the most important one is getSurfaceFormat function which having
> following form:
> 
> int getSurfaceFormat(int handle, int* width, int* height, int* depth,
> int* isMSB);
>        Return general information about the OS drawing surface.
>        Return true if successful, false otherwise.
> 
>        The returned values describe the basic properties such as
>        width, height, depth and LSB vs. MSB pixels.
> 
> 
> And now finally about the bug itself.
> I implemented all these functions (needed for sqSurfaceDispatch) using
> nativeboost and they worked well, but for
> surfaces of certain dimensions i got VM crashes.
> 
> For instance, if i create a surface 400 at 400 x 32bpp it works perfectly,
> but if i create a surface 800 at 800 x 32bpp it crashing..
> however if i use
> 801 at 801, it works..
> 
> after some debugging i discovered that bitblt tries to read bytes past
> the actual buffer size, i.e.
> if in getSurfaceFormat() i reporting
> width=800
> height= 800
> depth= 32
> msb = true
> 
> i see that bitblt reading bytes past
> buffer start address + (800*800*4) bytes
> 
> (there is additional function involved, which answers the pitch for
> the scanline:
> int lockSurface(int handle, int *pitch, int x, int y, int w, int h);
> 
> so it is actually not (800*800*4)
> but
> 
> pitch * (height = 800)
> 
> anyways...
> i placed a workaround by reporting 1 pixel less height, so if i have 800*800
> bitmap, i reporting 800*799
> 
> Why we don't have issues with this in image? Well i think because you
> can safely read couple more bytes past any object in object memory
> and be 100% sure that you won't have SEGFAULT.
> In contrast, if you working with buffers allocated on C heap, and your
> buffer size is close to page size alignment (4kb) it could be quite
> easy to
> hit SEGFAULT by reading even 1 byte past the buffer size.
> 
> So, i suspect there is a bug in one of the copy loop bounds, something
> as subtle as:
> 
> [0 ... count]
> vs
> [0 .. count)
> 
> i.e. it loops over
> 0 to: count
> instead of:
> 0 to: count -1
> 
> i would appreciate if people with better expertise about bitblt will
> comment what i have found.
> 
> --
> Best regards,
> Igor Stasenko.