There might be a bug in Bitblt copyloop

>
> 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@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@400 x 32bpp it works perfectly,
> but if i create a surface 800@800 x 32bpp it crashing..
> however if i use
> 801@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.

>
> 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@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@400 x 32bpp it works perfectly,
> but if i create a surface 800@800 x 32bpp it crashing..
> however if i use
> 801@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.
>