diff mbox series

libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr

Message ID 87lg2x5gsf.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr | expand

Commit Message

Florian Weimer Feb. 3, 2019, 10:04 a.m. UTC
These variables are only used to determine if a stdio stream is
a pre-allocated stream, but it is possible to do so by comparing
a FILE * to all pre-allocated stream objects.  As a result, it is
not necessary to keep those pointers in separate variables.

Behavior with symbol interposition is unchanged because _IO_stdin_,
_IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of
libc if symbol interposition or copy relocations are involved.

2019-02-03  Florian Weimer  <fweimer@redhat.com>

	* libio/libio.h (_IO_stdin, _IO_stdout, _IO_stderr): Remove
	declaration.
	* libio/stdio.c (AL, AL2, _IO_stdin, _IO_stdout, _IO_stderr):
	Remove definitions.
	* libio/stdfiles.c: Update comment.
	* libio/oldstdfiles.c (_IO_check_libio): Update comment.  Do not
	set _IO_stdin, _IO_stdout, _IO_stderr.
	* libio/libioP.h (_IO_fake_stdiobuf): Remove unused declaration.
	[SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)] (_IO_legacy_file): New
	inline function.
	(_IO_deallocate_file): New inline function.
	* libio/iolibio.h (_IO_vprintf): Remove definition.
	* libio/iofclose.c (_IO_new_fclose): Use _IO_deallocate_file.
	* libio/oldiofclose.c (_IO_old_fclose): Likewise.
	* libio/iofwide.c (_IO_fwide): Use __glibc_unlikely and
	_IO_legacy_file.
	* libio/oldfileops.c (_IO_old_file_init_internal): Remove
	__builtin_expect.  Use _IO_legacy_file.

Comments

Florian Weimer Feb. 12, 2019, 2:18 p.m. UTC | #1
* Florian Weimer:

> These variables are only used to determine if a stdio stream is
> a pre-allocated stream, but it is possible to do so by comparing
> a FILE * to all pre-allocated stream objects.  As a result, it is
> not necessary to keep those pointers in separate variables.
>
> Behavior with symbol interposition is unchanged because _IO_stdin_,
> _IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of
> libc if symbol interposition or copy relocations are involved.
>
> 2019-02-03  Florian Weimer  <fweimer@redhat.com>
>
> 	* libio/libio.h (_IO_stdin, _IO_stdout, _IO_stderr): Remove
> 	declaration.
> 	* libio/stdio.c (AL, AL2, _IO_stdin, _IO_stdout, _IO_stderr):
> 	Remove definitions.
> 	* libio/stdfiles.c: Update comment.
> 	* libio/oldstdfiles.c (_IO_check_libio): Update comment.  Do not
> 	set _IO_stdin, _IO_stdout, _IO_stderr.
> 	* libio/libioP.h (_IO_fake_stdiobuf): Remove unused declaration.
> 	[SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)] (_IO_legacy_file): New
> 	inline function.
> 	(_IO_deallocate_file): New inline function.
> 	* libio/iolibio.h (_IO_vprintf): Remove definition.
> 	* libio/iofclose.c (_IO_new_fclose): Use _IO_deallocate_file.
> 	* libio/oldiofclose.c (_IO_old_fclose): Likewise.
> 	* libio/iofwide.c (_IO_fwide): Use __glibc_unlikely and
> 	_IO_legacy_file.
> 	* libio/oldfileops.c (_IO_old_file_init_internal): Remove
> 	__builtin_expect.  Use _IO_legacy_file.

Ping?  <https://sourceware.org/ml/libc-alpha/2019-02/msg00057.html>

Thanks,
Florian
Gabriel F. T. Gomes Feb. 12, 2019, 4:36 p.m. UTC | #2
Hi, Florian.

I have a question about the change in libio/oldfileops.c (see below).
The rest of the patch looks good to me.

On Sun, Feb 03 2019, Florian Weimer wrote:
> These variables are only used to determine if a stdio stream is
> a pre-allocated stream, but it is possible to do so by comparing
> a FILE * to all pre-allocated stream objects.  As a result, it is
> not necessary to keep those pointers in separate variables.
> 
> Behavior with symbol interposition is unchanged because _IO_stdin_,
> _IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of
> libc if symbol interposition or copy relocations are involved.
> 
> 2019-02-03  Florian Weimer  <fweimer@redhat.com>
> 
> 	* libio/libio.h (_IO_stdin, _IO_stdout, _IO_stderr): Remove
> 	declaration.
> 	* libio/stdio.c (AL, AL2, _IO_stdin, _IO_stdout, _IO_stderr):
> 	Remove definitions.
> 	* libio/stdfiles.c: Update comment.
> 	* libio/oldstdfiles.c (_IO_check_libio): Update comment.  Do not
> 	set _IO_stdin, _IO_stdout, _IO_stderr.
> 	* libio/libioP.h (_IO_fake_stdiobuf): Remove unused declaration.
> 	[SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)] (_IO_legacy_file): New
> 	inline function.
> 	(_IO_deallocate_file): New inline function.
> 	* libio/iolibio.h (_IO_vprintf): Remove definition.
> 	* libio/iofclose.c (_IO_new_fclose): Use _IO_deallocate_file.
> 	* libio/oldiofclose.c (_IO_old_fclose): Likewise.
> 	* libio/iofwide.c (_IO_fwide): Use __glibc_unlikely and
> 	_IO_legacy_file.
> 	* libio/oldfileops.c (_IO_old_file_init_internal): Remove
> 	__builtin_expect.  Use _IO_legacy_file.
> 
> diff --git a/libio/iofclose.c b/libio/iofclose.c
> index 9b39a6cc4e..8a80dd0b78 100644
> --- a/libio/iofclose.c
> +++ b/libio/iofclose.c
> @@ -71,12 +71,7 @@ _IO_new_fclose (FILE *fp)
>        if (_IO_have_backup (fp))
>  	_IO_free_backup_area (fp);
>      }
> -  if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr)
> -    {
> -      fp->_flags = 0;
> -      free(fp);
> -    }
> -
> +  _IO_deallocate_file (fp);

OK.

>    return status;
>  }
>  
> diff --git a/libio/iofwide.c b/libio/iofwide.c
> index 6676ad5e53..247cfde3d0 100644
> --- a/libio/iofwide.c
> +++ b/libio/iofwide.c
> @@ -87,8 +87,7 @@ _IO_fwide (FILE *fp, int mode)
>    mode = mode < 0 ? -1 : (mode == 0 ? 0 : 1);
>  
>  #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> -  if (__builtin_expect (&_IO_stdin_used == NULL, 0)
> -      && (fp == _IO_stdin || fp == _IO_stdout || fp == _IO_stderr))
> +  if (__glibc_unlikely (&_IO_stdin_used == NULL) && _IO_legacy_file (fp))

OK.

