From b0cb9f85ca3626e0e68fd451c3090d253ceb4300 Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Tue, 17 May 2022 18:28:52 +0200 Subject: [PATCH] Use global stdio streams for all configurations The _REENT_GLOBAL_STDIO_STREAMS was introduced by commit 668a4c8722090fffd10869dbb15b879651c1370d in 2017. Since then it was enabled by default for RTEMS. Recently, the option was enabled for Cygwin which previously used an alternative implementation to use global stdio streams. In Newlib, the stdio streams are defined to thread-specific pointers _reent::_stdin, _reent::_stdout and _reent::_stderr. If the option is disabled (the default for most systems), then these pointers are initialized to thread-specific FILE objects which use file descriptors 0, 1, and 2, respectively. There are at least three problems with this: (1) The thread-specific FILE objects are closed by _reclaim_reent(). This leads to problems with language run-time libraries that provide wrappers to the C/POSIX stdio streams (for example C++ and Ada), since they use the thread-specific FILE objects of the initialization thread. In case the initialization thread is deleted, then they use freed memory. (2) Since thread-specific FILE objects are used with a common output device via file descriptors 0, 1 and 2, the locking at FILE object level cannot ensure atomicity of the output, e.g. a call to printf(). (3) There are resource managment issues, see: https://sourceware.org/pipermail/newlib/2022/019558.html https://bugs.linaro.org/show_bug.cgi?id=5841 This patch enables the _REENT_GLOBAL_STDIO_STREAMS behaviour for all Newlib configurations and removes the option. This removes a couple of #ifdef blocks. --- newlib/README | 6 -- newlib/configure.ac | 10 +-- newlib/libc/include/sys/config.h | 7 -- newlib/libc/include/sys/reent.h | 118 +++----------------------- newlib/libc/reent/impure.c | 6 -- newlib/libc/reent/reent.c | 20 ----- newlib/libc/stdio/fcloseall.c | 4 - newlib/libc/stdio/findfp.c | 53 +----------- newlib/libc/stdio/local.h | 17 ---- winsup/cygwin/include/cygwin/config.h | 1 - 10 files changed, 14 insertions(+), 228 deletions(-) diff --git a/newlib/README b/newlib/README index f00e66019..dd4686eeb 100644 --- a/newlib/README +++ b/newlib/README @@ -294,12 +294,6 @@ One feature can be enabled by specifying `--enable-FEATURE=yes' or Disable dynamic allocation of atexit entries. Most hosts and targets have it enabled in configure.host. -`--enable-newlib-global-stdio-streams' - Enable to move the stdio stream FILE objects out of struct _reent and make - them global. The stdio stream pointers of struct _reent are initialized - to point to the global stdio FILE stream objects. - Disabled by default. - `--enable-newlib-reent-small' Enable small reentrant struct support. Disabled by default. diff --git a/newlib/configure.ac b/newlib/configure.ac index 57f830960..1951aab0c 100644 --- a/newlib/configure.ac +++ b/newlib/configure.ac @@ -174,13 +174,13 @@ AC_ARG_ENABLE(newlib-reent-binary-compat, fi], [newlib_reent_binary_compat=no])dnl dnl Support --enable-newlib-global-stdio-streams +dnl This is no longer optional. It is enabled in all Newlib configurations. AC_ARG_ENABLE(newlib-global-stdio-streams, [ --enable-newlib-global-stdio-streams enable global stdio streams], [case "${enableval}" in yes) newlib_global_stdio_streams=yes;; - no) newlib_global_stdio_streams=no ;; *) AC_MSG_ERROR(bad value ${enableval} for newlib-global-stdio-streams option) ;; - esac], [newlib_global_stdio_streams=])dnl + esac], [newlib_global_stdio_streams=yes])dnl dnl Support --disable-newlib-fvwrite-in-streamio AC_ARG_ENABLE(newlib-fvwrite-in-streamio, @@ -435,12 +435,6 @@ if test "${newlib_reent_binary_compat}" = "yes"; then AC_DEFINE(_WANT_REENT_BACKWARD_BINARY_COMPAT, 1, [Define to enable backward binary compatibility for struct _reent.]) fi -if test "${newlib_global_stdio_streams}" = "yes"; then - AC_DEFINE(_WANT_REENT_GLOBAL_STDIO_STREAMS, 1, - [Define to move the stdio stream FILE objects out of struct _reent and make them global. - The stdio stream pointers of struct _reent are initialized to point to the global stdio FILE stream objects.]) -fi - _mb_len_max=1 if test "${newlib_mb}" = "yes"; then AC_DEFINE(_MB_CAPABLE, 1, [Multibyte supported.]) diff --git a/newlib/libc/include/sys/config.h b/newlib/libc/include/sys/config.h index c40bb51c2..4bce10de3 100644 --- a/newlib/libc/include/sys/config.h +++ b/newlib/libc/include/sys/config.h @@ -242,7 +242,6 @@ #define __FILENAME_MAX__ 255 #define _READ_WRITE_RETURN_TYPE _ssize_t #define __DYNAMIC_REENT__ -#define _REENT_GLOBAL_STDIO_STREAMS #endif #ifndef __EXPORT @@ -280,12 +279,6 @@ #endif #endif -#ifdef _WANT_REENT_GLOBAL_STDIO_STREAMS -#ifndef _REENT_GLOBAL_STDIO_STREAMS -#define _REENT_GLOBAL_STDIO_STREAMS -#endif -#endif - #ifdef _WANT_USE_LONG_TIME_T #ifndef _USE_LONG_TIME_T #define _USE_LONG_TIME_T diff --git a/newlib/libc/include/sys/reent.h b/newlib/libc/include/sys/reent.h index 5d889b4a2..2d144f653 100644 --- a/newlib/libc/include/sys/reent.h +++ b/newlib/libc/include/sys/reent.h @@ -142,39 +142,7 @@ struct __sbuf { * _ub._base!=NULL) and _up and _ur save the current values of _p and _r. */ -#if defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS) -/* - * struct __sFILE_fake is the start of a struct __sFILE, with only the - * minimal fields allocated. In __sinit() we really allocate the 3 - * standard streams, etc., and point away from this fake. - */ -struct __sFILE_fake { - unsigned char *_p; /* current position in (some) buffer */ - int _r; /* read space left for getc() */ - int _w; /* write space left for putc() */ - short _flags; /* flags, below; this FILE is free if 0 */ - short _file; /* fileno, if Unix descriptor, else -1 */ - struct __sbuf _bf; /* the buffer (at least 1 byte, if !NULL) */ - int _lbfsize; /* 0 or -_bf._size, for inline putc */ - - struct _reent *_data; -}; - -/* Following is needed both in libc/stdio and libc/stdlib so we put it - * here instead of libc/stdio/local.h where it was previously. */ - -extern void __sinit (struct _reent *); - -# define _REENT_SMALL_CHECK_INIT(ptr) \ - do \ - { \ - if ((ptr) && !(ptr)->__cleanup) \ - __sinit (ptr); \ - } \ - while (0) -#else /* _REENT_SMALL && !_REENT_GLOBAL_STDIO_STREAMS */ -# define _REENT_SMALL_CHECK_INIT(ptr) /* nothing */ -#endif /* _REENT_SMALL && !_REENT_GLOBAL_STDIO_STREAMS */ +#define _REENT_SMALL_CHECK_INIT(ptr) /* nothing */ struct __sFILE { unsigned char *_p; /* current position in (some) buffer */ @@ -286,9 +254,7 @@ typedef struct __sFILE __FILE; #endif /* __LARGE64_FILES */ #endif /* !__CUSTOM_FILE_IO__ */ -#ifdef _REENT_GLOBAL_STDIO_STREAMS extern __FILE __sf[3]; -#endif struct _glue { @@ -340,11 +306,13 @@ struct _rand48 { #define _REENT_INIT_RESERVED_1 0, #define _REENT_INIT_RESERVED_2 0, #define _REENT_INIT_RESERVED_6_7 _NULL, _ATEXIT_INIT, +#define _REENT_INIT_RESERVED_8 {_NULL, 0, _NULL}, #else #define _REENT_INIT_RESERVED_0 /* Nothing to initialize */ #define _REENT_INIT_RESERVED_1 /* Nothing to initialize */ #define _REENT_INIT_RESERVED_2 /* Nothing to initialize */ #define _REENT_INIT_RESERVED_6_7 /* Nothing to initialize */ +#define _REENT_INIT_RESERVED_8 /* Nothing to initialize */ #endif /* @@ -427,16 +395,14 @@ struct _reent #ifdef _REENT_BACKWARD_BINARY_COMPAT struct _atexit *_reserved_6; struct _atexit _reserved_7; + struct _glue _reserved_8; #endif - struct _glue __sglue; /* root of glue chain */ __FILE *__sf; /* file descriptors */ struct _misc_reent *_misc; /* strtok, multibyte states */ char *_signal_buf; /* strsignal */ }; -#ifdef _REENT_GLOBAL_STDIO_STREAMS - # define _REENT_INIT(var) \ { 0, \ &__sf[0], \ @@ -457,7 +423,7 @@ struct _reent _NULL, \ _NULL, \ _REENT_INIT_RESERVED_6_7 \ - {_NULL, 0, _NULL}, \ + _REENT_INIT_RESERVED_8 \ _NULL, \ _NULL, \ _NULL \ @@ -469,46 +435,6 @@ struct _reent (var)->_stderr = &__sf[2]; \ } -#else /* _REENT_GLOBAL_STDIO_STREAMS */ - -extern const struct __sFILE_fake __sf_fake_stdin; -extern const struct __sFILE_fake __sf_fake_stdout; -extern const struct __sFILE_fake __sf_fake_stderr; - -# define _REENT_INIT(var) \ - { 0, \ - (__FILE *)&__sf_fake_stdin, \ - (__FILE *)&__sf_fake_stdout, \ - (__FILE *)&__sf_fake_stderr, \ - 0, \ - _NULL, \ - _REENT_INIT_RESERVED_0 \ - _REENT_INIT_RESERVED_1 \ - _NULL, \ - _NULL, \ - _NULL, \ - 0, \ - 0, \ - _NULL, \ - _NULL, \ - _NULL, \ - _NULL, \ - _NULL, \ - _REENT_INIT_RESERVED_6_7 \ - {_NULL, 0, _NULL}, \ - _NULL, \ - _NULL, \ - _NULL \ - } - -#define _REENT_INIT_PTR_ZEROED(var) \ - { (var)->_stdin = (__FILE *)&__sf_fake_stdin; \ - (var)->_stdout = (__FILE *)&__sf_fake_stdout; \ - (var)->_stderr = (__FILE *)&__sf_fake_stderr; \ - } - -#endif /* _REENT_GLOBAL_STDIO_STREAMS */ - /* Specify how to handle reent_check malloc failures. */ #ifdef _REENT_CHECK_VERIFY #include @@ -696,33 +622,13 @@ struct _reent /* signal info */ void (**_sig_func)(int); - - /* These are here last so that __FILE can grow without changing the offsets - of the above members (on the off chance that future binary compatibility - would be broken otherwise). */ -# ifndef _REENT_GLOBAL_STDIO_STREAMS - struct _glue __sglue; /* root of glue chain */ - __FILE __sf[3]; /* first three file descriptors */ -# endif }; -#ifdef _REENT_GLOBAL_STDIO_STREAMS -#define _REENT_STDIO_STREAM(var, index) &__sf[index] -#define _REENT_INIT_SGLUE(_ptr) /* nothing to initialize */ -#define _REENT_INIT_SGLUE_ZEROED(_ptr) /* nothing to set */ -#else -#define _REENT_STDIO_STREAM(var, index) &(var)->__sf[index] -#define _REENT_INIT_SGLUE(_ptr) , { _NULL, 3, &(_ptr)->__sf[0] } -#define _REENT_INIT_SGLUE_ZEROED(_ptr) \ - (_ptr)->__sglue._niobs = 3; \ - (_ptr)->__sglue._iobs = &(_ptr)->__sf[0]; -#endif - #define _REENT_INIT(var) \ { 0, \ - _REENT_STDIO_STREAM(&(var), 0), \ - _REENT_STDIO_STREAM(&(var), 1), \ - _REENT_STDIO_STREAM(&(var), 2), \ + &__sf[0], \ + &__sf[1], \ + &__sf[2], \ 0, \ "", \ _REENT_INIT_RESERVED_1 \ @@ -763,13 +669,12 @@ struct _reent }, \ _REENT_INIT_RESERVED_6_7 \ _NULL \ - _REENT_INIT_SGLUE(&(var)) \ } #define _REENT_INIT_PTR_ZEROED(var) \ - { (var)->_stdin = _REENT_STDIO_STREAM(var, 0); \ - (var)->_stdout = _REENT_STDIO_STREAM(var, 1); \ - (var)->_stderr = _REENT_STDIO_STREAM(var, 2); \ + { (var)->_stdin = &__sf[0]; \ + (var)->_stdout = &__sf[1]; \ + (var)->_stderr = &__sf[2]; \ (var)->_new._reent._rand_next = 1; \ (var)->_new._reent._r48._seed[0] = _RAND48_SEED_0; \ (var)->_new._reent._r48._seed[1] = _RAND48_SEED_1; \ @@ -778,7 +683,6 @@ struct _reent (var)->_new._reent._r48._mult[1] = _RAND48_MULT_1; \ (var)->_new._reent._r48._mult[2] = _RAND48_MULT_2; \ (var)->_new._reent._r48._add = _RAND48_ADD; \ - _REENT_INIT_SGLUE_ZEROED(var) \ } #define _REENT_CHECK_RAND48(ptr) /* nothing */ diff --git a/newlib/libc/reent/impure.c b/newlib/libc/reent/impure.c index 643a511c6..9290c9cd5 100644 --- a/newlib/libc/reent/impure.c +++ b/newlib/libc/reent/impure.c @@ -6,13 +6,7 @@ important to reduce image size for targets with very small amounts of memory. */ #ifdef _REENT_SMALL -#ifdef _REENT_GLOBAL_STDIO_STREAMS extern __FILE __sf[3] _ATTRIBUTE ((weak)); -#else -extern const struct __sFILE_fake __sf_fake_stdin _ATTRIBUTE ((weak)); -extern const struct __sFILE_fake __sf_fake_stdout _ATTRIBUTE ((weak)); -extern const struct __sFILE_fake __sf_fake_stderr _ATTRIBUTE ((weak)); -#endif #endif struct _reent __ATTRIBUTE_IMPURE_DATA__ _impure_data = _REENT_INIT (_impure_data); diff --git a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c index 04942ce4d..d61415901 100644 --- a/newlib/libc/reent/reent.c +++ b/newlib/libc/reent/reent.c @@ -27,21 +27,6 @@ int errno; #endif -#ifndef _REENT_GLOBAL_STDIO_STREAMS -/* Interim cleanup code */ - -static void -cleanup_glue (struct _reent *ptr, - struct _glue *glue) -{ - /* Have to reclaim these in reverse order: */ - if (glue->_next) - cleanup_glue (ptr, glue->_next); - - _free_r (ptr, glue); -} -#endif - void _reclaim_reent (struct _reent *ptr) { @@ -106,11 +91,6 @@ _reclaim_reent (struct _reent *ptr) /* cleanup won't reclaim memory 'coz usually it's run before the program exits, and who wants to wait for that? */ ptr->__cleanup (ptr); - -#ifndef _REENT_GLOBAL_STDIO_STREAMS - if (ptr->__sglue._next) - cleanup_glue (ptr, ptr->__sglue._next); -#endif } /* Malloc memory not reclaimed; no good way to return memory anyway. */ diff --git a/newlib/libc/stdio/fcloseall.c b/newlib/libc/stdio/fcloseall.c index 642dc7d94..e840af27c 100644 --- a/newlib/libc/stdio/fcloseall.c +++ b/newlib/libc/stdio/fcloseall.c @@ -59,12 +59,8 @@ Required OS subroutines: <>, <>, <>, <>, int _fcloseall_r (struct _reent *ptr) { -#ifdef _REENT_GLOBAL_STDIO_STREAMS /* There are no thread-specific FILE objects */ return 0; -#else - return _fwalk_sglue (ptr, _fclose_r, &ptr->__sglue); -#endif } #ifndef _REENT_ONLY diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c index 858c09ee3..ee991ed24 100644 --- a/newlib/libc/stdio/findfp.c +++ b/newlib/libc/stdio/findfp.c @@ -28,21 +28,9 @@ void (*__stdio_exit_handler) (void); -#if defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS) -const struct __sFILE_fake __sf_fake_stdin = - {_NULL, 0, 0, 0, 0, {_NULL, 0}, 0, _NULL}; -const struct __sFILE_fake __sf_fake_stdout = - {_NULL, 0, 0, 0, 0, {_NULL, 0}, 0, _NULL}; -const struct __sFILE_fake __sf_fake_stderr = - {_NULL, 0, 0, 0, 0, {_NULL, 0}, 0, _NULL}; -#endif - -#ifdef _REENT_GLOBAL_STDIO_STREAMS __FILE __sf[3]; + struct _glue __sglue = {NULL, 3, &__sf[0]}; -#else -struct _glue __sglue = {NULL, 0, NULL}; -#endif #ifdef _STDIO_BSD_SEMANTICS /* BSD and Glibc systems only flush streams which have been written to @@ -161,11 +149,9 @@ global_stdio_init (void) { if (__stdio_exit_handler == NULL) { __stdio_exit_handler = stdio_exit_handler; -#ifdef _REENT_GLOBAL_STDIO_STREAMS stdin_init (&__sf[0]); stdout_init (&__sf[1]); stderr_init (&__sf[2]); -#endif } } @@ -232,16 +218,12 @@ found: static void cleanup_stdio (struct _reent *ptr) { -#ifdef _REENT_GLOBAL_STDIO_STREAMS if (ptr->_stdin != &__sf[0]) CLEANUP_FILE (ptr, ptr->_stdin); if (ptr->_stdout != &__sf[1]) CLEANUP_FILE (ptr, ptr->_stdout); if (ptr->_stderr != &__sf[2]) CLEANUP_FILE (ptr, ptr->_stderr); -#else - (void) _fwalk_sglue (ptr, CLEANUP_FILE, &ptr->__sglue); -#endif } /* @@ -262,22 +244,7 @@ __sinit (struct _reent *s) /* make sure we clean up on exit */ s->__cleanup = cleanup_stdio; /* conservative */ -#ifdef _REENT_SMALL -# ifndef _REENT_GLOBAL_STDIO_STREAMS - s->_stdin = __sfp(s); - s->_stdout = __sfp(s); - s->_stderr = __sfp(s); -# endif /* _REENT_GLOBAL_STDIO_STREAMS */ -#endif - global_stdio_init (); - -#ifndef _REENT_GLOBAL_STDIO_STREAMS - stdin_init (s->_stdin); - stdout_init (s->_stdout); - stderr_init (s->_stderr); -#endif /* _REENT_GLOBAL_STDIO_STREAMS */ - __sfp_lock_release (); } @@ -320,32 +287,14 @@ __fp_unlock (struct _reent * ptr __unused, FILE * fp) void __fp_lock_all (void) { -#ifndef _REENT_GLOBAL_STDIO_STREAMS - struct _reent *ptr; -#endif - __sfp_lock_acquire (); - -#ifndef _REENT_GLOBAL_STDIO_STREAMS - ptr = _REENT; - (void) _fwalk_sglue (ptr, __fp_lock, &ptr->__sglue); -#else (void) _fwalk_sglue (NULL, __fp_lock, &__sglue); -#endif } void __fp_unlock_all (void) { -#ifndef _REENT_GLOBAL_STDIO_STREAMS - struct _reent *ptr; - - ptr = _REENT; - (void) _fwalk_sglue (ptr, __fp_unlock, &ptr->__sglue); -#else (void) _fwalk_sglue (NULL, __fp_unlock, &__sglue); -#endif - __sfp_lock_release (); } #endif diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h index e245fdb4e..9b355e3ac 100644 --- a/newlib/libc/stdio/local.h +++ b/newlib/libc/stdio/local.h @@ -193,22 +193,6 @@ extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *, /* Called by the main entry point fns to ensure stdio has been initialized. */ -#if defined(_REENT_SMALL) && !defined(_REENT_GLOBAL_STDIO_STREAMS) -#define CHECK_INIT(ptr, fp) \ - do \ - { \ - struct _reent *_check_init_ptr = (ptr); \ - if ((_check_init_ptr) && !(_check_init_ptr)->__cleanup) \ - __sinit (_check_init_ptr); \ - if ((fp) == (FILE *)&__sf_fake_stdin) \ - (fp) = _stdin_r(_check_init_ptr); \ - else if ((fp) == (FILE *)&__sf_fake_stdout) \ - (fp) = _stdout_r(_check_init_ptr); \ - else if ((fp) == (FILE *)&__sf_fake_stderr) \ - (fp) = _stderr_r(_check_init_ptr); \ - } \ - while (0) -#else /* !_REENT_SMALL || _REENT_GLOBAL_STDIO_STREAMS */ #define CHECK_INIT(ptr, fp) \ do \ { \ @@ -217,7 +201,6 @@ extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *, __sinit (_check_init_ptr); \ } \ while (0) -#endif /* !_REENT_SMALL || _REENT_GLOBAL_STDIO_STREAMS */ /* Return true and set errno and stream error flag iff the given FILE cannot be written now. */ diff --git a/winsup/cygwin/include/cygwin/config.h b/winsup/cygwin/include/cygwin/config.h index fd3093755..f6d1b68f0 100644 --- a/winsup/cygwin/include/cygwin/config.h +++ b/winsup/cygwin/include/cygwin/config.h @@ -48,7 +48,6 @@ extern inline struct _reent *__getreent (void) /* The following block of macros is required to build newlib correctly for Cygwin. Changing them in applications has no or not the desired effect. Just leave them alone. */ -#define _REENT_GLOBAL_STDIO_STREAMS 1 #define _READ_WRITE_RETURN_TYPE _ssize_t #define _READ_WRITE_BUFSIZE_TYPE size_t #define __LARGE64_FILES 1