Thread overview
[dmd-internals] DMD internal memory management issue
Dec 08, 2009
Denis
Dec 08, 2009
Walter Bright
Dec 09, 2009
Denis
Dec 09, 2009
Walter Bright
December 08, 2009
I came across one memory allocation related problem in DMD and decided to make a post about it in this list.

It looks like at some point Walter decided to introduce some kind of a
garbage collector by wrapping all allocations with
mem.malloc()/mem.free()/mem.mark()/mem.fullcollect() etc. Problem is,
the feature is somewhat half-implemented (not the Garbage collection
itself, but moving all the memory allocations to a proxy object): half
of DMD uses mem.malloc, and the other half uses raw ::malloc. In some
cases it even allocates using mem.malloc() but deallocates with
::free().

I came across this issue while porting code to D.

Naturally, I forward mem.malloc() to GC.malloc() so that memory gets
garbage collected. But in some cases memory gets allocated in
front-end using mem.malloc(), but deallocated in backend using ::free.
That's where my application either hangs or causes Access Violations.

I strongly believe this is a (logical) bug, but a minor one. It doesn't showup in DMD so it's somewhat pointless to put into bugzilla. Nevertheless, I believe it would be nice to get this bug fixed.

So far I only noticed it in one place, but I suspect it might appear in other areas, too.

Here it is:

s2ir.c, FuncDeclaration::toObj() (handling switch statement)

targ_llong *pu = (targ_llong *) mem.malloc(sizeof(*pu) * (numcases +
1)); // allocating using mem.malloc()
mystate.switchBlock.BS.Bswitch = pu;

backend/blockopt.c, void block_free(block *b) (memory cleanup)

switch (b.BC)
 {
    case BCswitch:
    case BCifthen:
    case BCjmptab:
        MEM_PH_FREE(b.BS.Bswitch); // freeing using ::free()
        break;

---
Denis Koroskin
December 08, 2009
Good catch, fixed with changeset 288.

Denis wrote:
> I came across one memory allocation related problem in DMD and decided to make a post about it in this list.
>
> 
December 09, 2009
On Tue, Dec 8, 2009 at 10:49 PM, Walter Bright <walter at digitalmars.com> wrote:
> Good catch, fixed with changeset 288.
>
> Denis wrote:
>>
>> I came across one memory allocation related problem in DMD and decided to make a post about it in this list.
>>
>>
>
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>

That's the most obvious case. Unfortunately, there are cases that are
harder to track and fix.
For example, OutBuffer uses mem.malloc/mem.realloc and result is
sometimes assigned to elem*, Symbol* and block* variables (and freed
by backend).
A string, created in FileName::forceExt (it evaluates return new
FileName(s, 0); which does mem.strdup in String ctor) is also
deallocated in backend.
el_setLoc does raw pointer assignment. Who knows how original string
was allocated? Deallocated in backend.

You could probably catch all the occurrences saving all the allocated pinters in a map and checking that the pointer to deallocate is indeed present in that map. MEM_DEBUG already half of the needed functionality. Fortunately, all the backend allocations/deallocation are forwarded to tk/mem.c (at least the problematic ones).
December 09, 2009
What can I say? These all need to be fixed. Thanks for helping identify them.

Denis wrote:
>
>
> That's the most obvious case. Unfortunately, there are cases that are
> harder to track and fix.
> For example, OutBuffer uses mem.malloc/mem.realloc and result is
> sometimes assigned to elem*, Symbol* and block* variables (and freed
> by backend).
> A string, created in FileName::forceExt (it evaluates return new
> FileName(s, 0); which does mem.strdup in String ctor) is also
> deallocated in backend.
> el_setLoc does raw pointer assignment. Who knows how original string
> was allocated? Deallocated in backend.
>
> You could probably catch all the occurrences saving all the allocated pinters in a map and checking that the pointer to deallocate is indeed present in that map. MEM_DEBUG already half of the needed functionality. Fortunately, all the backend allocations/deallocation are forwarded to tk/mem.c (at least the problematic ones).
>
>