>      /* This is for a stream in the glibc 2.0 format.  */
>      return -1;
>  #endif
> diff --git a/libio/iolibio.h b/libio/iolibio.h
> index 2642d71e4f..9561833655 100644
> --- a/libio/iolibio.h
> +++ b/libio/iolibio.h
> @@ -58,8 +58,6 @@ extern int _IO_vsscanf (const char *, const char *, __gnuc_va_list) __THROW;
>     == _IO_pos_BAD ? EOF : 0)
>  #define _IO_rewind(FILE) \
>    (void) _IO_seekoff_unlocked (FILE, 0, 0, _IOS_INPUT|_IOS_OUTPUT)
> -#define _IO_vprintf(FORMAT, ARGS) \
> -  _IO_vfprintf (_IO_stdout, FORMAT, ARGS)
>  #define _IO_freopen(FILENAME, MODE, FP) \
>    (_IO_file_close_it (FP), \
>     _IO_file_fopen (FP, FILENAME, MODE, 1))
> diff --git a/libio/libio.h b/libio/libio.h
> index d21162aab0..872574cfb7 100644
> --- a/libio/libio.h
> +++ b/libio/libio.h
> @@ -185,9 +185,6 @@ struct _IO_FILE_plus;
>  extern struct _IO_FILE_plus _IO_2_1_stdin_;
>  extern struct _IO_FILE_plus _IO_2_1_stdout_;
>  extern struct _IO_FILE_plus _IO_2_1_stderr_;
> -extern FILE *_IO_stdin attribute_hidden;
> -extern FILE *_IO_stdout attribute_hidden;
> -extern FILE *_IO_stderr attribute_hidden;

The removal.  OK.

>  
>  struct _IO_cookie_file;
>  
> diff --git a/libio/libioP.h b/libio/libioP.h
> index 8c75f15167..1c434ec3a1 100644
> --- a/libio/libioP.h
> +++ b/libio/libioP.h
> @@ -817,7 +817,35 @@ extern int _IO_vscanf (const char *, va_list) __THROW;
>  # endif
>  #endif
>  
> -extern struct _IO_fake_stdiobuf _IO_stdin_buf, _IO_stdout_buf, _IO_stderr_buf;
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> +/* See oldstdfiles.c.  These are the old stream variables.  */
> +extern struct _IO_FILE_plus _IO_stdin_;
> +extern struct _IO_FILE_plus _IO_stdout_;
> +extern struct _IO_FILE_plus _IO_stderr_;
> +
> +static inline bool
> +_IO_legacy_file (FILE *fp)
> +{
> +  return fp == (FILE *) &_IO_stdin_ || fp == (FILE *) &_IO_stdout_
> +    || fp == (FILE *) &_IO_stderr_;
> +}
> +#endif
> +
> +/* Deallocate a stream if it is heap-allocated.  Preallocated
> +   stdin/stdout/stderr streams are not deallocated. */
> +static inline void
> +_IO_deallocate_file (FILE *fp)
> +{
> +  /* The current stream variables.  */
> +  if (fp == (FILE *) &_IO_2_1_stdin_ || fp == (FILE *) &_IO_2_1_stdout_
> +      || fp == (FILE *) &_IO_2_1_stderr_)
> +    return;
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> +  if (_IO_legacy_file (fp))
> +    return;
> +#endif
> +  free (fp);
> +}

OK.

>  
>  #ifdef IO_DEBUG
>  # define CHECK_FILE(FILE, RET) do {			\
> diff --git a/libio/oldfileops.c b/libio/oldfileops.c
> index 4d6c5e3fe7..10f2205e8a 100644
> --- a/libio/oldfileops.c
> +++ b/libio/oldfileops.c
> @@ -109,10 +109,7 @@ _IO_old_file_init_internal (struct _IO_FILE_plus *fp)
>  			     - (int) sizeof (struct _IO_FILE_complete));
>    fp->file._fileno = -1;
>  
> -  if (__builtin_expect (&_IO_stdin_used != NULL, 1)
> -      || (fp != (struct _IO_FILE_plus *) _IO_stdin
> -	  && fp != (struct _IO_FILE_plus *) _IO_stdout
> -	  && fp != (struct _IO_FILE_plus *) _IO_stderr))
> +  if (&_IO_stdin_used != NULL && !_IO_legacy_file ((FILE *) fp))

Why did the `||' become an `&&'?  Is this solving a bug?

>      /* The object is dynamically allocated and large enough.  Initialize
>         the _mode element as well.  */
>      ((struct _IO_FILE_complete *) fp)->_mode = -1;
> diff --git a/libio/oldiofclose.c b/libio/oldiofclose.c
> index e4cbf88566..be5044c3bd 100644
> --- a/libio/oldiofclose.c
> +++ b/libio/oldiofclose.c
> @@ -58,12 +58,7 @@ _IO_old_fclose (FILE *fp)
>    _IO_FINISH (fp);
>    if (_IO_have_backup (fp))
>      _IO_free_backup_area (fp);
> -  if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr)
> -    {
> -      fp->_flags = 0;
> -      free(fp);
> -    }
> -
> +  _IO_deallocate_file (fp);

OK.

