_reclaim_reent and _REENT_SMALL

[Hans-Erik Floryd <hans-erik dot floryd at rt-labs dot com>]

Hello,

We have built newlib with --enable-newlib-reent-small. We also use 
--enable-newlib-multithread and __DYNAMIC_REENT__ to interface to an 
RTOS. When a thread exits we call _reclaim_reent to release the thread 
reent structure. Unfortunately, there seems to be a few cases in this 
function where a NULL pointer ends up being dereferenced.

The first case is this:

#ifdef _REENT_SMALL
      if (ptr->_mp)	/* don't bother allocating it! */
#endif
      if (_REENT_MP_FREELIST(ptr))
	{
	  int i;
	  for (i = 0; i < 15 /* _Kmax */; i++)
	    {
	      struct _Bigint *thisone, *nextone;
	
	      nextone = _REENT_MP_FREELIST(ptr)[i];
	      while (nextone)
		{
		  thisone = nextone;
		  nextone = nextone->_next;
		  _free_r (ptr, thisone);
		}
	    }

	  _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)
	_free_r (ptr, ptr->_emergency);
[...]

When using _REENT_SMALL, _REENT_MP_RESULT(ptr) (i.e. ptr->_mp->_result) 
is evaluated even though ptr->_mp may be NULL. A patch for this is attached.

Further on in the code this also causes problems:

      if (ptr->_atexit->_on_exit_args_ptr)
	_free_r (ptr, ptr->_atexit->_on_exit_args_ptr);

There is no check that ptr->_atexit is not NULL. The attached patch 
fixes this also.

However, I am not sure I understand how atexit works for 
__DYNAMIC_REENT__. The code in _wrapup_reent calls all registered exit 
procedures in the given reent structure, but __register_exitproc always 
works on the _GLOBAL_REENT structure. So how does a thread register its 
own exit function?

Having been unable to register an exit procedure for a thread I am 
unsure of the following, but this code in _wrapup_reent also seems 
questionable:

  register struct _atexit *p;

#ifdef _REENT_SMALL
  for (p = &ptr->_atexit, n = p->_ind; --n >= 0;)
    (*p->_fns[n]) ();
#else

This gives a warning when compiling:

../../../../../../newlib-1.16.0/newlib/libc/reent/reent.c:139: warning: 
assignment from incompatible pointer type

which is because the type of &ptr->_atexit is struct _atexit **, whereas 
p is just struct _atexit *. When evaluating p->_ind, wouldn't you get 
the value of some other member in the reent structure? I suspect 
something like this

  if (ptr->_atexit)
  {
    for (p = ptr->_atexit, n = p->_ind; --n >= 0;)
      (*p->_fns[n]) ();
  }

would work better but as I said I haven't been able to investigate if 
this works or if there really is a problem here.

Hints from anyone using _REENT_SMALL would be most welcome!

Cheers,
 Hans-Erik Floryd

diff -Naur newlib-1.16.0/newlib/libc/reent/reent.c 
newlib-1.16.0-mod/newlib/libc/reent/reent.c
--- newlib-1.16.0/newlib/libc/reent/reent.c     2006-10-11 
10:04:50.000000000 +0200
+++ newlib-1.16.0-mod/newlib/libc/reent/reent.c 2009-03-06 
10:29:11.000000000 +0100
@@ -50,6 +50,7 @@
       /* used by mprec routines. */
 #ifdef _REENT_SMALL
       if (ptr->_mp)    /* don't bother allocating it! */
+       {
 #endif
       if (_REENT_MP_FREELIST(ptr))
        {
@@ -73,6 +74,7 @@
        _free_r (ptr, _REENT_MP_RESULT(ptr));

 #ifdef _REENT_SMALL
+       }
       if (ptr->_emergency)
        _free_r (ptr, ptr->_emergency);
       if (ptr->_mp)
@@ -83,7 +85,7 @@
        _free_r (ptr, ptr->_localtime_buf);
       if (ptr->_asctime_buf)
        _free_r (ptr, ptr->_asctime_buf);
-      if (ptr->_atexit->_on_exit_args_ptr)
+      if (ptr->_atexit && ptr->_atexit->_on_exit_args_ptr)
        _free_r (ptr, ptr->_atexit->_on_exit_args_ptr);
 #else
       /* atexit stuff */