mismatched free/delete in typeregister.h

[David Nolden]

2011/5/28 Christoph Bartoschek <bartoschek at gmx.de>:
> Hi
>
> abstracttype.h:254 has the line
>
> return *new (new char[size]) DataType(rhs);
>
> Here one first allocates size bytes with new[]. Into the allocated space a
> new DataType object is created by using placement new. Placement new gets a
> pointer to a memory location and creates the object there instead of
> fetching memory internally. The syntax is:
>
> new (pointer) T();
>
> This is also used in typeregister.h. In line 99 however the memory is freed
> with
>
> delete temp;
>
> Valgrind complains because delete cannot be used for memory that was
> allocated with new []. Instead one has to use delete []. Therefore the
> correct code in line 99 should be:
>
> delete [] reinterpret_cast<char *>(temp);

This is right, it is deallocated like this in other places, and should
be deallocated here in the same way.

> The cast is necessary because the memory was allocated as a char array.
>
> There is one problem with the code: The destructor of the DataType is not
> called. Therefore the solution is:
>
> temp->~Data();
>
> delete [] reinterpret_cast<char *>(temp);

Yes, this call should be added too.

> In my opinion it is also ugly to return a reference in copyDataDirectly. It
> does not make clear that one has to delete the object returned.

It might be a bit ugly, but all the places where this is called take a
reference, and it's better to be ugly in one place, than being ugly in
50 places (eg. one additional "*" in every copy-constructor of a
type-class). We could change the specific type-class constructors to
accept pointers instead of references, but that would require changes
in really many places, including all language plugins.

> It is also extremely dangerous to create objects in memory that has been
> fetched wit h new char []. The alignment might not fit to the object that is
> later constructed in it and one gets really hard to debug errors.

This is something we might investigate, but we never had problems with
it, and most probably even the start a "char*" array is aligned
according to the architecture (eg. at a 32-bit address).

Greetings, David