>    return status;
>  }
>  
> diff --git a/libio/oldstdfiles.c b/libio/oldstdfiles.c
> index 524e260b7e..2b861cd754 100644
> --- a/libio/oldstdfiles.c
> +++ b/libio/oldstdfiles.c
> @@ -27,11 +27,8 @@
>  #include <shlib-compat.h>
>  #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
>  
> -/* This file provides definitions of _IO_stdin, _IO_stdout, and _IO_stderr
> -   for C code.  Compare stdstreams.cc.
> -   (The difference is that here the vtable field is set to 0,
> -   so the objects defined are not valid C++ objects.  On the other
> -   hand, we don't need a C++ compiler to build this file.) */
> +/* This file provides legacy definitions of _IO_stdin_, _IO_stdout_,
> +   and _IO_stderr_.  See stdfiles.c for the current definitions.  */
>  
>  #define _IO_USE_OLD_IO_FILE
>  #include "libioP.h"
> @@ -78,13 +75,12 @@ _IO_check_libio (void)
>    if (&_IO_stdin_used == NULL)
>      {
>        /* We are using the old one. */
> -      _IO_stdin = stdin = (FILE *) &_IO_stdin_;
> -      _IO_stdout = stdout = (FILE *) &_IO_stdout_;
> -      _IO_stderr = stderr = (FILE *) &_IO_stderr_;
> +      stdin = (FILE *) &_IO_stdin_;
> +      stdout = (FILE *) &_IO_stdout_;
> +      stderr = (FILE *) &_IO_stderr_;

_IO_std* are gone.  OK.

>        _IO_list_all = &_IO_stderr_;
> -      _IO_stdin->_vtable_offset = _IO_stdout->_vtable_offset =
> -	_IO_stderr->_vtable_offset = stdin->_vtable_offset =
> -	stdout->_vtable_offset = stderr->_vtable_offset =
> +      stdin->_vtable_offset = stdout->_vtable_offset
> +	= stderr->_vtable_offset =

Likewise.  OK.

>  	((int) sizeof (struct _IO_FILE)
>  	 - (int) sizeof (struct _IO_FILE_complete));
>      }
> diff --git a/libio/stdfiles.c b/libio/stdfiles.c
> index 605e006474..9c779b47eb 100644
> --- a/libio/stdfiles.c
> +++ b/libio/stdfiles.c
> @@ -25,11 +25,10 @@
>     in files containing the exception.  */
>  
>  
> -/* This file provides definitions of _IO_stdin, _IO_stdout, and _IO_stderr
> -   for C code.  Compare stdstreams.cc.
> -   (The difference is that here the vtable field is set to 0,
> -   so the objects defined are not valid C++ objects.  On the other
> -   hand, we don't need a C++ compiler to build this file.) */
> +/* This file provides definitions of _IO_2_1_stdin_, _IO_2_1_stdout_,
> +   and _IO_2_1_stderr_, the default values of stdin, stdout, stderr.
> +   See oldstdfiles.c for glibc 2.0 legacy definitions without wide
> +   character support.  */
>  
>  #include "libioP.h"
>  
> diff --git a/libio/stdio.c b/libio/stdio.c
> index 1294d2842e..522de44a27 100644
> --- a/libio/stdio.c
> +++ b/libio/stdio.c
> @@ -33,14 +33,3 @@
>  FILE *stdin = (FILE *) &_IO_2_1_stdin_;
>  FILE *stdout = (FILE *) &_IO_2_1_stdout_;
>  FILE *stderr = (FILE *) &_IO_2_1_stderr_;
> -
> -#undef _IO_stdin
> -#undef _IO_stdout
> -#undef _IO_stderr
> -#define AL(name) AL2 (name, _IO_##name)
> -#define AL2(name, al) \
> -  extern __typeof (name) al __attribute__ ((alias (#name),                    \
> -                                            visibility ("hidden")))
> -AL(stdin);
> -AL(stdout);
> -AL(stderr);

Gone.  OK.
Dmitry V. Levin Feb. 13, 2019, 12:51 a.m. UTC | #3
Hi,

On Sun, Feb 03, 2019 at 11:04:48AM +0100, Florian Weimer wrote:
> These variables are only used to determine if a stdio stream is
> a pre-allocated stream, but it is possible to do so by comparing
> a FILE * to all pre-allocated stream objects.  As a result, it is
> not necessary to keep those pointers in separate variables.
> 
> Behavior with symbol interposition is unchanged because _IO_stdin_,
> _IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of
> libc if symbol interposition or copy relocations are involved.
> 
> 2019-02-03  Florian Weimer  <fweimer@redhat.com>
> 
> 	* libio/libio.h (_IO_stdin, _IO_stdout, _IO_stderr): Remove
> 	declaration.
> 	* libio/stdio.c (AL, AL2, _IO_stdin, _IO_stdout, _IO_stderr):
> 	Remove definitions.
> 	* libio/stdfiles.c: Update comment.
> 	* libio/oldstdfiles.c (_IO_check_libio): Update comment.  Do not
> 	set _IO_stdin, _IO_stdout, _IO_stderr.
> 	* libio/libioP.h (_IO_fake_stdiobuf): Remove unused declaration.
> 	[SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)] (_IO_legacy_file): New
> 	inline function.
> 	(_IO_deallocate_file): New inline function.
> 	* libio/iolibio.h (_IO_vprintf): Remove definition.
> 	* libio/iofclose.c (_IO_new_fclose): Use _IO_deallocate_file.
> 	* libio/oldiofclose.c (_IO_old_fclose): Likewise.
> 	* libio/iofwide.c (_IO_fwide): Use __glibc_unlikely and
> 	_IO_legacy_file.
> 	* libio/oldfileops.c (_IO_old_file_init_internal): Remove
> 	__builtin_expect.  Use _IO_legacy_file.

Looks like _IO_legacy_file makes sense only when &_IO_stdin_used == NULL.
If the check was moved inside _IO_legacy_file, then ...

> diff --git a/libio/iofclose.c b/libio/iofclose.c
> index 9b39a6cc4e..8a80dd0b78 100644
> --- a/libio/iofclose.c
> +++ b/libio/iofclose.c
> @@ -71,12 +71,7 @@ _IO_new_fclose (FILE *fp)
>        if (_IO_have_backup (fp))
>  	_IO_free_backup_area (fp);
>      }
> -  if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr)
> -    {
> -      fp->_flags = 0;
> -      free(fp);
> -    }
> -
> +  _IO_deallocate_file (fp);
>    return status;
>  }
>  
> diff --git a/libio/iofwide.c b/libio/iofwide.c
> index 6676ad5e53..247cfde3d0 100644
> --- a/libio/iofwide.c
> +++ b/libio/iofwide.c
> @@ -87,8 +87,7 @@ _IO_fwide (FILE *fp, int mode)
>    mode = mode < 0 ? -1 : (mode == 0 ? 0 : 1);
>  
>  #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> -  if (__builtin_expect (&_IO_stdin_used == NULL, 0)
> -      && (fp == _IO_stdin || fp == _IO_stdout || fp == _IO_stderr))
> +  if (__glibc_unlikely (&_IO_stdin_used == NULL) && _IO_legacy_file (fp))

... this would become
	if (_IO_legacy_file (fp)))

