Re: #14650: wxImage::Scale causes program crash when new size is too large

[wxTrac]

#14650: wxImage::Scale causes program crash when new size is too large
-------------------------+-------------------------------------------------
Reporter: Jive | Owner:
Dadson |
Type: defect | Status: closed
Priority: normal | Milestone:
Component: wxMSW | Version: stable-latest
Resolution: invalid | Keywords: Scale wxCHECK_MSG image.cpp
| GetData crash
Blocked By: | Blocking:
Patch: 0 |
-------------------------+-------------------------------------------------

Old description:

> I have an app with a "+" button that the user can click on to rescale an
> image larger. If the user clicks it too many times, the program crashes
> in wxImage::Scale. The library code does not throw an exception that the
> application code could handle, nor does it return an image for which
> IsOk() returns false, either of which would be acceptable if documented.
> Instead it intentionally crashes the application, even in the Release
> version.
>
> What happens internally is that wxImage::Scale calls
> wxImage::ResampleNearest, which attempts to make a new image.
> ResampleNearest then calls GetData on the new image. GetData then invokes
> the macro wxCHECK_MSG, which crashes the application. Even if it did
> not, the next line in ResampleNearest would do so, because it is another
> call to wxCHECK_MSG.
>
> This is a recurring anti-pattern in image.cpp (at least). Where the
> function should either throw an exception or return a bad image that will
> return false for IsOk(), it instead crashes the application on purpose.
>
> I put the following work-around in my code:
>
> { // RAII block
> // Work around bug in wxWidgets. Claim as much
> space
> // as Scale will need. Hope the memory survives
> long enough
> // after we free it on exit from this block.
> wxImage new_image;
> new_image.Create( new_x, new_y, false );
> if(!new_image.IsOk()) {
> backout();
> return;
> }
> }

New description:

I have an app with a "+" button that the user can click on to rescale an
image larger. If the user clicks it too many times, the program crashes in
wxImage::Scale. The library code does not throw an exception that the
application code could handle, nor does it return an image for which
IsOk() returns false, either of which would be acceptable if documented.
Instead it intentionally crashes the application, even in the Release
version.

What happens internally is that wxImage::Scale calls
wxImage::ResampleNearest, which attempts to make a new image.
ResampleNearest then calls GetData on the new image. GetData then invokes
the macro wxCHECK_MSG, which crashes the application. Even if it did not,
the next line in ResampleNearest would do so, because it is another call
to wxCHECK_MSG.

This is a recurring anti-pattern in image.cpp (at least). Where the
function should either throw an exception or return a bad image that will
return false for IsOk(), it instead crashes the application on purpose.

I put the following work-around in my code:
{{{
{ // RAII block
// Work around bug in wxWidgets. Claim as much
space
// as Scale will need. Hope the memory survives
long enough
// after we free it on exit from this block.
wxImage new_image;
new_image.Create( new_x, new_y, false );
if(!new_image.IsOk()) {
backout();
return;
}
}
}}}t

--

Comment (by vadz):

FWIW this is now really an assert failure rather than crash, which I think
it could have been before, i.e. when this ticket was reported -- see
#18550.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14650#comment:5>