#813 Bug in Delete command

[Sockla]

When compiling a Delete command referring to a global variable or dereferenced pointer, this variable or pointer is accessed twice -- once for calling the destructor and once for freeing the memory. If the destructor changes the value of the global variable or pointer, the wrong memory block will be freed and you will get a segfault/invalid memory access.

This bug is very hard to track, since it will only become apparent if you allocate memory after this erroneous Delete and, by bad luck, get the pointer to a memory block you do not think you have freed (but FB did for you).

Example code:

Type Node
    pParent As Node Pointer

    Declare Constructor()
    Declare Destructor()
End Type

Dim Shared mpCurrentNode As Node Pointer

Constructor Node()
    pParent = mpCurrentNode
    mpCurrentNode = @This
End Constructor
Destructor Node()
    If mpCurrentNode = @This Then
        mpCurrentNode = pParent
    End If
End Destructor

Print Hex$(New Node)
Delete mpCurrentNode

Generated assembly (with "fbc -RR Example.bas"):

...
cmp dword ptr [_MPCURRENTNODE], 0
je .Lt_000C
push dword ptr [_MPCURRENTNODE]
call __ZN4NODED1Ev
add esp, 4
; Destructor has changed the value in [_MPCURRENTNODE]
push dword ptr [_MPCURRENTNODE]
call _free
...

What this should look like is

...
mov ebx, dword ptr [_MPCURRENTNODE]]
test ebx, ebx
je .Lt_000C
push ebx
call __ZN4NODED1Ev
add esp, 4
; Destructor did not change ebx
push ebx
call _free
...

[dkl]

Yea, the expression should be copied to a temp var, so we can be sure to work on the same value during the whole Delete (check against null, destructor call, free() call).

I'm afraid the same problem may exist for example with Select Case or With blocks, at least if the expressions contain function calls (the only way in FB to change variables during an expression).

In case of Delete though, it doesn't matter whether the expression has side-effects, as the destructor call may already cause side-effects.

This should be pretty easy to fix though.