>      /* This is for a stream in the glibc 2.0 format.  */
>      return -1;
>  #endif
> diff --git a/libio/iolibio.h b/libio/iolibio.h
> index 2642d71e4f..9561833655 100644
> --- a/libio/iolibio.h
> +++ b/libio/iolibio.h
> @@ -58,8 +58,6 @@ extern int _IO_vsscanf (const char *, const char *, __gnuc_va_list) __THROW;
>     == _IO_pos_BAD ? EOF : 0)
>  #define _IO_rewind(FILE) \
>    (void) _IO_seekoff_unlocked (FILE, 0, 0, _IOS_INPUT|_IOS_OUTPUT)
> -#define _IO_vprintf(FORMAT, ARGS) \
> -  _IO_vfprintf (_IO_stdout, FORMAT, ARGS)
>  #define _IO_freopen(FILENAME, MODE, FP) \
>    (_IO_file_close_it (FP), \
>     _IO_file_fopen (FP, FILENAME, MODE, 1))
> diff --git a/libio/libio.h b/libio/libio.h
> index d21162aab0..872574cfb7 100644
> --- a/libio/libio.h
> +++ b/libio/libio.h
> @@ -185,9 +185,6 @@ struct _IO_FILE_plus;
>  extern struct _IO_FILE_plus _IO_2_1_stdin_;
>  extern struct _IO_FILE_plus _IO_2_1_stdout_;
>  extern struct _IO_FILE_plus _IO_2_1_stderr_;
> -extern FILE *_IO_stdin attribute_hidden;
> -extern FILE *_IO_stdout attribute_hidden;
> -extern FILE *_IO_stderr attribute_hidden;
>  
>  struct _IO_cookie_file;
>  
> diff --git a/libio/libioP.h b/libio/libioP.h
> index 8c75f15167..1c434ec3a1 100644
> --- a/libio/libioP.h
> +++ b/libio/libioP.h
> @@ -817,7 +817,35 @@ extern int _IO_vscanf (const char *, va_list) __THROW;
>  # endif
>  #endif
>  
> -extern struct _IO_fake_stdiobuf _IO_stdin_buf, _IO_stdout_buf, _IO_stderr_buf;
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> +/* See oldstdfiles.c.  These are the old stream variables.  */
> +extern struct _IO_FILE_plus _IO_stdin_;
> +extern struct _IO_FILE_plus _IO_stdout_;
> +extern struct _IO_FILE_plus _IO_stderr_;
> +
> +static inline bool
> +_IO_legacy_file (FILE *fp)
> +{
> +  return fp == (FILE *) &_IO_stdin_ || fp == (FILE *) &_IO_stdout_
> +    || fp == (FILE *) &_IO_stderr_;
> +}
> +#endif
> +
> +/* Deallocate a stream if it is heap-allocated.  Preallocated
> +   stdin/stdout/stderr streams are not deallocated. */
> +static inline void
> +_IO_deallocate_file (FILE *fp)
> +{
> +  /* The current stream variables.  */
> +  if (fp == (FILE *) &_IO_2_1_stdin_ || fp == (FILE *) &_IO_2_1_stdout_
> +      || fp == (FILE *) &_IO_2_1_stderr_)
> +    return;
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> +  if (_IO_legacy_file (fp))

... this would save us from 3 extra checks for _IO_stdin_, _IO_stdout_,
and _IO_stderr_, and ...

> +    return;
> +#endif
> +  free (fp);
> +}
>  
>  #ifdef IO_DEBUG
>  # define CHECK_FILE(FILE, RET) do {			\
> diff --git a/libio/oldfileops.c b/libio/oldfileops.c
> index 4d6c5e3fe7..10f2205e8a 100644
> --- a/libio/oldfileops.c
> +++ b/libio/oldfileops.c
> @@ -109,10 +109,7 @@ _IO_old_file_init_internal (struct _IO_FILE_plus *fp)
>  			     - (int) sizeof (struct _IO_FILE_complete));
>    fp->file._fileno = -1;
>  
> -  if (__builtin_expect (&_IO_stdin_used != NULL, 1)
> -      || (fp != (struct _IO_FILE_plus *) _IO_stdin
> -	  && fp != (struct _IO_FILE_plus *) _IO_stdout
> -	  && fp != (struct _IO_FILE_plus *) _IO_stderr))
> +  if (&_IO_stdin_used != NULL && !_IO_legacy_file ((FILE *) fp))

... and this would become
	if (!_IO_legacy_file ((FILE *) fp))

Overall a bit cleaner and a bit less error-prone.
Florian Weimer Feb. 18, 2019, 9:58 a.m. UTC | #4
* Dmitry V. Levin:

> Looks like _IO_legacy_file makes sense only when &_IO_stdin_used == NULL.
> If the check was moved inside _IO_legacy_file, then ...

I'm not sure.  We have seen cases where new binaries do not define
_IO_stdin_used, perhaps related to symbol visibility.

<https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634261>
<https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=816059>
<https://bugs.launchpad.net/ubuntu/+source/lua5.3/+bug/1570055>

So at least for the free call (in _IO_deallocate_file after the patch),
I really want to check that the object isn't any of the preallocated
ones, in case the crashes aren't immediate and we have the potential
here for causing heap corruption.

Thanks,
Florian
Florian Weimer Feb. 18, 2019, 11:43 a.m. UTC | #5
* Gabriel F. T. Gomes:

>> diff --git a/libio/oldfileops.c b/libio/oldfileops.c
>> index 4d6c5e3fe7..10f2205e8a 100644
>> --- a/libio/oldfileops.c
>> +++ b/libio/oldfileops.c
>> @@ -109,10 +109,7 @@ _IO_old_file_init_internal (struct _IO_FILE_plus *fp)
>>  			     - (int) sizeof (struct _IO_FILE_complete));
>>    fp->file._fileno = -1;
>>  
>> -  if (__builtin_expect (&_IO_stdin_used != NULL, 1)
>> -      || (fp != (struct _IO_FILE_plus *) _IO_stdin
>> -	  && fp != (struct _IO_FILE_plus *) _IO_stdout
>> -	  && fp != (struct _IO_FILE_plus *) _IO_stderr))
>> +  if (&_IO_stdin_used != NULL && !_IO_legacy_file ((FILE *) fp))
>
> Why did the `||' become an `&&'?  Is this solving a bug?

No, this is was a mistake on my part. 8-( I did not want to change
behavior here.

The old code in _IO_old_file_init_internal looks like this:

  if (__builtin_expect (&_IO_stdin_used != NULL, 1)
      || (fp != (struct _IO_FILE_plus *) _IO_stdin
          && fp != (struct _IO_FILE_plus *) _IO_stdout
          && fp != (struct _IO_FILE_plus *) _IO_stderr))
    /* The object is dynamically allocated and large enough.  Initialize
       the _mode element as well.  */
    ((struct _IO_FILE_complete *) fp)->_mode = -1;

So in a new program (which defines _IO_stdin_used), we would assume the
_mode member is present.  In an old program (no _IO_stdin_used), we look
at _IO_stdin etc., which will have been initialized to point to
_IO_stdin_ etc.  If fp does not point to one of these objects, we write
to _mode.

So the condition matching the old code would indeed be:

  &_IO_stdin_used != NULL || !_IO_legacy_file ((FILE *) fp)

New patch below.

libio is somewhat inconsistent in the way it checks for legacy file
handles.  freopen uses the absence of _IO_stdin_used.  _IO_old_fopen
(the old implementation of fopen) does *not* look at _IO_stdin_used and
always allocates a large struct, which includes the _mode member:

$ gdb libio/oldiofopen.os
…
(gdb) print sizeof(struct _IO_FILE_complete_plus)
$1 = 152

Compare this to:

$ gdb libio/oldstdfiles.os
…
(gdb) print sizeof(_IO_stdin_)
$1 = 80

Presumably, this makes the assignment to _mode safe in all cases where
the object is not one of the pre-allocated _IO_stdin_ (etc.) objects.
It is not safe if a shared object brings its own statically allocated
FILE object (no matter how it is named) and the shared object is loaded
into a new application (which defines _IO_stdin_used).  This object will
not have a _mode member, yet, _IO_old_file_init_internal tries to assign
to it.

We know that _IO_stdin_used is unreliable (see my reply to Dmitry).  The
question is whether we can find a representative set of old binaries to
make changes in this area.

Thanks,
Florian

libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr

These variables are only used to determine if a stdio stream is
a pre-allocated stream, but it is possible to do so by comparing
a FILE * to all pre-allocated stream objects.  As a result, it is
not necessary to keep those pointers in separate variables.

Behavior with symbol interposition is unchanged because _IO_stdin_,
_IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of
libc if symbol interposition or copy relocations are involved.  (The
removed variables _IO_stdin, _IO_stdout, _IO_stderr were not exported,
of course.)

2019-02-18  Florian Weimer  <fweimer@redhat.com>

	* libio/libio.h (_IO_stdin, _IO_stdout, _IO_stderr): Remove
	declaration.
	* libio/stdio.c (AL, AL2, _IO_stdin, _IO_stdout, _IO_stderr):
	Remove definitions.
	* libio/stdfiles.c: Update comment.
	* libio/oldstdfiles.c (_IO_check_libio): Update comment.  Do not
	set _IO_stdin, _IO_stdout, _IO_stderr.
	* libio/libioP.h (_IO_fake_stdiobuf): Remove unused declaration.
	[SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)] (_IO_legacy_file): New
	inline function.
	(_IO_deallocate_file): New inline function.
	* libio/iolibio.h (_IO_vprintf): Remove definition.
	* libio/iofclose.c (_IO_new_fclose): Use _IO_deallocate_file.
	* libio/oldiofclose.c (_IO_old_fclose): Likewise.
	* libio/iofwide.c (_IO_fwide): Use __glibc_unlikely and
	_IO_legacy_file.
	* libio/oldfileops.c (_IO_old_file_init_internal): Remove
	__builtin_expect.  Use _IO_legacy_file.

