Re: [PATCH] Fix memory leak in dtoa/ldtoa

[Jeff Johnston <jjohnstn at redhat dot com>]

Joel Sherrill wrote:
> Hi,
>
> This patch caused a side discussion among a few RTEMS folks. We don't 
> have a concern about this patch but some general concerns
> that I am asking about.
>
> Balloc appears to correctly return NULL when calloc fails.
> But I did see any Balloc caller check for a NULL being returned.
> What should happen?

What does happen is that one gets a null pointer access.  If you are 
asking should we add graceful return checking for all callers of Balloc, 
 IMO, no.  Considering the very small amount of storage that Balloc is 
asking for, the application is really out of storage and is extremely 
unlikely to check and recover so I'm not convinced the added overhead 
would be justified.  We could have Balloc perform an assert as we are 
assuming everywhere it is used that it is returning a non-null value.

> Is there a master list of things that cause memory allocation per thread
> in newlib?  Eric Norum has an RTEMS test case which demonstrates
> this leak for RTEMS applications and prints how much memory is available
> after each thread create/delete.  We could enhance this to make the test 
> thread
> do the things that cause memory allocation to verify there isn't another 
> leak.
>
> --joel


There isn't such a list available.  Would you like to help put one together?

-- Jeff J.

> Jeff Johnston wrote:
>
> > Corinna Vinschen wrote:
> >
> > > Hi,
> > >
> > > the following report has been posted to the Cygwin list.  A test 
> > > application
> > > using printf("%f") in short-lived threads uses up all memory.  The test
> > > application is very simple:
> > >
> > >   void *testThread(void *data)
> > >   {
> > >     printf("\r\n%f",1.232231212);
> > >     return(NULL);
> > >   }
> > >   int main (int argc, char *argv[])
> > >   {
> > >     pthread_t t;
> > >
> > >     while(1)
> > >       {
> > >       pthread_create(&t,NULL,testThread,NULL);
> > >       pthread_join(t,NULL);
> > >       }     return 1;
> > >   }
> > > The cause is apparently a memory leak in ldtoa resp. dtoa.  The first
> > > time the function is called, _REENT_MP_RESULT(ptr) gets newly allocated
> > > memory by a call to Balloc.  This memory is Bfree'd in subsequent calls
> > > to ldtoa/dtoa, but immediately re-Ballocated.  Since a Ballocated area
> > > is only actually freed in _reclaim_reent when a matching Bfree has been
> > > called, the _REENT_MP_RESULT(ptr) memory is never actually freed and
> > > every new thread using ldtoa/dtoa will leak this memory area when the
> > > thread exits.
> > >
> > > The solution is to explicitely free the _REENT_MP_RESULT(ptr) area in
> > > _reclaim_reent.  That's what the below patch does.  Ok to apply?
> >
> > Yes, please go ahead.  Thanks.
> >
> > -- Jeff J.
> >
> > > Corinna
> > >
> > >
> > >     * libc/reent/reeent.c (_reclaim_reent): Free _REENT_MP_RESULT.
> > >
> > >
> > > Index: libc/reent/reent.c
> > > ===================================================================
> > > RCS file: /cvs/src/src/newlib/libc/reent/reent.c,v
> > > retrieving revision 1.8
> > > diff -u -p -r1.8 reent.c
> > > --- libc/reent/reent.c    9 Sep 2004 19:46:54 -0000    1.8
> > > +++ libc/reent/reent.c    9 Oct 2006 17:44:46 -0000
> > > @@ -69,6 +69,8 @@ _DEFUN (_reclaim_reent, (ptr),
> > >  
> > >        _free_r (ptr, _REENT_MP_FREELIST(ptr));
> > >      }
> > > +      if (_REENT_MP_RESULT(ptr))
> > > +        _free_r (ptr, _REENT_MP_RESULT(ptr));
> > >  
> > >  #ifdef _REENT_SMALL
> > >        if (ptr->_emergency)