Galleondragon/qb64

Glitch/Bug between _FREEIMAGE/_COPYIMAGE/_MEM (Critical Error #308)

Closed this issue · 1 comments

As reported by RhoSigma at https://www.qb64.org/forum/index.php?topic=350.msg2335#msg2335

Hi QB64 Team,

It seems I found a longstanding glitch/bug between _FREEIMAGE, _COPYIMAGE and the _MEM functions (probably _MEMIMAGE), which leads to an Critical Error #308 (memory has been freed).

The error happens, whenever _COPYIMAGE does use a handle, which was already used by another image earlier but which got freed in the meantime and hence is available for re-use. I could verify this error starting from the old SDL 0.954 version through different GL builts in between up to the latest GL development built available from QB64.org.

I've made a small program to show the issue. You also need the attached image or in alternative you may adjust the _LOADIMAGE line (3) to use any image on your system (must be at least 1024x768).

To get an understanding of the workflow, first read the comments right above the ModifyBrightness&() function (lines 37-40), then the comments in the main module (lines 14-26).

If you run the program as is, it will show the loaded image and wait for a keypress. When you press a key, it will do the _FREIMAGE and then the final ModifyBrightness&() call and will error out with #308 immediately. After that try to comment the _FREEIMAGE line !!OR!! commenting the _COPYIMAGE in ModifyBrightness&() and instead uncommenting the _NEWIMAGE/_PUTIMAGE pair (lines 63-67) and try again. It will work in both cases, a further keypress will then end the program.

'--- 1st show the original image ---
_TITLE "The original Image, press any key..."
ihan& = _LOADIMAGE("BeachGirl.jpg", 32) 'any image file (at least 1024x768)
IF ihan& >= -1 THEN ERROR 53 'file not found
SCREEN ihan&

'--- now make a darken cascade ---
_TITLE "Darken Cascade, press any key..."
dark& = ModifyBrightness&(ihan&, -0.15, 250, 70, 820, -1)
darker& = ModifyBrightness&(dark&, -0.15, 275, 95, 795, -1)
COLOR _RGB32(255, 255, 200): PRINT "Used Handle: "; dark&: SLEEP

'--------------------------------------------------------------------
'--- With activated _FREEIMAGE the freed handle is re-used by the
'--- next _COPYIMAGE call done in ModifyBrightness&() called right
'--- below the _FREEIMAGE, but the handle in question obviously gets
'--- not correctly initialized by _COPYIMAGE, as the _MEM function(s)
'--- used later to iterate through the image do fail to work on it
'--- (Critical Error #308 (memory was freed)).
'-----
'--- If otherwise the _FREEIMAGE call is commented out, then it works
'--- with _COPYIMAGE, as a new clean handle must be used instead.
'-----
'--- In alternative it also works with activated _FREEIMAGE, but change
'--- the _COPYIMAGE in FUNCTION ModifyBrightness&() into a combination
'--- of _NEWIMAGE and _PUTIMAGE instead to make the image copy.
'--------------------------------------------------------------------
_FREEIMAGE dark&
darkest& = ModifyBrightness&(darker&, -0.15, 300, 120, 770, -1)
'--------------------------------------------------------------------

SCREEN darkest&
COLOR _RGB32(255, 255, 200): PRINT "Used Handle: "; darkest&: SLEEP
'--- done ---
SYSTEM

'This function is a cutout from 'QB64Library\IMG-Support\imageprocess.bm',
'which is part of my Libraries Collection. It takes a source image handle
'and returns a modified image in a new handle usually made with _COPYIMAGE.
'This seems to fail, if for the copy an old (freed) handle is re-used.
'---------------------------------------------------------------------
'NAME
'   ModifyBrightness -- Brightness adjustment, keep alpha
'
'TEMPLATE
'   newImg& = ModifyBrightness& (shan&, change#, minX%, minY%, maxX%, maxY%)
'
'   change# -- must be between -1.0 and 1.0  (clipped)
'
' DESCRIPTION
'   This changes the general brightness of the image. The smaller the
'   value is the darker the image becomes.  0.0 means no change.
'   The alpha channel of the source image is retained and copied 1:1.
'---------------------------------------------------------------------
FUNCTION ModifyBrightness& (shan&, change#, minX%, minY%, maxX%, maxY%)
    ModifyBrightness& = -1 'so far return invalid handle
    IF shan& < -1 THEN
        IF _PIXELSIZE(shan&) = 4 THEN
            '--- get source image size and a copy of the image ---
            swid% = _WIDTH(shan&): shei% = _HEIGHT(shan&)

            '--------------------------------------------------
            '--- This works only, if a new handle must be used,
            nhan& = _COPYIMAGE(shan&)
            '--- and this works for both re-used and new handles
            'nhan& = _NEWIMAGE(swid%, shei%, 32)
            '_PUTIMAGE ,shan&, nhan&
            '--------------------------------------------------

            '--- check selected processing area ---
            IF minX% < 0 OR minX% >= swid% THEN minX% = 0
            IF maxX% < 0 OR maxX% >= swid% THEN maxX% = swid% - 1
            IF minY% < 0 OR minY% >= shei% THEN minY% = 0
            IF maxY% < 0 OR maxY% >= shei% THEN maxY% = shei% - 1
            '--- process copied image ---
            IF nhan& < -1 THEN
                '--- build histogram transformation table ---
                IF change# < -1.0# THEN change# = -1.0
                IF change# > 1.0# THEN change# = 1.0
                REDIM hist%(255)
                FOR i% = 0 TO 255
                    v% = FIX(i% * (change# + 1.0#))
                    IF v% < 0 THEN v% = 0: ELSE IF v% > 255 THEN v% = 255
                    hist%(i%) = v%
                NEXT i%
                '--- for speed we do direct memory access ---
                DIM nbuf AS _MEM: nbuf = _MEMIMAGE(nhan&)
                '--- iterate through pixels ---
                FOR y% = minY% TO maxY%
                    noff%& = nbuf.OFFSET + (y% * swid% * 4) + (minX% * 4)
                    FOR x% = minX% TO maxX%
                        '--- get pixel ARGB value from source ---
                        _MEMGET nbuf, noff%&, orgb~&
                        '--- modify pixel components ---
                        newA% = _ALPHA32(orgb~&) 'ignored (put through)
                        newR% = hist%(_RED32(orgb~&))
                        newG% = hist%(_GREEN32(orgb~&))
                        newB% = hist%(_BLUE32(orgb~&))
                        '--- put new pixel ARGB value to dest ---
                        nrgb~& = _RGBA32(newR%, newG%, newB%, newA%)
                        _MEMPUT nbuf, noff%&, nrgb~&
                        '--- set next pixel offset ---
                        noff%& = noff%& + 4
                    NEXT x%
                NEXT y%
                '--- cleanup ---
                _MEMFREE nbuf
                ERASE hist%
                '--- set result ---
                ModifyBrightness& = nhan&
            END IF
        END IF
    END IF
END FUNCTION

Simpler sample to detect same error (SMcNeill, same thread as indicated above):

DIM m AS _MEM

ihan& = _SCREENIMAGE
IF ihan& >= -1 THEN ERROR 53: END 'file not found
SCREEN ihan&

dark& = _COPYIMAGE(ihan&) 'make a copy
m = _MEMIMAGE(dark&)
PRINT _MEMGET(m, m.OFFSET, _UNSIGNED _BYTE) 'just print the first byte of info to show we can access memory from the image
_MEMFREE m


darker& = _COPYIMAGE(dark&) 'make a copy of the copy

_FREEIMAGE dark& 'free the first copy

m = _MEMIMAGE(darker&)
PRINT _MEMGET(m, m.OFFSET, _UNSIGNED _BYTE)
_MEMFREE m


END