diff --git a/libio/iofclose.c b/libio/iofclose.c
index 9b39a6cc4e..8a80dd0b78 100644
--- a/libio/iofclose.c
+++ b/libio/iofclose.c
@@ -71,12 +71,7 @@ _IO_new_fclose (FILE *fp)
       if (_IO_have_backup (fp))
 	_IO_free_backup_area (fp);
     }
-  if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr)
-    {
-      fp->_flags = 0;
-      free(fp);
-    }
-
+  _IO_deallocate_file (fp);
   return status;
 }
 
diff --git a/libio/iofwide.c b/libio/iofwide.c
index 6676ad5e53..247cfde3d0 100644
--- a/libio/iofwide.c
+++ b/libio/iofwide.c
@@ -87,8 +87,7 @@ _IO_fwide (FILE *fp, int mode)
   mode = mode < 0 ? -1 : (mode == 0 ? 0 : 1);
 
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
-  if (__builtin_expect (&_IO_stdin_used == NULL, 0)
-      && (fp == _IO_stdin || fp == _IO_stdout || fp == _IO_stderr))
+  if (__glibc_unlikely (&_IO_stdin_used == NULL) && _IO_legacy_file (fp))
     /* This is for a stream in the glibc 2.0 format.  */
     return -1;
 #endif
diff --git a/libio/iolibio.h b/libio/iolibio.h
index 2642d71e4f..9561833655 100644
--- a/libio/iolibio.h
+++ b/libio/iolibio.h
@@ -58,8 +58,6 @@ extern int _IO_vsscanf (const char *, const char *, __gnuc_va_list) __THROW;
    == _IO_pos_BAD ? EOF : 0)
 #define _IO_rewind(FILE) \
   (void) _IO_seekoff_unlocked (FILE, 0, 0, _IOS_INPUT|_IOS_OUTPUT)
-#define _IO_vprintf(FORMAT, ARGS) \
-  _IO_vfprintf (_IO_stdout, FORMAT, ARGS)
 #define _IO_freopen(FILENAME, MODE, FP) \
   (_IO_file_close_it (FP), \
    _IO_file_fopen (FP, FILENAME, MODE, 1))
diff --git a/libio/libio.h b/libio/libio.h
index 9c08c7eba1..c38095ff77 100644
--- a/libio/libio.h
+++ b/libio/libio.h
@@ -185,9 +185,6 @@ struct _IO_FILE_plus;
 extern struct _IO_FILE_plus _IO_2_1_stdin_;
 extern struct _IO_FILE_plus _IO_2_1_stdout_;
 extern struct _IO_FILE_plus _IO_2_1_stderr_;
-extern FILE *_IO_stdin attribute_hidden;
-extern FILE *_IO_stdout attribute_hidden;
-extern FILE *_IO_stderr attribute_hidden;
 
 struct _IO_cookie_file;
 
diff --git a/libio/libioP.h b/libio/libioP.h
index 8c75f15167..1c434ec3a1 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -817,7 +817,35 @@ extern int _IO_vscanf (const char *, va_list) __THROW;
 # endif
 #endif
 
-extern struct _IO_fake_stdiobuf _IO_stdin_buf, _IO_stdout_buf, _IO_stderr_buf;
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
+/* See oldstdfiles.c.  These are the old stream variables.  */
+extern struct _IO_FILE_plus _IO_stdin_;
+extern struct _IO_FILE_plus _IO_stdout_;
+extern struct _IO_FILE_plus _IO_stderr_;
+
+static inline bool
+_IO_legacy_file (FILE *fp)
+{
+  return fp == (FILE *) &_IO_stdin_ || fp == (FILE *) &_IO_stdout_
+    || fp == (FILE *) &_IO_stderr_;
+}
+#endif
+
+/* Deallocate a stream if it is heap-allocated.  Preallocated
+   stdin/stdout/stderr streams are not deallocated. */
+static inline void
+_IO_deallocate_file (FILE *fp)
+{
+  /* The current stream variables.  */
+  if (fp == (FILE *) &_IO_2_1_stdin_ || fp == (FILE *) &_IO_2_1_stdout_
+      || fp == (FILE *) &_IO_2_1_stderr_)
+    return;
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
+  if (_IO_legacy_file (fp))
+    return;
+#endif
+  free (fp);
+}
 
 #ifdef IO_DEBUG
 # define CHECK_FILE(FILE, RET) do {			\
diff --git a/libio/oldfileops.c b/libio/oldfileops.c
index 4d6c5e3fe7..5c319beacd 100644
--- a/libio/oldfileops.c
+++ b/libio/oldfileops.c
@@ -109,10 +109,7 @@ _IO_old_file_init_internal (struct _IO_FILE_plus *fp)
 			     - (int) sizeof (struct _IO_FILE_complete));
   fp->file._fileno = -1;
 
-  if (__builtin_expect (&_IO_stdin_used != NULL, 1)
-      || (fp != (struct _IO_FILE_plus *) _IO_stdin
-	  && fp != (struct _IO_FILE_plus *) _IO_stdout
-	  && fp != (struct _IO_FILE_plus *) _IO_stderr))
+  if (&_IO_stdin_used != NULL || !_IO_legacy_file ((FILE *) fp))
     /* The object is dynamically allocated and large enough.  Initialize
        the _mode element as well.  */
     ((struct _IO_FILE_complete *) fp)->_mode = -1;
diff --git a/libio/oldiofclose.c b/libio/oldiofclose.c
index e4cbf88566..be5044c3bd 100644
--- a/libio/oldiofclose.c
+++ b/libio/oldiofclose.c
@@ -58,12 +58,7 @@ _IO_old_fclose (FILE *fp)
   _IO_FINISH (fp);
   if (_IO_have_backup (fp))
     _IO_free_backup_area (fp);
