This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Remove _ATEXIT_DYNAMIC_ALLOC


Sorry for the delay. I wanted to test this one a bit and think about it some more. I was a little uneasy with the idea of atexit changing its behavior based on how the application was linked.

After thinking about it, I am ok with the concept. atexit is only guaranteed to give 32 entries and the limit ATEXIT_MAX can be undefined if the value is >=32 and undefined. Current x86-linux newlib does not return ATEXIT_MAX via sysconf(). I cannot vouch for what RTEMS and Cygwin do. Regardless, the patch does not appear to affect x86-linux behavior.

That said, there is a problem caused by the fact that the current underlying atexit mechanism should be using _malloc_r and _free_r instead of malloc/free. The _malloc_r and _free_r routines are used internally by newlib (and by malloc and free on platforms that do not supply their own malloc/free).

In the current code, when a malloc failure occurs, errno will get set. However, errno should be set per reentrancy struct which implies either that a reentrant version of the atexit routine is needed or errno needs to be saved and restored around the malloc call. The 2nd option would be ok due to the fact that atexit is not expected to set errno and it has a failure return code.

Even if errno is saved/restored, there is still an issue with using malloc instead of _malloc_r.

Consider the following example:

#include <stdlib.h>
#include <stdio.h>

int k;

void x (void) {
  printf ("hello world\n");
  ++k;
}

int main() {
  int i;

  for (i = 0; i < 64; ++i) {
     if (atexit(&x) != 0)
       break;
  }

  return i;
}

In this scenario, run on mn10300-elf with your patch applied, printf actually drags in the malloc logic via _malloc_r, but atexit doesn't specifically see "malloc" so it holds to the 32 limit which it should not do.

-- Jeff J.

Paul Brook wrote:
The attached patch removes _ATEXIT_DYNAMIC_ALLOC. It is replaced by a weak reference to malloc/free, as suggested by Richard Earnshaw:
http://sources.redhat.com/ml/newlib/2006/msg00207.html


