Bug in __register_exitproc() in __atexit.c

[Freddie Chopin]

On Wednesday 09 of December 2015 10:27:31 Corinna Vinschen wrote:
> On Dec  8 16:40, Freddie Chopin wrote:
> > Hello!
> > 
> > Today I noticed that even when I disable dynamic allocation of memory in
> > atexit (with --disable-newlib-atexit-dynamic-alloc switch) malloc() is
> > used in __register_exitproc().
> > 
> > First of all, if this switch is used, then _ATEXIT_DYNAMIC_ALLOC is not
> > defined, so the first use of malloc() in this function - at line 93 - is
> > indeed disabled. But if small reent is used, malloc() is used again at
> > line
> > 120 - this time it's not conditional on _ATEXIT_DYNAMIC_ALLOC being
> > defined.
> > 
> > Moreover, at first glance it seems that there's a bug in this function.
> > The
> > __atexit_lock lock is acquired at entry, but there are at two places where
> > it is not released before return - at line 86 and at line 91.
> > 
> > Should I try to provide a patch for both issues?
> 
> That would be nice :)

OK, I've just sent the patches.

I've checked that newlib still compiles with all 8 combinations of related 
options (reent small, global atexit, dynamic atexit). The fix I implemented 
also solves the issue I had - __register_exitproc() no longer uses malloc() if 
--disable-newlib-atexit-dynamic-alloc was used.

The first patch - the one for missing lock releases - is trivial and there's 
not much to debate.

The second one - with the fix for dynamic atexit allocation - is more 
problematic... It turns out that when both --disable-newlib-atexit-dynamic-
alloc and --enable-newlib-reent-small are used, then __register_exitproc() 
called by on_exit() or __cxa_atexit() will never succeed. Both of these 
variants pass an argument to the registered function and this argument is 
placed in a struct that is _NOT_ part of _atexit struct.

This can actually be solved easily - you'd just need to allocate a static 
instance of _on_exit_args struct and use it for the first 32 registered 
functions. The only problem is that the size of this struct is - for 32-bit 
ARM - 264 bytes, so not exactly "small". So I was wondering what approach 
would you suggest? Leave it as it is (with the fix) or maybe add static 
instance of this struct when dynamic allocation is disabled for atexit-like 
functions?

Maybe it would be possible to have the instance of this struct as a weak 
reference and pull it in only if on_exit() or __cxa_atexit() are used?

Regards,
FCh