-  if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr)
-    {
-      fp->_flags = 0;
-      free(fp);
-    }
-
+  _IO_deallocate_file (fp);
   return status;
 }
 
diff --git a/libio/oldstdfiles.c b/libio/oldstdfiles.c
index 524e260b7e..2b861cd754 100644
--- a/libio/oldstdfiles.c
+++ b/libio/oldstdfiles.c
@@ -27,11 +27,8 @@
 #include <shlib-compat.h>
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
 
-/* This file provides definitions of _IO_stdin, _IO_stdout, and _IO_stderr
-   for C code.  Compare stdstreams.cc.
-   (The difference is that here the vtable field is set to 0,
-   so the objects defined are not valid C++ objects.  On the other
-   hand, we don't need a C++ compiler to build this file.) */
+/* This file provides legacy definitions of _IO_stdin_, _IO_stdout_,
+   and _IO_stderr_.  See stdfiles.c for the current definitions.  */
 
 #define _IO_USE_OLD_IO_FILE
 #include "libioP.h"
@@ -78,13 +75,12 @@ _IO_check_libio (void)
   if (&_IO_stdin_used == NULL)
     {
       /* We are using the old one. */
-      _IO_stdin = stdin = (FILE *) &_IO_stdin_;
-      _IO_stdout = stdout = (FILE *) &_IO_stdout_;
-      _IO_stderr = stderr = (FILE *) &_IO_stderr_;
+      stdin = (FILE *) &_IO_stdin_;
+      stdout = (FILE *) &_IO_stdout_;
+      stderr = (FILE *) &_IO_stderr_;
       _IO_list_all = &_IO_stderr_;
-      _IO_stdin->_vtable_offset = _IO_stdout->_vtable_offset =
-	_IO_stderr->_vtable_offset = stdin->_vtable_offset =
-	stdout->_vtable_offset = stderr->_vtable_offset =
+      stdin->_vtable_offset = stdout->_vtable_offset
+	= stderr->_vtable_offset =
 	((int) sizeof (struct _IO_FILE)
 	 - (int) sizeof (struct _IO_FILE_complete));
     }
diff --git a/libio/stdfiles.c b/libio/stdfiles.c
index 605e006474..9c779b47eb 100644
--- a/libio/stdfiles.c
+++ b/libio/stdfiles.c
@@ -25,11 +25,10 @@
    in files containing the exception.  */
 
 
-/* This file provides definitions of _IO_stdin, _IO_stdout, and _IO_stderr
-   for C code.  Compare stdstreams.cc.
-   (The difference is that here the vtable field is set to 0,
-   so the objects defined are not valid C++ objects.  On the other
-   hand, we don't need a C++ compiler to build this file.) */
+/* This file provides definitions of _IO_2_1_stdin_, _IO_2_1_stdout_,
+   and _IO_2_1_stderr_, the default values of stdin, stdout, stderr.
+   See oldstdfiles.c for glibc 2.0 legacy definitions without wide
+   character support.  */
 
 #include "libioP.h"
 
diff --git a/libio/stdio.c b/libio/stdio.c
index 1294d2842e..522de44a27 100644
--- a/libio/stdio.c
+++ b/libio/stdio.c
@@ -33,14 +33,3 @@
 FILE *stdin = (FILE *) &_IO_2_1_stdin_;
 FILE *stdout = (FILE *) &_IO_2_1_stdout_;
 FILE *stderr = (FILE *) &_IO_2_1_stderr_;
-
-#undef _IO_stdin
-#undef _IO_stdout
-#undef _IO_stderr
-#define AL(name) AL2 (name, _IO_##name)
-#define AL2(name, al) \
-  extern __typeof (name) al __attribute__ ((alias (#name),                    \
-                                            visibility ("hidden")))
-AL(stdin);
-AL(stdout);
-AL(stderr);
Dmitry V. Levin Feb. 18, 2019, 11:51 a.m. UTC | #6
On Mon, Feb 18, 2019 at 10:58:47AM +0100, Florian Weimer wrote:
> * Dmitry V. Levin:
> 
> > Looks like _IO_legacy_file makes sense only when &_IO_stdin_used == NULL.
> > If the check was moved inside _IO_legacy_file, then ...
> 
> I'm not sure.  We have seen cases where new binaries do not define
> _IO_stdin_used, perhaps related to symbol visibility.
> 
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634261>
> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=816059>
> <https://bugs.launchpad.net/ubuntu/+source/lua5.3/+bug/1570055>

Yes, and we also have
https://sourceware.org/bugzilla/show_bug.cgi?id=17908

> So at least for the free call (in _IO_deallocate_file after the patch),
> I really want to check that the object isn't any of the preallocated
> ones, in case the crashes aren't immediate and we have the potential
> here for causing heap corruption.

Fair enough.

Why do we check
	(&_IO_stdin_used == NULL) && _IO_legacy_file (fp)
instead of just
	_IO_legacy_file (fp)
then?  Is it just an optimization?
Florian Weimer Feb. 18, 2019, 12:02 p.m. UTC | #7
* Dmitry V. Levin:

> On Mon, Feb 18, 2019 at 10:58:47AM +0100, Florian Weimer wrote:
>> * Dmitry V. Levin:
>> 
>> > Looks like _IO_legacy_file makes sense only when &_IO_stdin_used == NULL.
>> > If the check was moved inside _IO_legacy_file, then ...
>> 
>> I'm not sure.  We have seen cases where new binaries do not define
>> _IO_stdin_used, perhaps related to symbol visibility.
>> 
>> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634261>
>> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=816059>
>> <https://bugs.launchpad.net/ubuntu/+source/lua5.3/+bug/1570055>
>
> Yes, and we also have
> https://sourceware.org/bugzilla/show_bug.cgi?id=17908
>
>> So at least for the free call (in _IO_deallocate_file after the patch),
>> I really want to check that the object isn't any of the preallocated
>> ones, in case the crashes aren't immediate and we have the potential
>> here for causing heap corruption.
>
> Fair enough.
>
> Why do we check
> 	(&_IO_stdin_used == NULL) && _IO_legacy_file (fp)
> instead of just
> 	_IO_legacy_file (fp)
> then?  Is it just an optimization?

Mainly to match the previous (buggy) implementation.

I think in _IO_fwide and _IO_old_file_init_internal, we could perform
the simplified check.  I've looked into that for my reply to Gabriel,
and the code assumes that we over-allocate even in the old
implementation, and only the objects that come from _IO_stdin_ etc. (the
three statically allocated FILE objects) are too small.

I don't think this assumption is entirely correct, and I'd prefer to
match the old behavior as close as possible here.  The reason for
eliminating _IO_stdin is a slight simplification of libio, not to fix
all those bugs.

Thanks,
Florian
Dmitry V. Levin Feb. 18, 2019, 12:54 p.m. UTC | #8
On Mon, Feb 18, 2019 at 12:43:32PM +0100, Florian Weimer wrote:
[...]
> These variables are only used to determine if a stdio stream is
> a pre-allocated stream, but it is possible to do so by comparing
> a FILE * to all pre-allocated stream objects.  As a result, it is
> not necessary to keep those pointers in separate variables.
> 
> Behavior with symbol interposition is unchanged because _IO_stdin_,
> _IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of
> libc if symbol interposition or copy relocations are involved.  (The
> removed variables _IO_stdin, _IO_stdout, _IO_stderr were not exported,
> of course.)
> 
> 2019-02-18  Florian Weimer  <fweimer@redhat.com>
> 
> 	* libio/libio.h (_IO_stdin, _IO_stdout, _IO_stderr): Remove
> 	declaration.
> 	* libio/stdio.c (AL, AL2, _IO_stdin, _IO_stdout, _IO_stderr):
> 	Remove definitions.
> 	* libio/stdfiles.c: Update comment.
> 	* libio/oldstdfiles.c (_IO_check_libio): Update comment.  Do not
> 	set _IO_stdin, _IO_stdout, _IO_stderr.
> 	* libio/libioP.h (_IO_fake_stdiobuf): Remove unused declaration.
> 	[SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)] (_IO_legacy_file): New
> 	inline function.
> 	(_IO_deallocate_file): New inline function.
> 	* libio/iolibio.h (_IO_vprintf): Remove definition.
> 	* libio/iofclose.c (_IO_new_fclose): Use _IO_deallocate_file.
> 	* libio/oldiofclose.c (_IO_old_fclose): Likewise.
> 	* libio/iofwide.c (_IO_fwide): Use __glibc_unlikely and
> 	_IO_legacy_file.
> 	* libio/oldfileops.c (_IO_old_file_init_internal): Remove
> 	__builtin_expect.  Use _IO_legacy_file.