This means the same library can be used for both large and small applications. Applications that already use malloc get the full dynamic atexit implementation (it's almost free anyway), whereas applications that don't use malloc are limited to 32 atexit slots, but don't pull in the malloc code.

The theory is that any application big enough to need more atexit slots probably uses malloc. The main heavy user of atexit slots is c++ (via __cxa_atexit), and an empty c++ program already pulls in malloc.

Ok?

Paul

2006-08-24 Paul Brook <paul@codesourcery.com>

	* acconfig.h (_ATEXIT_DYNAMIC_ALLOC): Remove.
	* configure.in: Remove --disable-newlib-atexit-dynamic-alloc.
	* libc/stdlib/__atexit.c (__register_exitproc): Use weak reference to
	malloc.  Only allocate dynamically if it is present.
	* libc/stdlib/__call_atexit.c (__call_exitprocs): Use weak reference
	to free.
	* configure: Regenerate.
	* newlib.hin: Regenerate.


------------------------------------------------------------------------


Index: newlib/acconfig.h
===================================================================
RCS file: /var/cvsroot/src-cvs/src/newlib/acconfig.h,v
retrieving revision 1.2
diff -u -p -r1.2 acconfig.h
--- newlib/acconfig.h 21 Mar 2006 00:57:34 -0000 1.2
+++ newlib/acconfig.h 23 Aug 2006 23:45:29 -0000
@@ -34,10 +34,6 @@
* sections. */
#undef HAVE_INITFINI_ARRAY
-/* True if atexit() may dynamically allocate space for cleanup
- functions. */
-#undef _ATEXIT_DYNAMIC_ALLOC
-
/* Define if the compiler supports aliasing an array to an address. */
#undef _HAVE_ARRAY_ALIASING
@BOTTOM@
Index: newlib/configure
===================================================================
RCS file: /var/cvsroot/src-cvs/src/newlib/configure,v
retrieving revision 1.50
diff -u -p -r1.50 configure
--- newlib/configure 31 Jul 2006 23:00:56 -0000 1.50
+++ newlib/configure 24 Aug 2006 00:18:03 -0000
@@ -830,7 +830,6 @@ Optional Features:
--enable-newlib-iconv-from-encodings enable specific comma-separated list of \"from\" iconv encodings to be built-in
--enable-newlib-iconv-to-encodings enable specific comma-separated list of \"to\" iconv encodings to be built-in
--enable-newlib-iconv-external-ccs enable capabilities to load external CCS files for iconv
- --disable-newlib-atexit-alloc disable dynamic allocation of atexit entries
--enable-multilib build many library versions (default)
--enable-target-optspace optimize for space
--enable-malloc-debugging indicate malloc debugging requested
@@ -1449,21 +1448,6 @@ echo "$as_me: error: bad value ${enablev
else
newlib_iconv_external_ccs=${newlib_iconv_external_ccs}
fi;
-# Check whether --enable-newlib-atexit-dynamic-alloc or --disable-newlib-atexit-dynamic-alloc was given.
-if test "${enable_newlib_atexit_dynamic_alloc+set}" = set; then
- enableval="$enable_newlib_atexit_dynamic_alloc"
- if test "${newlib_atexit_dynamic_alloc+set}" != set; then
- case "${enableval}" in
- yes) newlib_atexit_dynamic_alloc=yes ;;
- no) newlib_atexit_dynamic_alloc=no ;;
- *) { { echo "$as_me:$LINENO: error: bad value ${enableval} for newlib-atexit-dynamic-alloc option" >&5
-echo "$as_me: error: bad value ${enableval} for newlib-atexit-dynamic-alloc option" >&2;}
- { (exit 1); exit 1; }; } ;;
- esac
- fi
-else
- newlib_atexit_dynamic_alloc=${newlib_atexit_dynamic_alloc}
-fi;
# Make sure we can run config.sub.
$ac_config_sub sun4 >/dev/null 2>&1 ||
@@ -4764,7 +4748,7 @@ test x"$pic_mode" = xno && libtool_flags
case $host in
*-*-irix6*)
# Find out which ABI we are using.
- echo '#line 4767 "configure"' > conftest.$ac_ext
+ echo '#line 4751 "configure"' > conftest.$ac_ext
if { (eval echo "$as_me:$LINENO: \"$ac_compile\"") >&5
(eval $ac_compile) 2>&5
ac_status=$?
@@ -5257,13 +5241,6 @@ _ACEOF
fi
-if test "${newlib_atexit_dynamic_alloc}" = "yes"; then
-cat >>confdefs.h <<_ACEOF
-#define _ATEXIT_DYNAMIC_ALLOC 1
-_ACEOF
-
-fi
-
if test "x${iconv_encodings}" != "x" \
|| test "x${iconv_to_encodings}" != "x" \
Index: newlib/configure.in
===================================================================
RCS file: /var/cvsroot/src-cvs/src/newlib/configure.in,v
retrieving revision 1.34
diff -u -p -r1.34 configure.in
--- newlib/configure.in 31 Jul 2006 23:00:56 -0000 1.34
+++ newlib/configure.in 24 Aug 2006 00:17:42 -0000
@@ -86,17 +86,6 @@ AC_ARG_ENABLE(newlib-iconv-external-ccs,
esac
fi], [newlib_iconv_external_ccs=${newlib_iconv_external_ccs}])dnl
-dnl Support --disable-newlib-atexit-dynamic-alloc
-AC_ARG_ENABLE(newlib-atexit-dynamic-alloc,
-[ --disable-newlib-atexit-alloc disable dynamic allocation of atexit entries],
-[if test "${newlib_atexit_dynamic_alloc+set}" != set; then
- case "${enableval}" in
- yes) newlib_atexit_dynamic_alloc=yes ;;
- no) newlib_atexit_dynamic_alloc=no ;;
- *) AC_MSG_ERROR(bad value ${enableval} for newlib-atexit-dynamic-alloc option) ;;
- esac
- fi], [newlib_atexit_dynamic_alloc=${newlib_atexit_dynamic_alloc}])dnl
-
NEWLIB_CONFIGURE(.)
dnl We have to enable libtool after NEWLIB_CONFIGURE because if we try and
@@ -262,10 +251,6 @@ if test "x${newlib_iconv_external_ccs}" AC_DEFINE_UNQUOTED(_ICONV_ENABLE_EXTERNAL_CCS,1)
fi
-if test "${newlib_atexit_dynamic_alloc}" = "yes"; then
-AC_DEFINE_UNQUOTED(_ATEXIT_DYNAMIC_ALLOC)
-fi
-
dnl
dnl Parse --enable-newlib-iconv-encodings option argument
dnl
Index: newlib/newlib.hin
===================================================================
RCS file: /var/cvsroot/src-cvs/src/newlib/newlib.hin,v
retrieving revision 1.12
diff -u -p -r1.12 newlib.hin
--- newlib/newlib.hin 21 Mar 2006 00:57:34 -0000 1.12
+++ newlib/newlib.hin 24 Aug 2006 00:11:35 -0000
@@ -34,9 +34,5 @@
* sections. */
#undef HAVE_INITFINI_ARRAY
-/* True if atexit() may dynamically allocate space for cleanup
- functions. */
-#undef _ATEXIT_DYNAMIC_ALLOC
-
/* Define if the compiler supports aliasing an array to an address. */
#undef _HAVE_ARRAY_ALIASING
Index: newlib/libc/stdlib/__atexit.c
===================================================================
RCS file: /var/cvsroot/src-cvs/src/newlib/libc/stdlib/__atexit.c,v
retrieving revision 1.4
diff -u -p -r1.4 __atexit.c
--- newlib/libc/stdlib/__atexit.c 21 Mar 2006 00:57:34 -0000 1.4
+++ newlib/libc/stdlib/__atexit.c 23 Aug 2006 23:45:31 -0000
@@ -8,6 +8,8 @@
#include <sys/lock.h>
#include "atexit.h"
+/* Make this a weak reference to avoid pulling in malloc. */
+void * malloc(size_t) __attribute__((weak));
/*
* Register a function to be performed at exit or on shared library unload.
@@ -35,9 +37,8 @@ _DEFUN (__register_exitproc,
_GLOBAL_REENT->_atexit = p = &_GLOBAL_REENT->_atexit0;
if (p->_ind >= _ATEXIT_SIZE)
{
-#ifndef _ATEXIT_DYNAMIC_ALLOC
- return -1;
-#else
+ if (!malloc)
+ return -1;
p = (struct _atexit *) malloc (sizeof *p);
if (p == NULL)
{
@@ -53,7 +54,6 @@ _DEFUN (__register_exitproc,
p->_on_exit_args._fntypes = 0;
p->_on_exit_args._is_cxa = 0;
#endif
-#endif
}
if (type != __et_atexit)
Index: newlib/libc/stdlib/__call_atexit.c
===================================================================
RCS file: /var/cvsroot/src-cvs/src/newlib/libc/stdlib/__call_atexit.c,v
retrieving revision 1.4
diff -u -p -r1.4 __call_atexit.c
--- newlib/libc/stdlib/__call_atexit.c 21 Mar 2006 00:57:34 -0000 1.4
+++ newlib/libc/stdlib/__call_atexit.c 23 Aug 2006 23:45:31 -0000
@@ -12,6 +12,9 @@
* otherwise only the handlers from that DSO are called.
*/
+/* Make this a weak reference to avoid pulling in malloc. */
+void free(void *) __attribute__((weak));
+
void _DEFUN (__call_exitprocs, (code, d),
int code _AND _PTR d)
@@ -61,9 +64,9 @@ _DEFUN (__call_exitprocs, (code, d),
(*((void (*)(_PTR)) fn))(args->_fnargs[n]);
}
-#ifndef _ATEXIT_DYNAMIC_ALLOC
- break;
-#else
+ if (!free)
+ break;
+
/* Move to the next block. Free empty blocks except the last one,
which is part of _GLOBAL_REENT. */
if (p->_ind == 0 && p->_next)
@@ -82,6 +85,5 @@ _DEFUN (__call_exitprocs, (code, d),
lastp = &p->_next;
p = p->_next;
}
-#endif
}
}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]