Looks good, thanks.
Gabriel F. T. Gomes Feb. 18, 2019, 3:06 p.m. UTC | #9
On Mon, Feb 18 2019, Florian Weimer wrote:
> 
> So in a new program (which defines _IO_stdin_used), we would assume the
> _mode member is present.  In an old program (no _IO_stdin_used), we look
> at _IO_stdin etc., which will have been initialized to point to
> _IO_stdin_ etc.  If fp does not point to one of these objects, we write
> to _mode.
>
> ...
>
> libio is somewhat inconsistent in the way it checks for legacy file
> handles.  freopen uses the absence of _IO_stdin_used.  _IO_old_fopen
> (the old implementation of fopen) does *not* look at _IO_stdin_used and
> always allocates a large struct, which includes the _mode member:
> 
> $ gdb libio/oldiofopen.os
> …
> (gdb) print sizeof(struct _IO_FILE_complete_plus)
> $1 = 152
> 
> Compare this to:
> 
> $ gdb libio/oldstdfiles.os
> …
> (gdb) print sizeof(_IO_stdin_)
> $1 = 80
> 
> Presumably, this makes the assignment to _mode safe in all cases where
> the object is not one of the pre-allocated _IO_stdin_ (etc.) objects.
> It is not safe if a shared object brings its own statically allocated
> FILE object (no matter how it is named) and the shared object is loaded
> into a new application (which defines _IO_stdin_used).  This object will
> not have a _mode member, yet, _IO_old_file_init_internal tries to assign
> to it.

Thanks for the detailed explanations.

> libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr
> 
> These variables are only used to determine if a stdio stream is
> a pre-allocated stream, but it is possible to do so by comparing
> a FILE * to all pre-allocated stream objects.  As a result, it is
> not necessary to keep those pointers in separate variables.
> 
> Behavior with symbol interposition is unchanged because _IO_stdin_,
> _IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of
> libc if symbol interposition or copy relocations are involved.  (The
> removed variables _IO_stdin, _IO_stdout, _IO_stderr were not exported,
> of course.)

OK.

> -  if (__builtin_expect (&_IO_stdin_used != NULL, 1)
> -      || (fp != (struct _IO_FILE_plus *) _IO_stdin
> -	  && fp != (struct _IO_FILE_plus *) _IO_stdout
> -	  && fp != (struct _IO_FILE_plus *) _IO_stderr))
> +  if (&_IO_stdin_used != NULL || !_IO_legacy_file ((FILE *) fp))

OK.

This version looks good to me. Thanks!
diff mbox series

Patch

diff --git a/libio/iofclose.c b/libio/iofclose.c
index 9b39a6cc4e..8a80dd0b78 100644
--- a/libio/iofclose.c
+++ b/libio/iofclose.c
@@ -71,12 +71,7 @@  _IO_new_fclose (FILE *fp)
       if (_IO_have_backup (fp))
 	_IO_free_backup_area (fp);
     }
-  if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr)
-    {
-      fp->_flags = 0;
-      free(fp);
-    }
-
+  _IO_deallocate_file (fp);
   return status;
 }
 
diff --git a/libio/iofwide.c b/libio/iofwide.c
index 6676ad5e53..247cfde3d0 100644
--- a/libio/iofwide.c
+++ b/libio/iofwide.c
@@ -87,8 +87,7 @@  _IO_fwide (FILE *fp, int mode)
   mode = mode < 0 ? -1 : (mode == 0 ? 0 : 1);
 
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
-  if (__builtin_expect (&_IO_stdin_used == NULL, 0)
-      && (fp == _IO_stdin || fp == _IO_stdout || fp == _IO_stderr))
+  if (__glibc_unlikely (&_IO_stdin_used == NULL) && _IO_legacy_file (fp))
     /* This is for a stream in the glibc 2.0 format.  */
     return -1;
 #endif
diff --git a/libio/iolibio.h b/libio/iolibio.h
index 2642d71e4f..9561833655 100644
--- a/libio/iolibio.h
+++ b/libio/iolibio.h
@@ -58,8 +58,6 @@  extern int _IO_vsscanf (const char *, const char *, __gnuc_va_list) __THROW;
    == _IO_pos_BAD ? EOF : 0)
 #define _IO_rewind(FILE) \
   (void) _IO_seekoff_unlocked (FILE, 0, 0, _IOS_INPUT|_IOS_OUTPUT)
-#define _IO_vprintf(FORMAT, ARGS) \
-  _IO_vfprintf (_IO_stdout, FORMAT, ARGS)
 #define _IO_freopen(FILENAME, MODE, FP) \
   (_IO_file_close_it (FP), \
    _IO_file_fopen (FP, FILENAME, MODE, 1))
diff --git a/libio/libio.h b/libio/libio.h
index d21162aab0..872574cfb7 100644
--- a/libio/libio.h
+++ b/libio/libio.h
@@ -185,9 +185,6 @@  struct _IO_FILE_plus;
 extern struct _IO_FILE_plus _IO_2_1_stdin_;
 extern struct _IO_FILE_plus _IO_2_1_stdout_;
 extern struct _IO_FILE_plus _IO_2_1_stderr_;
-extern FILE *_IO_stdin attribute_hidden;
-extern FILE *_IO_stdout attribute_hidden;
-extern FILE *_IO_stderr attribute_hidden;
 
 struct _IO_cookie_file;
 
diff --git a/libio/libioP.h b/libio/libioP.h
index 8c75f15167..1c434ec3a1 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -817,7 +817,35 @@  extern int _IO_vscanf (const char *, va_list) __THROW;
 # endif
 #endif
 
-extern struct _IO_fake_stdiobuf _IO_stdin_buf, _IO_stdout_buf, _IO_stderr_buf;
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
+/* See oldstdfiles.c.  These are the old stream variables.  */
+extern struct _IO_FILE_plus _IO_stdin_;
+extern struct _IO_FILE_plus _IO_stdout_;
+extern struct _IO_FILE_plus _IO_stderr_;
+
+static inline bool
+_IO_legacy_file (FILE *fp)
+{
+  return fp == (FILE *) &_IO_stdin_ || fp == (FILE *) &_IO_stdout_
+    || fp == (FILE *) &_IO_stderr_;
+}
+#endif
+
+/* Deallocate a stream if it is heap-allocated.  Preallocated
+   stdin/stdout/stderr streams are not deallocated. */
+static inline void
+_IO_deallocate_file (FILE *fp)
+{
+  /* The current stream variables.  */
+  if (fp == (FILE *) &_IO_2_1_stdin_ || fp == (FILE *) &_IO_2_1_stdout_
+      || fp == (FILE *) &_IO_2_1_stderr_)
+    return;
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
+  if (_IO_legacy_file (fp))
+    return;
+#endif
+  free (fp);
+}
 
 #ifdef IO_DEBUG
 # define CHECK_FILE(FILE, RET) do {			\
diff --git a/libio/oldfileops.c b/libio/oldfileops.c
index 4d6c5e3fe7..10f2205e8a 100644
--- a/libio/oldfileops.c
+++ b/libio/oldfileops.c
@@ -109,10 +109,7 @@  _IO_old_file_init_internal (struct _IO_FILE_plus *fp)
 			     - (int) sizeof (struct _IO_FILE_complete));
   fp->file._fileno = -1;
 
-  if (__builtin_expect (&_IO_stdin_used != NULL, 1)
-      || (fp != (struct _IO_FILE_plus *) _IO_stdin
-	  && fp != (struct _IO_FILE_plus *) _IO_stdout
-	  && fp != (struct _IO_FILE_plus *) _IO_stderr))
+  if (&_IO_stdin_used != NULL && !_IO_legacy_file ((FILE *) fp))
     /* The object is dynamically allocated and large enough.  Initialize
        the _mode element as well.  */
     ((struct _IO_FILE_complete *) fp)->_mode = -1;
diff --git a/libio/oldiofclose.c b/libio/oldiofclose.c
index e4cbf88566..be5044c3bd 100644
--- a/libio/oldiofclose.c
+++ b/libio/oldiofclose.c
@@ -58,12 +58,7 @@  _IO_old_fclose (FILE *fp)
   _IO_FINISH (fp);
   if (_IO_have_backup (fp))
     _IO_free_backup_area (fp);
-  if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr)
-    {
-      fp->_flags = 0;
-      free(fp);
-    }
-
+  _IO_deallocate_file (fp);
   return status;
 }
 
diff --git a/libio/oldstdfiles.c b/libio/oldstdfiles.c
index 524e260b7e..2b861cd754 100644
--- a/libio/oldstdfiles.c
+++ b/libio/oldstdfiles.c
@@ -27,11 +27,8 @@ 
 #include <shlib-compat.h>
 #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
 
-/* This file provides definitions of _IO_stdin, _IO_stdout, and _IO_stderr
-   for C code.  Compare stdstreams.cc.
-   (The difference is that here the vtable field is set to 0,
-   so the objects defined are not valid C++ objects.  On the other
-   hand, we don't need a C++ compiler to build this file.) */
+/* This file provides legacy definitions of _IO_stdin_, _IO_stdout_,
+   and _IO_stderr_.  See stdfiles.c for the current definitions.  */
 
 #define _IO_USE_OLD_IO_FILE
 #include "libioP.h"
@@ -78,13 +75,12 @@  _IO_check_libio (void)
   if (&_IO_stdin_used == NULL)
     {
       /* We are using the old one. */
-      _IO_stdin = stdin = (FILE *) &_IO_stdin_;
-      _IO_stdout = stdout = (FILE *) &_IO_stdout_;
-      _IO_stderr = stderr = (FILE *) &_IO_stderr_;
+      stdin = (FILE *) &_IO_stdin_;
+      stdout = (FILE *) &_IO_stdout_;
+      stderr = (FILE *) &_IO_stderr_;
       _IO_list_all = &_IO_stderr_;
-      _IO_stdin->_vtable_offset = _IO_stdout->_vtable_offset =
-	_IO_stderr->_vtable_offset = stdin->_vtable_offset =
-	stdout->_vtable_offset = stderr->_vtable_offset =
+      stdin->_vtable_offset = stdout->_vtable_offset
+	= stderr->_vtable_offset =
 	((int) sizeof (struct _IO_FILE)
 	 - (int) sizeof (struct _IO_FILE_complete));
     }
diff --git a/libio/stdfiles.c b/libio/stdfiles.c
index 605e006474..9c779b47eb 100644
--- a/libio/stdfiles.c
+++ b/libio/stdfiles.c
@@ -25,11 +25,10 @@ 
    in files containing the exception.  */
 
 
-/* This file provides definitions of _IO_stdin, _IO_stdout, and _IO_stderr
-   for C code.  Compare stdstreams.cc.
-   (The difference is that here the vtable field is set to 0,
-   so the objects defined are not valid C++ objects.  On the other
-   hand, we don't need a C++ compiler to build this file.) */
+/* This file provides definitions of _IO_2_1_stdin_, _IO_2_1_stdout_,
+   and _IO_2_1_stderr_, the default values of stdin, stdout, stderr.
+   See oldstdfiles.c for glibc 2.0 legacy definitions without wide
+   character support.  */
 
 #include "libioP.h"
 
diff --git a/libio/stdio.c b/libio/stdio.c
index 1294d2842e..522de44a27 100644
--- a/libio/stdio.c
+++ b/libio/stdio.c
@@ -33,14 +33,3 @@ 
 FILE *stdin = (FILE *) &_IO_2_1_stdin_;
 FILE *stdout = (FILE *) &_IO_2_1_stdout_;
 FILE *stderr = (FILE *) &_IO_2_1_stderr_;
-
-#undef _IO_stdin
-#undef _IO_stdout
-#undef _IO_stderr
-#define AL(name) AL2 (name, _IO_##name)
-#define AL2(name, al) \
-  extern __typeof (name) al __attribute__ ((alias (#name),                    \
-                                            visibility ("hidden")))
-AL(stdin);
-AL(stdout);
-AL(stderr);