[PATCH v2 0/8] Use more flags parameters instead of global bits in stdio

classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 6/8] Add __vsyslog_internal, with same flags as __v*printf_internal.

Adhemerval Zanella-2


On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:

> From: Zack Weinberg <[hidden email]>
>
> Changed since v1:
>
>   - Fixed white-space errors.
>   - Removed internal declaration of __vsyslog_chk, because it's no
>     longer called from within libc.  There's a call to it in the inline
>     definition of vsyslog in bits/syslog.h, but that definition is only
>     inlined in user code with:
>       #if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function
>   - Removed libc_hidden_def and libc_hidden_proto for vsyslog, as it is
>     never called internally (it never was).
>   - Added attribute_hidden to the internal declaration of
>     __vsyslog_internal.  Here are the objdumps of one internal call,
>     before and after this change, on a 32-bits powerpc machine:
>     Without attribute_hidden:
>       $ objdump -d --reloc SYSLOG-PRISTINE-glibc/libc.so | grep "<vsyslog@@GLIBC_2.4>:" -A 17
>       000fc790 <vsyslog@@GLIBC_2.4>:
>          fc790:       94 21 ff f0     stwu    r1,-16(r1)
>          fc794:       38 c0 00 00     li      r6,0
>          fc798:       7c 08 02 a6     mflr    r0
>          fc79c:       42 9f 00 05     bcl     20,4*cr7+so,fc7a0 <vsyslog@@GLIBC_2.4+0x10>
>          fc7a0:       93 c1 00 08     stw     r30,8(r1)
>          fc7a4:       90 01 00 14     stw     r0,20(r1)
>          fc7a8:       7f c8 02 a6     mflr    r30
>          fc7ac:       3f de 00 0a     addis   r30,r30,10
>          fc7b0:       3b de 38 54     addi    r30,r30,14420
>          fc7b4:       4b ff f9 9d     bl      fc150 <__vsyslog_internal>
>          fc7b8:       80 01 00 14     lwz     r0,20(r1)
>          fc7bc:       83 c1 00 08     lwz     r30,8(r1)
>          fc7c0:       38 21 00 10     addi    r1,r1,16
>          fc7c4:       7c 08 03 a6     mtlr    r0
>          fc7c8:       4e 80 00 20     blr
>          fc7cc:       60 00 00 00     nop
>     With attribute_hidden:
>       $ objdump -d --reloc SYSLOG-PATCHED-glibc/libc.so | grep "<vsyslog@@GLIBC_2.4>:" -A 5
>       000fc780 <vsyslog@@GLIBC_2.4>:
>          fc780:       38 c0 00 00     li      r6,0
>          fc784:       4b ff f9 cc     b       fc150 <__vsyslog_internal>
>          fc788:       60 00 00 00     nop
>          fc78c:       60 00 00 00     nop
>
> -- 8< --
> __nldbl___vsyslog_chk will ultimately want to pass PRINTF_LDBL_IS_DBL
> down to __vfprintf_internal *as well as* possibly setting PRINTF_FORTIFY.
> To make that possible, we need a __vsyslog_internal that takes the
> same flags as printf.  The code in misc/syslog.c does also get a
> little simpler.
>
> Tested for powerpc and powerpc64le.

LGTM.

>
> 2018-10-16  Zack Weinberg  <[hidden email]>
>    Gabriel F. T. Gomes  <[hidden email]>
>
> * misc/syslog.c: Include libioP.h, not iolibio.h.
> (__vsyslog_internal): New function with the former body of
> __vsyslog_chk; takes mode_flags argument same as
> __v*printf_internal.  Call __vfprintf_internal directly.
>
> (__vsyslog_chk): Now a wrapper around __vsyslog_internal.
> Remove libc_hidden_def.
> (__syslog, __syslog_chk): Use __vsyslog_internal.
> (__vsyslog): Move to just below __syslog.  Use __vsyslog_internal.
>
> * include/sys/syslog.h: Add multiple inclusion guard.
> Add prototype for __vsyslog_internal.
> Remove declaration and libc_hidden_proto for __vsyslog_chk.
>
> * sysdeps/ieee754/ldbl-opt/nldbl-compat.c (__nldbl___vsyslog_chk):
> Use __vsyslog_internal.
>
> Signed-off-by: Zack Weinberg <[hidden email]>
> Signed-off-by: Gabriel F. T. Gomes <[hidden email]>

We don't use DCO, but copyright assignments.

> ---
>  include/sys/syslog.h                    | 19 ++++++++++-------
>  misc/syslog.c                           | 36 +++++++++++++++++----------------
>  sysdeps/ieee754/ldbl-opt/nldbl-compat.c |  2 +-
>  3 files changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/include/sys/syslog.h b/include/sys/syslog.h
> index 3be3189ed1..89d3479ebc 100644
> --- a/include/sys/syslog.h
> +++ b/include/sys/syslog.h
> @@ -1,11 +1,16 @@
> +#ifndef _LIBC_SYS_SYSLOG_H
> +#define _LIBC_SYS_SYSLOG_H 1
>  #include <misc/sys/syslog.h>
> -
>  #ifndef _ISOMAC
> +
>  libc_hidden_proto (syslog)
> -libc_hidden_proto (vsyslog)
>  
> -extern void __vsyslog_chk (int __pri, int __flag, const char *__fmt,
> -   __gnuc_va_list __ap)
> -     __attribute__ ((__format__ (__printf__, 3, 0)));
> -libc_hidden_proto (__vsyslog_chk)
> -#endif
> +/* __vsyslog_internal uses the same mode_flags bits as
> +   __v*printf_internal; see libio/libioP.h.  */
> +extern void __vsyslog_internal (int pri, const char *fmt, __gnuc_va_list ap,
> + unsigned int mode_flags)
> +     attribute_hidden
> +     __attribute__ ((__format__ (__printf__, 2, 0)));
> +
> +#endif /* _ISOMAC */
> +#endif /* syslog.h */

Ok.

> diff --git a/misc/syslog.c b/misc/syslog.c
> index 644dbe80ec..3a15da41ce 100644
> --- a/misc/syslog.c
> +++ b/misc/syslog.c
> @@ -53,7 +53,7 @@ static char sccsid[] = "@(#)syslog.c 8.4 (Berkeley) 3/18/94";
>  
>  #include <stdarg.h>
>  
> -#include <libio/iolibio.h>
> +#include <libio/libioP.h>
>  #include <math_ldbl_opt.h>
>  
>  #include <kernel-features.h>
> @@ -114,24 +114,38 @@ __syslog(int pri, const char *fmt, ...)
>   va_list ap;
>  
>   va_start(ap, fmt);
> - __vsyslog_chk(pri, -1, fmt, ap);
> + __vsyslog_internal(pri, fmt, ap, 0);
>   va_end(ap);
>  }
>  ldbl_hidden_def (__syslog, syslog)
>  ldbl_strong_alias (__syslog, syslog)
>  
> +void
> +__vsyslog(int pri, const char *fmt, va_list ap)
> +{
> + __vsyslog_internal(pri, fmt, ap, 0);
> +}
> +ldbl_weak_alias (__vsyslog, vsyslog)
> +

Ok.

>  void
>  __syslog_chk(int pri, int flag, const char *fmt, ...)
>  {
>   va_list ap;
>  
>   va_start(ap, fmt);
> - __vsyslog_chk(pri, flag, fmt, ap);
> + __vsyslog_internal(pri, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
>   va_end(ap);
>  }
>  
>  void
>  __vsyslog_chk(int pri, int flag, const char *fmt, va_list ap)
> +{
> + __vsyslog_internal(pri, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
> +}
> +
> +void
> +__vsyslog_internal(int pri, const char *fmt, va_list ap,
> +   unsigned int mode_flags)
>  {
>   struct tm now_tm;
>   time_t now;
> @@ -215,11 +229,8 @@ __vsyslog_chk(int pri, int flag, const char *fmt, va_list ap)
>      __set_errno (saved_errno);
>  
>      /* We have the header.  Print the user's format into the
> -               buffer.  */
> -    if (flag == -1)
> -      vfprintf (f, fmt, ap);
> -    else
> -      __vfprintf_chk (f, flag, fmt, ap);
> +       buffer.  */
> +    __vfprintf_internal (f, fmt, ap, mode_flags);
>  
>      /* Close the memory stream; this will finalize the data
>         into a malloc'd buffer in BUF.  */
> @@ -316,15 +327,6 @@ __vsyslog_chk(int pri, int flag, const char *fmt, va_list ap)
>   if (buf != failbuf)
>   free (buf);
>  }
> -libc_hidden_def (__vsyslog_chk)
> -
> -void
> -__vsyslog(int pri, const char *fmt, va_list ap)
> -{
> -  __vsyslog_chk (pri, -1, fmt, ap);
> -}
> -ldbl_hidden_def (__vsyslog, vsyslog)
> -ldbl_weak_alias (__vsyslog, vsyslog)
>  
>  static struct sockaddr_un SyslogAddr; /* AF_UNIX address of local logger */
>  

Ok.

> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> index bda84af0bb..958bbc1834 100644
> --- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> @@ -843,7 +843,7 @@ attribute_compat_text_section
>  __nldbl___vsyslog_chk (int pri, int flag, const char *fmt, va_list ap)
>  {
>    set_no_long_double ();
> -  __vsyslog_chk (pri, flag, fmt, ap);
> +  __vsyslog_internal (pri, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
>    clear_no_long_double ();
>  }
>  libc_hidden_def (__nldbl___vsyslog_chk)
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.

Adhemerval Zanella-2
In reply to this post by Gabriel F. T. Gomes-2


On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:

> From: Zack Weinberg <[hidden email]>
>
> Changes since v1:
>
>   - Fixed white-space errors.
>   - Updated commit message.
>   - In the declaration of __vsnprintf_internal, in libio/libioP.h,
>     mention that passing -1 to the maxlen argument is the behavior of
>     ordinary (v)sprintf function (this was already described in the
>     commit message, but seems relevant to the code itself).
>
> -- 8< --
> The _chk variants of all of the printf functions become much simpler.
> This is the last thing that we needed _IO_acquire_lock_clear_flags2
> for, so it can go as well.  I took the opportunity to make the headers
> included and the names of all local variables consistent across all the
> affected files.
>
> Since we ultimately want to get rid of __no_long_double as well, it
> must be possible to get all of the nontrivial effects of the _chk
> functions by calling the _internal functions with appropriate flags.
> For most of the __(v)xprintf_chk functions, this is covered by
> PRINTF_FORTIFY plus some up-front argument checks that can be
> duplicated.  However, __(v)sprintf_chk installs a custom jump table so
> that it can crash instead of overflowing the output buffer.  This
> functionality is moved to __vsprintf_internal, which now has a
> 'maxlen' argument like __vsnprintf_internal; to get the unsafe
> behavior of ordinary (v)sprintf, pass -1 for that argument.
>
> obstack_printf_chk and obstack_vprintf_chk are no longer in the same
> file.
>
> Tested for powerpc and powerpc64le.

LGTM with two minor issues below.

>
> 2018-10-24  Zack Weinberg  <[hidden email]>
>    Gabriel F. T. Gomes  <[hidden email]>
>
> * libio/iovsprintf.c (_IO_str_chk_overflow, libio_vtable):
> Moved here from debug/vsprintf_chk.c.
> (__vsprintf_internal): Add 'maxlen' argument.  Change the setup
> and completion logic for the strfile to match exactly what
> __vsprintf_chk used to do, except, when maxlen is -1, pass -1 to
> _IO_str_init_static_internal instead of maxlen-1.
> (__vsprintf): Pass -1 as maxlen to __vsprintf_internal.
> * stdio-common/sprintf.c (__sprintf): Pass -1 as maxlen to
> __vsprintf_internal.
>
> * debug/vsprintf_chk.c (__vsprintf_chk)
> * debug/sprintf_chk.c (__sprintf_chk):
> Directly call __vsprintf_internal, passing PRINTF_FORTIFY if
> 'flags' argument is positive, and slen as maxlen.  No need to lock
> the FILE and/or construct a temporary FILE.  Minimize and normalize
> header inclusions and variable names.  Do not libc_hidden_def anything.
>
> * debug/asprintf_chk.c (__asprintf_chk)
> * debug/dprintf_chk.c (__dprintf_chk)
> * debug/fprintf_chk.c (__fprintf_chk)
> * debug/fwprintf_chk.c (__fwprintf_chk)
> * debug/printf_chk.c (__printf_chk)
> * debug/snprintf_chk.c (__snprintf_chk)
> * debug/swprintf_chk.c (__swprintf_chk)
> * debug/vasprintf_chk.c (__vasprintf_chk)
> * debug/vdprintf_chk.c (__vdprintf_chk)
> * debug/vfprintf_chk.c (__vfprintf_chk)
> * debug/vfwprintf_chk.c (__vfwprintf_chk)
> * debug/vprintf_chk.c (__vprintf_chk)
> * debug/vsnprintf_chk.c (__vsnprintf_chk)
> * debug/vswprintf_chk.c (__vswprintf_chk)
> * debug/vwprintf_chk.c (__vwprintf_chk)
> * debug/wprintf_chk.c (__wprintf_chk):
> Directly call the corresponding vxxprintf_internal function, passing
> PRINTF_FORTIFY if 'flag' argument is positive. No need to lock
> the FILE and/or construct a temporary FILE.  Minimize and normalize
> header inclusions and variable names.  Do not libc_hidden_def anything.
>
> * debug/obprintf_chk.c (__obstack_printf_chk): Directly call
> __obstack_vprintf_internal.
> (__obstack_vprintf_chk): Convert into a wrapper that calls
> __obstack_vprintf_internal (these two functions already had the
> same code) and move to new file...
> * debug/vobprintf_chk.c (__obstack_vprintf_chk): ... here.  New
> file.
> * debug/obprintf.c (__obstack_vprintf_internal): Remove the checking of
> the flags argument and the setting of _IO_FLAGS2_FORTIFY.
> * debug/Makefile (routines): Add vobprintf_chk.
>
> * sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> (__nldbl___vsprintf): Pass -1 as maxlen to __vsprintf_internal.
> (__nldbl___vfprintf_chk, __nldbl___vsnprintf_chk)
> (__nldbl___vsprintf_chk, __nldbl___vswprintf_chk)
> (__nldbl___vasprintf_chk, __nldbl___vdprintf_chk)
> (__nldbl___obstack_vfprintf_chk):
> Directly call the corresponding vxxprintf_internal function,
> passing PRINTF_FORTIFY if 'flag' argument is positive.  If necessary,
> duplicate comparison of slen with 0 or maxlen from the corresponding
> non-__nldbl function.
>
> * include/stdio.h (__vsnprintf_chk, __vfprintf_chk, __vasprintf_chk)
> (__vdprintf_chk, __obstack_vfprintf_chk): Remove libc_hidden_proto.
> * include/wchar.h (__vfwprintf_chk, __vswprintf_chk):
> Remove libc_hidden_proto.
>
> * stdio-common/vfprintf-internal.c
> (__vfprintf_internal, __vfwprintf_internal):
> Do not check _IO_FLAGS2_FORTIFY.
> * libio/libio.h (_IO_FLAGS2_FORTIFY): Remove.
> * libio/libioP.h: Update prototype of __vsprintf_internal and add
> a comment explaining why it has the maxlen argument.
> (_IO_acquire_lock_clear_flags2_fct): Remove.
> (_IO_acquire_lock_clear_flags2): Remove.
> (_IO_release_lock): Remove conditional statement which will
> now never execute.
> (_IO_acquire_lock): Remove variable which is now unused.
> * sysdeps/generic/stdio-lock.h (_IO_acquire_lock_clear_flags2): Remove.
> * sysdeps/nptl/stdio-lock.h (_IO_acquire_lock_clear_flags2): Remove.
>
> Signed-off-by: Zack Weinberg <[hidden email]>
> Signed-off-by: Gabriel F. T. Gomes <[hidden email]>

We don't use DCO, but copyright assignments.

> ---
>  debug/Makefile                          |  2 +-
>  debug/asprintf_chk.c                    | 20 +++----
>  debug/dprintf_chk.c                     | 20 +++----
>  debug/fprintf_chk.c                     | 20 +++----
>  debug/fwprintf_chk.c                    | 20 +++----
>  debug/obprintf_chk.c                    | 96 ++++-----------------------------
>  debug/printf_chk.c                      | 20 +++----
>  debug/snprintf_chk.c                    | 24 +++++----
>  debug/sprintf_chk.c                     | 25 +++++----
>  debug/swprintf_chk.c                    | 27 ++++++----
>  debug/vasprintf_chk.c                   | 68 ++---------------------
>  debug/vdprintf_chk.c                    | 37 ++-----------
>  debug/vfprintf_chk.c                    | 21 ++------
>  debug/vfwprintf_chk.c                   | 21 ++------
>  debug/vobprintf_chk.c                   | 32 +++++++++++
>  debug/vprintf_chk.c                     | 20 ++-----
>  debug/vsnprintf_chk.c                   | 46 +++-------------
>  debug/vsprintf_chk.c                    | 69 +++---------------------
>  debug/vswprintf_chk.c                   | 51 +++---------------
>  debug/vwprintf_chk.c                    | 21 ++------
>  debug/wprintf_chk.c                     | 21 +++-----
>  include/stdio.h                         |  5 --
>  include/wchar.h                         |  2 -
>  libio/iovsprintf.c                      | 54 +++++++++++++++++--
>  libio/libio.h                           |  1 -
>  libio/libioP.h                          | 27 ++++------
>  stdio-common/sprintf.c                  |  2 +-
>  stdio-common/vfprintf-internal.c        |  2 -
>  sysdeps/generic/stdio-lock.h            |  7 ---
>  sysdeps/ieee754/ldbl-opt/nldbl-compat.c | 32 +++++++----
>  sysdeps/nptl/stdio-lock.h               |  7 ---
>  31 files changed, 270 insertions(+), 550 deletions(-)
>  create mode 100644 debug/vobprintf_chk.c
>
> diff --git a/debug/Makefile b/debug/Makefile
> index 506cebc3c4..2ef08cf23b 100644
> --- a/debug/Makefile
> +++ b/debug/Makefile
> @@ -45,7 +45,7 @@ routines  = backtrace backtracesyms backtracesymsfd noophooks \
>      gethostname_chk getdomainname_chk wcrtomb_chk mbsnrtowcs_chk \
>      wcsnrtombs_chk mbsrtowcs_chk wcsrtombs_chk mbstowcs_chk \
>      wcstombs_chk asprintf_chk vasprintf_chk dprintf_chk \
> -    vdprintf_chk obprintf_chk \
> +    vdprintf_chk obprintf_chk vobprintf_chk \
>      longjmp_chk ____longjmp_chk \
>      fdelt_chk poll_chk ppoll_chk \
>      explicit_bzero_chk \

Ok.

> diff --git a/debug/asprintf_chk.c b/debug/asprintf_chk.c
> index 9cd4143f2e..eb885c35ca 100644
> --- a/debug/asprintf_chk.c
> +++ b/debug/asprintf_chk.c
> @@ -15,22 +15,24 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <libioP.h>
>  #include <stdarg.h>
> -#include <stdio.h>
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output from FORMAT to a string which is
>     allocated with malloc and stored in *STRING_PTR.  */
>  int
> -__asprintf_chk (char **result_ptr, int flags, const char *format, ...)
> +__asprintf_chk (char **result_ptr, int flag, const char *format, ...)
>  {
> -  va_list arg;
> -  int done;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, format);
> -  done = __vasprintf_chk (result_ptr, flags, format, arg);
> -  va_end (arg);
> +  va_start (ap, format);
> +  ret = __vasprintf_internal (result_ptr, format, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }

Ok.

> diff --git a/debug/dprintf_chk.c b/debug/dprintf_chk.c
> index df3867c61c..b5c62827c0 100644
> --- a/debug/dprintf_chk.c
> +++ b/debug/dprintf_chk.c
> @@ -15,21 +15,23 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <libioP.h>
>  #include <stdarg.h>
> -#include <stdio.h>
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to D, according to the format string FORMAT.  */
>  int
> -__dprintf_chk (int d, int flags, const char *format, ...)
> +__dprintf_chk (int d, int flag, const char *format, ...)
>  {
> -  va_list arg;
> -  int done;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, format);
> -  done = __vdprintf_chk (d, flags, format, arg);
> -  va_end (arg);
> +  va_start (ap, format);
> +  ret = __vdprintf_internal (d, format, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }

Ok.

> diff --git a/debug/fprintf_chk.c b/debug/fprintf_chk.c
> index cff4438afb..14afc073b2 100644
> --- a/debug/fprintf_chk.c
> +++ b/debug/fprintf_chk.c
> @@ -16,29 +16,23 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <stdarg.h>
> -#include <stdio.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to FP from the format string FORMAT.  */
>  int
>  ___fprintf_chk (FILE *fp, int flag, const char *format, ...)
>  {
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>    va_list ap;
> -  int done;
> -
> -  _IO_acquire_lock_clear_flags2 (fp);
> -  if (flag > 0)
> -    fp->_flags2 |= _IO_FLAGS2_FORTIFY;
> +  int ret;
>  
>    va_start (ap, format);
> -  done = vfprintf (fp, format, ap);
> +  ret = __vfprintf_internal (fp, format, ap, mode);
>    va_end (ap);
>  
> -  if (flag > 0)
> -    fp->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (fp);
> -
> -  return done;
> +  return ret;
>  }
>  ldbl_strong_alias (___fprintf_chk, __fprintf_chk)

Ok.

> diff --git a/debug/fwprintf_chk.c b/debug/fwprintf_chk.c
> index 63167c1839..10d84ce98b 100644
> --- a/debug/fwprintf_chk.c
> +++ b/debug/fwprintf_chk.c
> @@ -16,28 +16,22 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <stdarg.h>
> -#include <wchar.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to FP from the format string FORMAT.  */
>  int
>  __fwprintf_chk (FILE *fp, int flag, const wchar_t *format, ...)
>  {
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>    va_list ap;
> -  int done;
> -
> -  _IO_acquire_lock_clear_flags2 (fp);
> -  if (flag > 0)
> -    fp->_flags2 |= _IO_FLAGS2_FORTIFY;
> +  int ret;
>  
>    va_start (ap, format);
> -  done = __vfwprintf_internal (fp, format, ap, 0);
> +  ret = __vfwprintf_internal (fp, format, ap, mode);
>    va_end (ap);
>  
> -  if (flag > 0)
> -    fp->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (fp);
> -
> -  return done;
> +  return ret;
>  }

Ok.

> diff --git a/debug/obprintf_chk.c b/debug/obprintf_chk.c
> index 41dd481c34..c1a8f9e9a9 100644
> --- a/debug/obprintf_chk.c
> +++ b/debug/obprintf_chk.c
> @@ -17,99 +17,23 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -
> -#include <stdlib.h>
> -#include <libioP.h>
> -#include "../libio/strfile.h"
> -#include <assert.h>
> -#include <string.h>
> -#include <errno.h>
> -#include <obstack.h>
> +#include <libio/libioP.h>
>  #include <stdarg.h>
> -#include <stdio_ext.h>
> -
> -
> -struct _IO_obstack_file
> -{
> -  struct _IO_FILE_plus file;
> -  struct obstack *obstack;
> -};
> -
> -extern const struct _IO_jump_t _IO_obstack_jumps libio_vtable attribute_hidden;
> -
> -int
> -__obstack_vprintf_chk (struct obstack *obstack, int flags, const char *format,
> -       va_list args)
> -{
> -  struct obstack_FILE
> -    {
> -      struct _IO_obstack_file ofile;
> -    } new_f;
> -  int result;
> -  int size;
> -  int room;
> -
> -#ifdef _IO_MTSAFE_IO
> -  new_f.ofile.file.file._lock = NULL;
> -#endif
> -
> -  _IO_no_init (&new_f.ofile.file.file, _IO_USER_LOCK, -1, NULL, NULL);
> -  _IO_JUMPS (&new_f.ofile.file) = &_IO_obstack_jumps;
> -  room = obstack_room (obstack);
> -  size = obstack_object_size (obstack) + room;
> -  if (size == 0)
> -    {
> -      /* We have to handle the allocation a bit different since the
> - `_IO_str_init_static' function would handle a size of zero
> - different from what we expect.  */
> -
> -      /* Get more memory.  */
> -      obstack_make_room (obstack, 64);
> -
> -      /* Recompute how much room we have.  */
> -      room = obstack_room (obstack);
> -      size = room;
> -
> -      assert (size != 0);
> -    }
> -
> -  _IO_str_init_static_internal ((struct _IO_strfile_ *) &new_f.ofile,
> - obstack_base (obstack),
> - size, obstack_next_free (obstack));
> -  /* Now allocate the rest of the current chunk.  */
> -  assert (size == (new_f.ofile.file.file._IO_write_end
> -   - new_f.ofile.file.file._IO_write_base));
> -  assert (new_f.ofile.file.file._IO_write_ptr
> -  == (new_f.ofile.file.file._IO_write_base
> -      + obstack_object_size (obstack)));
> -  obstack_blank_fast (obstack, room);
> -
> -  new_f.ofile.obstack = obstack;
> -
> -  /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> -     can only come from read-only format strings.  */
> -  if (flags > 0)
> -    new_f.ofile.file.file._flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  result = __vfprintf_internal (&new_f.ofile.file.file, format, args, 0);
> -
> -  /* Shrink the buffer to the space we really currently need.  */
> -  obstack_blank_fast (obstack, (new_f.ofile.file.file._IO_write_ptr
> - - new_f.ofile.file.file._IO_write_end));
> -
> -  return result;
> -}
> -libc_hidden_def (__obstack_vprintf_chk)
>  

Ok.

>  int
> -__obstack_printf_chk (struct obstack *obstack, int flags, const char *format,
> +__obstack_printf_chk (struct obstack *obstack, int flag, const char *format,
>        ...)
>  {
> -  int result;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>    va_list ap;
> +  int ret;
> +
>    va_start (ap, format);
> -  result = __obstack_vprintf_chk (obstack, flags, format, ap);
> +  ret = __obstack_vprintf_internal (obstack, format, ap, mode);
>    va_end (ap);
> -  return result;
> +
> +  return ret;
>  }

Ok.

> diff --git a/debug/printf_chk.c b/debug/printf_chk.c
> index 426dc78386..e035b42590 100644
> --- a/debug/printf_chk.c
> +++ b/debug/printf_chk.c
> @@ -16,29 +16,23 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <stdarg.h>
> -#include <stdio.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to stdout from the format string FORMAT.  */
>  int
>  ___printf_chk (int flag, const char *format, ...)
>  {
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>    va_list ap;
> -  int done;
> -
> -  _IO_acquire_lock_clear_flags2 (stdout);
> -  if (flag > 0)
> -    stdout->_flags2 |= _IO_FLAGS2_FORTIFY;
> +  int ret;
>  
>    va_start (ap, format);
> -  done = vfprintf (stdout, format, ap);
> +  ret = __vfprintf_internal (stdout, format, ap, mode);
>    va_end (ap);
>  
> -  if (flag > 0)
> -    stdout->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (stdout);
> -
> -  return done;
> +  return ret;
>  }
>  ldbl_strong_alias (___printf_chk, __printf_chk)

Ok.

> diff --git a/debug/snprintf_chk.c b/debug/snprintf_chk.c
> index cddba37109..984b5e8932 100644
> --- a/debug/snprintf_chk.c
> +++ b/debug/snprintf_chk.c
> @@ -15,25 +15,29 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <libioP.h>
>  #include <stdarg.h>
> -#include <stdio.h>
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output into S, according to the format
>     string FORMAT, writing no more than MAXLEN characters.  */
> -/* VARARGS5 */
>  int
> -___snprintf_chk (char *s, size_t maxlen, int flags, size_t slen,
> +___snprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
>   const char *format, ...)
>  {
> -  va_list arg;
> -  int done;
> +  if (__glibc_unlikely (slen < maxlen))
> +    __chk_fail ();
>  
> -  va_start (arg, format);
> -  done = __vsnprintf_chk (s, maxlen, flags, slen, format, arg);
> -  va_end (arg);
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
> +  va_list ap;
> +  int ret;
>  
> -  return done;
> +  va_start (ap, format);
> +  ret = __vsnprintf_internal (s, maxlen, format, ap, mode);
> +  va_end (ap);
> +
> +  return ret;
>  }
>  ldbl_strong_alias (___snprintf_chk, __snprintf_chk)

Ok.

> diff --git a/debug/sprintf_chk.c b/debug/sprintf_chk.c
> index 78214563dd..649e8ab4d5 100644
> --- a/debug/sprintf_chk.c
> +++ b/debug/sprintf_chk.c
> @@ -15,22 +15,27 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <libioP.h>
>  #include <stdarg.h>
> -#include <stdio.h>
> +#include <libio/libioP.h>
> +
>  
>  /* Write formatted output into S, according to the format string FORMAT.  */
> -/* VARARGS4 */
>  int
> -___sprintf_chk (char *s, int flags, size_t slen, const char *format, ...)
> +___sprintf_chk (char *s, int flag, size_t slen, const char *format, ...)
>  {
> -  va_list arg;
> -  int done;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
> +  va_list ap;
> +  int ret;
> +
> +  if (slen == 0)
> +    __chk_fail ();

Maybe a __glibc_unlikely here?

>  
> -  va_start (arg, format);
> -  done = __vsprintf_chk (s, flags, slen, format, arg);
> -  va_end (arg);
> +  va_start (ap, format);
> +  ret = __vsprintf_internal (s, slen, format, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  ldbl_strong_alias (___sprintf_chk, __sprintf_chk)

Ok.

> diff --git a/debug/swprintf_chk.c b/debug/swprintf_chk.c
> index 35887e48e2..186c17751c 100644
> --- a/debug/swprintf_chk.c
> +++ b/debug/swprintf_chk.c
> @@ -16,20 +16,27 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <stdarg.h>
> -#include <wchar.h>
> +#include <libio/libioP.h>
>  
> -/* Write formatted output into S, according to the format string FORMAT.  */
> -/* VARARGS5 */
> +
> +/* Write formatted output into S, according to the format string FORMAT,
> +   writing no more than MAXLEN characters.  */
>  int
> -__swprintf_chk (wchar_t *s, size_t n, int flag, size_t s_len,
> +__swprintf_chk (wchar_t *s, size_t maxlen, int flag, size_t slen,
>   const wchar_t *format, ...)
>  {
> -  va_list arg;
> -  int done;
> +  if (__glibc_unlikely (slen < maxlen))
> +    __chk_fail ();
> +
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, format);
> -  done = __vswprintf_chk (s, n, flag, s_len, format, arg);
> -  va_end (arg);
> +  va_start (ap, format);
> +  ret = __vswprintf_internal (s, maxlen, format, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }

Ok.

> diff --git a/debug/vasprintf_chk.c b/debug/vasprintf_chk.c
> index dbfebff83f..f5975ea02a 100644
> --- a/debug/vasprintf_chk.c
> +++ b/debug/vasprintf_chk.c
> @@ -24,72 +24,14 @@
>     This exception applies to code released by its copyright holders
>     in files containing the exception.  */
>  
> -#include <malloc.h>
> -#include <string.h>
> -#include <stdio.h>
> -#include <stdio_ext.h>
> -#include "../libio/libioP.h"
> -#include "../libio/strfile.h"
> +#include <libio/libioP.h>
>  
>  int
> -__vasprintf_chk (char **result_ptr, int flags, const char *format,
> - va_list args)
> +__vasprintf_chk (char **result_ptr, int flag, const char *format, va_list ap)
>  {
> -  /* Initial size of the buffer to be used.  Will be doubled each time an
> -     overflow occurs.  */
> -  const size_t init_string_size = 100;
> -  char *string;
> -  _IO_strfile sf;
> -  int ret;
> -  size_t needed;
> -  size_t allocated;
> -  /* No need to clear the memory here (unlike for open_memstream) since
> -     we know we will never seek on the stream.  */
> -  string = (char *) malloc (init_string_size);
> -  if (string == NULL)
> -    return -1;
> -#ifdef _IO_MTSAFE_IO
> -  sf._sbf._f._lock = NULL;
> -#endif
> -  _IO_no_init (&sf._sbf._f, _IO_USER_LOCK, -1, NULL, NULL);
> -  _IO_JUMPS (&sf._sbf) = &_IO_str_jumps;
> -  _IO_str_init_static_internal (&sf, string, init_string_size, string);
> -  sf._sbf._f._flags &= ~_IO_USER_BUF;
> -  sf._s._allocate_buffer_unused = (_IO_alloc_type) malloc;
> -  sf._s._free_buffer_unused = (_IO_free_type) free;
> -
> -  /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
>       can only come from read-only format strings.  */
> -  if (flags > 0)
> -    sf._sbf._f._flags2 |= _IO_FLAGS2_FORTIFY;
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  ret = __vfprintf_internal (&sf._sbf._f, format, args, 0);
> -  if (ret < 0)
> -    {
> -      free (sf._sbf._f._IO_buf_base);
> -      return ret;
> -    }
> -  /* Only use realloc if the size we need is of the same (binary)
> -     order of magnitude then the memory we allocated.  */
> -  needed = sf._sbf._f._IO_write_ptr - sf._sbf._f._IO_write_base + 1;
> -  allocated = sf._sbf._f._IO_write_end - sf._sbf._f._IO_write_base;
> -  if ((allocated >> 1) <= needed)
> -    *result_ptr = (char *) realloc (sf._sbf._f._IO_buf_base, needed);
> -  else
> -    {
> -      *result_ptr = (char *) malloc (needed);
> -      if (*result_ptr != NULL)
> - {
> -  memcpy (*result_ptr, sf._sbf._f._IO_buf_base, needed - 1);
> -  free (sf._sbf._f._IO_buf_base);
> - }
> -      else
> - /* We have no choice, use the buffer we already have.  */
> - *result_ptr = (char *) realloc (sf._sbf._f._IO_buf_base, needed);
> -    }
> -  if (*result_ptr == NULL)
> -    *result_ptr = sf._sbf._f._IO_buf_base;
> -  (*result_ptr)[needed - 1] = '\0';
> -  return ret;
> +  return __vasprintf_internal (result_ptr, format, ap, mode);
>  }
> -libc_hidden_def (__vasprintf_chk)

Ok.

> diff --git a/debug/vdprintf_chk.c b/debug/vdprintf_chk.c
> index 4386127cfe..e04514e355 100644
> --- a/debug/vdprintf_chk.c
> +++ b/debug/vdprintf_chk.c
> @@ -24,41 +24,14 @@
>     This exception applies to code released by its copyright holders
>     in files containing the exception.  */
>  
> -#include <libioP.h>
> -#include <stdio_ext.h>
> +#include <libio/libioP.h>
>  
>  int
> -__vdprintf_chk (int d, int flags, const char *format, va_list arg)
> +__vdprintf_chk (int d, int flag, const char *format, va_list ap)
>  {
> -  struct _IO_FILE_plus tmpfil;
> -  struct _IO_wide_data wd;
> -  int done;
> -
> -#ifdef _IO_MTSAFE_IO
> -  tmpfil.file._lock = NULL;
> -#endif
> -  _IO_no_init (&tmpfil.file, _IO_USER_LOCK, 0, &wd, &_IO_wfile_jumps);
> -  _IO_JUMPS (&tmpfil) = &_IO_file_jumps;
> -  _IO_new_file_init_internal (&tmpfil);
> -  if (_IO_file_attach (&tmpfil.file, d) == NULL)
> -    {
> -      _IO_un_link (&tmpfil);
> -      return EOF;
> -    }
> -  tmpfil.file._flags |= _IO_DELETE_DONT_CLOSE;
> -
> -  _IO_mask_flags (&tmpfil.file, _IO_NO_READS,
> -  _IO_NO_READS+_IO_NO_WRITES+_IO_IS_APPENDING);
> -
> -  /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
>       can only come from read-only format strings.  */
> -  if (flags > 0)
> -    tmpfil.file._flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  done = __vfprintf_internal (&tmpfil.file, format, arg, 0);
> -
> -  _IO_FINISH (&tmpfil.file);
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  return done;
> +  return __vdprintf_internal (d, format, ap, mode);
>  }
> -libc_hidden_def (__vdprintf_chk)

Ok.

> diff --git a/debug/vfprintf_chk.c b/debug/vfprintf_chk.c
> index 5babbf611e..44426e14fd 100644
> --- a/debug/vfprintf_chk.c
> +++ b/debug/vfprintf_chk.c
> @@ -15,28 +15,17 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdarg.h>
> -#include <stdio.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to FP from the format string FORMAT.  */
>  int
>  ___vfprintf_chk (FILE *fp, int flag, const char *format, va_list ap)
>  {
> -  int done;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  _IO_acquire_lock_clear_flags2 (fp);
> -  if (flag > 0)
> -    fp->_flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  done = vfprintf (fp, format, ap);
> -
> -  if (flag > 0)
> -    fp->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (fp);
> -
> -  return done;
> +  return __vfprintf_internal (fp, format, ap, mode);
>  }
> -ldbl_hidden_def (___vfprintf_chk, __vfprintf_chk)
>  ldbl_strong_alias (___vfprintf_chk, __vfprintf_chk)

Ok.

> diff --git a/debug/vfwprintf_chk.c b/debug/vfwprintf_chk.c
> index abf2bd6517..3aed308156 100644
> --- a/debug/vfwprintf_chk.c
> +++ b/debug/vfwprintf_chk.c
> @@ -15,27 +15,16 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdarg.h>
> -#include <wchar.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to FP from the format string FORMAT.  */
>  int
>  __vfwprintf_chk (FILE *fp, int flag, const wchar_t *format, va_list ap)
>  {
> -  int done;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  _IO_acquire_lock_clear_flags2 (fp);
> -  if (flag > 0)
> -    fp->_flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  done = __vfwprintf_internal (fp, format, ap, 0);
> -
> -  if (flag > 0)
> -    fp->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (fp);
> -
> -  return done;
> +  return __vfwprintf_internal (fp, format, ap, mode);
>  }
> -libc_hidden_def (__vfwprintf_chk)

Ok.

> diff --git a/debug/vobprintf_chk.c b/debug/vobprintf_chk.c
> new file mode 100644
> index 0000000000..edfbe8f00a
> --- /dev/null
> +++ b/debug/vobprintf_chk.c
> @@ -0,0 +1,32 @@
> +/* Print output of stream to given obstack.
> +   Copyright (C) 1996-2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Contributed by Ulrich Drepper <[hidden email]>, 1996.

I think it is better to just set Copyright to 2018 without 'Contributed by'
(this implementation is quite different than the original one).

> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <libio/libioP.h>
> +
> +
> +int
> +__obstack_vprintf_chk (struct obstack *obstack, int flag, const char *format,
> +       va_list ap)
> +{
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
> +
> +  return __obstack_vprintf_internal (obstack, format, ap, mode);
> +}

Ok.

> diff --git a/debug/vprintf_chk.c b/debug/vprintf_chk.c
> index b3b2c53df2..69fcb721ac 100644
> --- a/debug/vprintf_chk.c
> +++ b/debug/vprintf_chk.c
> @@ -15,27 +15,17 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdarg.h>
> -#include <stdio.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to stdout from the format string FORMAT.  */
>  int
>  ___vprintf_chk (int flag, const char *format, va_list ap)
>  {
> -  int done;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  _IO_acquire_lock_clear_flags2 (stdout);
> -  if (flag > 0)
> -    stdout->_flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  done = vfprintf (stdout, format, ap);
> -
> -  if (flag > 0)
> -    stdout->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (stdout);
> -
> -  return done;
> +  return __vfprintf_internal (stdout, format, ap, mode);
>  }
>  ldbl_strong_alias (___vprintf_chk, __vprintf_chk)

Ok.

> diff --git a/debug/vsnprintf_chk.c b/debug/vsnprintf_chk.c
> index 95d286f416..666a83b701 100644
> --- a/debug/vsnprintf_chk.c
> +++ b/debug/vsnprintf_chk.c
> @@ -15,56 +15,22 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdarg.h>
> -#include <stdio.h>
> -#include "../libio/libioP.h"
> -#include "../libio/strfile.h"
> +#include <libio/libioP.h>
>  
> -extern const struct _IO_jump_t _IO_strn_jumps libio_vtable attribute_hidden;
>  
>  /* Write formatted output into S, according to the format
>     string FORMAT, writing no more than MAXLEN characters.  */
> -/* VARARGS5 */
>  int
> -___vsnprintf_chk (char *s, size_t maxlen, int flags, size_t slen,
> -  const char *format, va_list args)
> +___vsnprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
> +  const char *format, va_list ap)
>  {
> -  /* XXX Maybe for less strict version do not fail immediately.
> -     Though, maxlen is supposed to be the size of buffer pointed
> -     to by s, so a conforming program can't pass such maxlen
> -     to *snprintf.  */
>    if (__glibc_unlikely (slen < maxlen))
>      __chk_fail ();
>  
> -  _IO_strnfile sf;
> -  int ret;
> -#ifdef _IO_MTSAFE_IO
> -  sf.f._sbf._f._lock = NULL;
> -#endif
> -
> -  /* We need to handle the special case where MAXLEN is 0.  Use the
> -     overflow buffer right from the start.  */
> -  if (maxlen == 0)
> -    {
> -      s = sf.overflow_buf;
> -      maxlen = sizeof (sf.overflow_buf);
> -    }
> -
> -  _IO_no_init (&sf.f._sbf._f, _IO_USER_LOCK, -1, NULL, NULL);
> -  _IO_JUMPS (&sf.f._sbf) = &_IO_strn_jumps;
> -  s[0] = '\0';
> -
> -  /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
>       can only come from read-only format strings.  */
> -  if (flags > 0)
> -    sf.f._sbf._f._flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  _IO_str_init_static_internal (&sf.f, s, maxlen - 1, s);
> -  ret = __vfprintf_internal (&sf.f._sbf._f, format, args, 0);
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  if (sf.f._sbf._f._IO_buf_base != sf.overflow_buf)
> -    *sf.f._sbf._f._IO_write_ptr = '\0';
> -  return ret;
> +  return __vsnprintf_internal (s, maxlen, format, ap, mode);
>  }
> -ldbl_hidden_def (___vsnprintf_chk, __vsnprintf_chk)
>  ldbl_strong_alias (___vsnprintf_chk, __vsnprintf_chk)

Ok.

> diff --git a/debug/vsprintf_chk.c b/debug/vsprintf_chk.c
> index 53f07236ae..c1b1a8da4f 100644
> --- a/debug/vsprintf_chk.c
> +++ b/debug/vsprintf_chk.c
> @@ -15,75 +15,20 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdarg.h>
> -#include <stdio.h>
> -#include "../libio/libioP.h"
> -#include "../libio/strfile.h"
> -
> -
> -static int _IO_str_chk_overflow (FILE *fp, int c) __THROW;
> -
> -static int
> -_IO_str_chk_overflow (FILE *fp, int c)
> -{
> -  /* When we come to here this means the user supplied buffer is
> -     filled.  */
> -  __chk_fail ();
> -}
> -
> -
> -static const struct _IO_jump_t _IO_str_chk_jumps libio_vtable =
> -{
> -  JUMP_INIT_DUMMY,
> -  JUMP_INIT(finish, _IO_str_finish),
> -  JUMP_INIT(overflow, _IO_str_chk_overflow),
> -  JUMP_INIT(underflow, _IO_str_underflow),
> -  JUMP_INIT(uflow, _IO_default_uflow),
> -  JUMP_INIT(pbackfail, _IO_str_pbackfail),
> -  JUMP_INIT(xsputn, _IO_default_xsputn),
> -  JUMP_INIT(xsgetn, _IO_default_xsgetn),
> -  JUMP_INIT(seekoff, _IO_str_seekoff),
> -  JUMP_INIT(seekpos, _IO_default_seekpos),
> -  JUMP_INIT(setbuf, _IO_default_setbuf),
> -  JUMP_INIT(sync, _IO_default_sync),
> -  JUMP_INIT(doallocate, _IO_default_doallocate),
> -  JUMP_INIT(read, _IO_default_read),
> -  JUMP_INIT(write, _IO_default_write),
> -  JUMP_INIT(seek, _IO_default_seek),
> -  JUMP_INIT(close, _IO_default_close),
> -  JUMP_INIT(stat, _IO_default_stat),
> -  JUMP_INIT(showmanyc, _IO_default_showmanyc),
> -  JUMP_INIT(imbue, _IO_default_imbue)
> -};
> -
> +#include <libio/libioP.h>
>  
>  int
> -___vsprintf_chk (char *s, int flags, size_t slen, const char *format,
> - va_list args)
> +___vsprintf_chk (char *s, int flag, size_t slen, const char *format,
> + va_list ap)
>  {
> -  _IO_strfile f;
> -  int ret;
> -#ifdef _IO_MTSAFE_IO
> -  f._sbf._f._lock = NULL;
> -#endif
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
>    if (slen == 0)
>      __chk_fail ();
>  
> -  _IO_no_init (&f._sbf._f, _IO_USER_LOCK, -1, NULL, NULL);
> -  _IO_JUMPS (&f._sbf) = &_IO_str_chk_jumps;
> -  s[0] = '\0';
> -  _IO_str_init_static_internal (&f, s, slen - 1, s);
> -
> -  /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> -     can only come from read-only format strings.  */
> -  if (flags > 0)
> -    f._sbf._f._flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  ret = __vfprintf_internal (&f._sbf._f, format, args, 0);
> -
> -  *f._sbf._f._IO_write_ptr = '\0';
> -  return ret;
> +  return __vsprintf_internal (s, slen, format, ap, mode);
>  }
>  ldbl_hidden_def (___vsprintf_chk, __vsprintf_chk)
>  ldbl_strong_alias (___vsprintf_chk, __vsprintf_chk)

Ok.

> diff --git a/debug/vswprintf_chk.c b/debug/vswprintf_chk.c
> index 4d616f8835..2c6fadd463 100644
> --- a/debug/vswprintf_chk.c
> +++ b/debug/vswprintf_chk.c
> @@ -15,60 +15,21 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdarg.h>
> -#include <wchar.h>
> -#include "../libio/libioP.h"
> -#include "../libio/strfile.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output into S, according to the format
>     string FORMAT, writing no more than MAXLEN characters.  */
> -/* VARARGS5 */
>  int
> -__vswprintf_chk (wchar_t *s, size_t maxlen, int flags, size_t slen,
> - const wchar_t *format, va_list args)
> +__vswprintf_chk (wchar_t *s, size_t maxlen, int flag, size_t slen,
> + const wchar_t *format, va_list ap)
>  {
> -  /* XXX Maybe for less strict version do not fail immediately.
> -     Though, maxlen is supposed to be the size of buffer pointed
> -     to by s, so a conforming program can't pass such maxlen
> -     to *snprintf.  */
>    if (__glibc_unlikely (slen < maxlen))
>      __chk_fail ();
>  
> -  _IO_wstrnfile sf;
> -  struct _IO_wide_data wd;
> -  int ret;
> -#ifdef _IO_MTSAFE_IO
> -  sf.f._sbf._f._lock = NULL;
> -#endif
> -
> -  /* We need to handle the special case where MAXLEN is 0.  Use the
> -     overflow buffer right from the start.  */
> -  if (__glibc_unlikely (maxlen == 0))
> -    /* Since we have to write at least the terminating L'\0' a buffer
> -       length of zero always makes the function fail.  */
> -    return -1;
> -
> -  _IO_no_init (&sf.f._sbf._f, _IO_USER_LOCK, 0, &wd, &_IO_wstrn_jumps);
> -  _IO_fwide (&sf.f._sbf._f, 1);
> -  s[0] = L'\0';
> -
> -  /* For flags > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
>       can only come from read-only format strings.  */
> -  if (flags > 0)
> -    sf.f._sbf._f._flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  _IO_wstr_init_static (&sf.f._sbf._f, s, maxlen - 1, s);
> -  ret = __vfwprintf_internal ((FILE *) &sf.f._sbf, format, args, 0);
> -
> -  if (sf.f._sbf._f._wide_data->_IO_buf_base == sf.overflow_buf)
> -    /* ISO C99 requires swprintf/vswprintf to return an error if the
> -       output does not fit int he provided buffer.  */
> -    return -1;
> -
> -  /* Terminate the string.  */
> -  *sf.f._sbf._f._wide_data->_IO_write_ptr = '\0';
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  return ret;
> +  return __vswprintf_internal (s, maxlen, format, ap, mode);
>  }
> -libc_hidden_def (__vswprintf_chk)

Ok.

> diff --git a/debug/vwprintf_chk.c b/debug/vwprintf_chk.c
> index fedc7a46bf..f1e8878a54 100644
> --- a/debug/vwprintf_chk.c
> +++ b/debug/vwprintf_chk.c
> @@ -15,27 +15,16 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <stdarg.h>
> -#include <stdio.h>
> -#include <wchar.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to stdout from the format string FORMAT.  */
>  int
>  __vwprintf_chk (int flag, const wchar_t *format, va_list ap)
>  {
> -  int done;
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>  
> -  _IO_acquire_lock_clear_flags2 (stdout);
> -  if (flag > 0)
> -    stdout->_flags2 |= _IO_FLAGS2_FORTIFY;
> -
> -  done = __vfwprintf_internal (stdout, format, ap, 0);
> -
> -  if (flag > 0)
> -    stdout->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (stdout);
> -
> -  return done;
> +  return __vfwprintf_internal (stdout, format, ap, mode);
>  }

Ok.

> diff --git a/debug/wprintf_chk.c b/debug/wprintf_chk.c
> index 819050e5af..9f406e95f8 100644
> --- a/debug/wprintf_chk.c
> +++ b/debug/wprintf_chk.c
> @@ -16,29 +16,22 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <stdarg.h>
> -#include <stdio.h>
> -#include <wchar.h>
> -#include "../libio/libioP.h"
> +#include <libio/libioP.h>
>  
>  
>  /* Write formatted output to stdout from the format string FORMAT.  */
>  int
>  __wprintf_chk (int flag, const wchar_t *format, ...)
>  {
> +  /* For flag > 0 (i.e. __USE_FORTIFY_LEVEL > 1) request that %n
> +     can only come from read-only format strings.  */
> +  unsigned int mode = (flag > 0) ? PRINTF_FORTIFY : 0;
>    va_list ap;
> -  int done;
> -
> -  _IO_acquire_lock_clear_flags2 (stdout);
> -  if (flag > 0)
> -    stdout->_flags2 |= _IO_FLAGS2_FORTIFY;
> +  int ret;
>  
>    va_start (ap, format);
> -  done = __vfwprintf_internal (stdout, format, ap, 0);
> +  ret = __vfwprintf_internal (stdout, format, ap, mode);
>    va_end (ap);
>  
> -  if (flag > 0)
> -    stdout->_flags2 &= ~_IO_FLAGS2_FORTIFY;
> -  _IO_release_lock (stdout);
> -
> -  return done;
> +  return ret;
>  }

Ok.

> diff --git a/include/stdio.h b/include/stdio.h
> index 0856d729d9..1b7da0f74d 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -216,11 +216,6 @@ libc_hidden_proto (__open_memstream)
>  libc_hidden_proto (__libc_fatal)
>  rtld_hidden_proto (__libc_fatal)
>  libc_hidden_proto (__vsprintf_chk)
> -libc_hidden_proto (__vsnprintf_chk)
> -libc_hidden_proto (__vfprintf_chk)
> -libc_hidden_proto (__vasprintf_chk)
> -libc_hidden_proto (__vdprintf_chk)
> -libc_hidden_proto (__obstack_vprintf_chk)
>  
>  extern FILE * __fmemopen (void *buf, size_t len, const char *mode);
>  libc_hidden_proto (__fmemopen)

Ok.

> diff --git a/include/wchar.h b/include/wchar.h
> index d0fe45c3a6..86506d28e9 100644
> --- a/include/wchar.h
> +++ b/include/wchar.h
> @@ -216,8 +216,6 @@ extern int __vswprintf_chk (wchar_t *__restrict __s, size_t __n,
>      const wchar_t *__restrict __format,
>      __gnuc_va_list __arg)
>       /* __attribute__ ((__format__ (__wprintf__, 5, 0))) */;
> -libc_hidden_proto (__vfwprintf_chk)
> -libc_hidden_proto (__vswprintf_chk)
>  
>  extern int __isoc99_fwscanf (__FILE *__restrict __stream,
>       const wchar_t *__restrict __format, ...);

Ok.

> diff --git a/libio/iovsprintf.c b/libio/iovsprintf.c
> index 3b1e8292b5..08e4002625 100644
> --- a/libio/iovsprintf.c
> +++ b/libio/iovsprintf.c
> @@ -27,8 +27,47 @@
>  #include "libioP.h"
>  #include "strfile.h"
>  
> +static int __THROW
> +_IO_str_chk_overflow (FILE *fp, int c)
> +{
> +  /* If we get here, the user-supplied buffer would be overrun by
> +     further output.  */
> +  __chk_fail ();
> +}
> +
> +static const struct _IO_jump_t _IO_str_chk_jumps libio_vtable =
> +{
> +  JUMP_INIT_DUMMY,
> +  JUMP_INIT(finish, _IO_str_finish),
> +  JUMP_INIT(overflow, _IO_str_chk_overflow),
> +  JUMP_INIT(underflow, _IO_str_underflow),
> +  JUMP_INIT(uflow, _IO_default_uflow),
> +  JUMP_INIT(pbackfail, _IO_str_pbackfail),
> +  JUMP_INIT(xsputn, _IO_default_xsputn),
> +  JUMP_INIT(xsgetn, _IO_default_xsgetn),
> +  JUMP_INIT(seekoff, _IO_str_seekoff),
> +  JUMP_INIT(seekpos, _IO_default_seekpos),
> +  JUMP_INIT(setbuf, _IO_default_setbuf),
> +  JUMP_INIT(sync, _IO_default_sync),
> +  JUMP_INIT(doallocate, _IO_default_doallocate),
> +  JUMP_INIT(read, _IO_default_read),
> +  JUMP_INIT(write, _IO_default_write),
> +  JUMP_INIT(seek, _IO_default_seek),
> +  JUMP_INIT(close, _IO_default_close),
> +  JUMP_INIT(stat, _IO_default_stat),
> +  JUMP_INIT(showmanyc, _IO_default_showmanyc),
> +  JUMP_INIT(imbue, _IO_default_imbue)
> +};
> +
> +/* This function is called by regular vsprintf with maxlen set to -1,
> +   and by vsprintf_chk with maxlen set to the size of the output
> +   string.  In the former case, _IO_str_chk_overflow will never be
> +   called; in the latter case it will crash the program if the buffer
> +   overflows.  */
> +
>  int
> -__vsprintf_internal (char *string, const char *format, va_list args,
> +__vsprintf_internal (char *string, size_t maxlen,
> +     const char *format, va_list args,
>       unsigned int mode_flags)
>  {
>    _IO_strfile sf;
> @@ -38,17 +77,22 @@ __vsprintf_internal (char *string, const char *format, va_list args,
>    sf._sbf._f._lock = NULL;
>  #endif
>    _IO_no_init (&sf._sbf._f, _IO_USER_LOCK, -1, NULL, NULL);
> -  _IO_JUMPS (&sf._sbf) = &_IO_str_jumps;
> -  _IO_str_init_static_internal (&sf, string, -1, string);
> +  _IO_JUMPS (&sf._sbf) = &_IO_str_chk_jumps;
> +  string[0] = '\0';
> +  _IO_str_init_static_internal (&sf, string,
> + (maxlen == -1) ? -1 : maxlen - 1,
> + string);
> +
>    ret = __vfprintf_internal (&sf._sbf._f, format, args, mode_flags);
> -  _IO_putc_unlocked ('\0', &sf._sbf._f);
> +
> +  *sf._sbf._f._IO_write_ptr = '\0';
>    return ret;
>  }

Ok.

>  
>  int
>  __vsprintf (char *string, const char *format, va_list args)
>  {
> -  return __vsprintf_internal (string, format, args, 0);
> +  return __vsprintf_internal (string, -1, format, args, 0);
>  }
>  
>  ldbl_strong_alias (__vsprintf, _IO_vsprintf)
> diff --git a/libio/libio.h b/libio/libio.h
> index c188814ccc..3a93807efc 100644
> --- a/libio/libio.h
> +++ b/libio/libio.h
> @@ -90,7 +90,6 @@ typedef union
>  /* Bits for the _flags2 field.  */
>  #define _IO_FLAGS2_MMAP 1
>  #define _IO_FLAGS2_NOTCANCEL 2
> -#define _IO_FLAGS2_FORTIFY 4
>  #define _IO_FLAGS2_USER_WBUF 8
>  #define _IO_FLAGS2_NOCLOSE 32
>  #define _IO_FLAGS2_CLOEXEC 64

Ok.

> diff --git a/libio/libioP.h b/libio/libioP.h
> index c762cf9b67..17270b126f 100644
> --- a/libio/libioP.h
> +++ b/libio/libioP.h
> @@ -677,9 +677,16 @@ extern int __obstack_vprintf_internal (struct obstack *ob, const char *fmt,
>         va_list ap, unsigned int mode_flags)
>      attribute_hidden;
>  
> -extern int __vsprintf_internal (char *string, const char *format, va_list ap,
> +/* Note: __vsprintf_internal, unlike vsprintf, does take a maxlen argument,
> +   because it's called by both vsprintf and vsprintf_chk.  If maxlen is
> +   not set to -1, overrunning the buffer will cause a prompt crash.
> +   This is the behavior of ordinary (v)sprintf functions, thus they call
> +   __vsprintf_internal with that argument set to -1.  */
> +extern int __vsprintf_internal (char *string, size_t maxlen,
> + const char *format, va_list ap,
>   unsigned int mode_flags)
>      attribute_hidden;
> +
>  extern int __vsnprintf_internal (char *string, size_t maxlen,
>   const char *format, va_list ap,
>   unsigned int mode_flags)
> @@ -798,26 +805,10 @@ _IO_acquire_lock_fct (FILE **p)
>      _IO_funlockfile (fp);
>  }
>  
> -static inline void
> -__attribute__ ((__always_inline__))
> -_IO_acquire_lock_clear_flags2_fct (FILE **p)
> -{
> -  FILE *fp = *p;
> -  fp->_flags2 &= ~(_IO_FLAGS2_FORTIFY);
> -  if ((fp->_flags & _IO_USER_LOCK) == 0)
> -    _IO_funlockfile (fp);
> -}
> -
>  #if !defined _IO_MTSAFE_IO && IS_IN (libc)
>  # define _IO_acquire_lock(_fp)      \
> -  do {      \
> -    FILE *_IO_acquire_lock_file = NULL
> -# define _IO_acquire_lock_clear_flags2(_fp)      \
> -  do {      \
> -    FILE *_IO_acquire_lock_file = (_fp)
> +  do {
>  # define _IO_release_lock(_fp)      \
> -    if (_IO_acquire_lock_file != NULL)      \
> -      _IO_acquire_lock_file->_flags2 &= ~(_IO_FLAGS2_FORTIFY);      \
>    } while (0)
>  #endif
>  

Ok.

> diff --git a/stdio-common/sprintf.c b/stdio-common/sprintf.c
> index 77423b292f..447faa4e25 100644
> --- a/stdio-common/sprintf.c
> +++ b/stdio-common/sprintf.c
> @@ -27,7 +27,7 @@ __sprintf (char *s, const char *format, ...)
>    int done;
>  
>    va_start (arg, format);
> -  done = __vsprintf_internal (s, format, arg, 0);
> +  done = __vsprintf_internal (s, -1, format, arg, 0);
>    va_end (arg);
>  
>    return done;

Ok.

> diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> index 7752ad5b68..b5ae868371 100644
> --- a/stdio-common/vfprintf-internal.c
> +++ b/stdio-common/vfprintf-internal.c
> @@ -1285,8 +1285,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
>    /* Temporarily honor environmental settings.  */
>    if (__ldbl_is_dbl)
>      mode_flags |= PRINTF_LDBL_IS_DBL;
> -  if (s->_flags2 & _IO_FLAGS2_FORTIFY)
> -    mode_flags |= PRINTF_FORTIFY;
>  
>    /* Orient the stream.  */
>  #ifdef ORIENT

Ok.

> diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h
> index 4a40618545..25ccd07f29 100644
> --- a/sysdeps/generic/stdio-lock.h
> +++ b/sysdeps/generic/stdio-lock.h
> @@ -54,15 +54,8 @@ __libc_lock_define_recursive (typedef, _IO_lock_t)
>   __attribute__((cleanup (_IO_acquire_lock_fct)))      \
>   = (_fp);      \
>      _IO_flockfile (_IO_acquire_lock_file);
> -#  define _IO_acquire_lock_clear_flags2(_fp) \
> -  do {      \
> -    FILE *_IO_acquire_lock_file      \
> - __attribute__((cleanup (_IO_acquire_lock_clear_flags2_fct)))      \
> - = (_fp);      \
> -    _IO_flockfile (_IO_acquire_lock_file);
>  # else
>  #  define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled
> -#  define _IO_acquire_lock_clear_flags2(_fp) _IO_acquire_lock (_fp)
>  # endif
>  # define _IO_release_lock(_fp) ; } while (0)
>  

Ok.

> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> index 958bbc1834..59b2c9fcdd 100644
> --- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> @@ -179,7 +179,7 @@ __nldbl___vsprintf (char *string, const char *fmt, va_list ap)
>  {
>    int done;
>    __no_long_double = 1;
> -  done = __vsprintf_internal (string, fmt, ap, 0);
> +  done = __vsprintf_internal (string, -1, fmt, ap, 0);
>    __no_long_double = 0;
>    return done;
>  }
> @@ -579,7 +579,7 @@ __nldbl___vfprintf_chk (FILE *s, int flag, const char *fmt, va_list ap)
>  {
>    int res;
>    set_no_long_double ();
> -  res = __vfprintf_chk (s, flag, fmt, ap);
> +  res = __vfprintf_internal (s, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
>    clear_no_long_double ();
>    return res;
>  }
> @@ -591,7 +591,7 @@ __nldbl___vfwprintf_chk (FILE *s, int flag, const wchar_t *fmt, va_list ap)
>  {
>    int res;
>    set_no_long_double ();
> -  res = __vfwprintf_chk (s, flag, fmt, ap);
> +  res = __vfwprintf_internal (s, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
>    clear_no_long_double ();
>    return res;
>  }
> @@ -609,9 +609,13 @@ attribute_compat_text_section
>  __nldbl___vsnprintf_chk (char *string, size_t maxlen, int flag, size_t slen,
>   const char *fmt, va_list ap)
>  {
> +  if (__glibc_unlikely (slen < maxlen))
> +    __chk_fail ();
> +
>    int res;
>    __no_long_double = 1;
> -  res = __vsnprintf_chk (string, maxlen, flag, slen, fmt, ap);
> +  res = __vsnprintf_internal (string, maxlen, fmt, ap,
> +      (flag > 0) ? PRINTF_FORTIFY : 0);
>    __no_long_double = 0;
>    return res;
>  }
> @@ -622,9 +626,13 @@ attribute_compat_text_section
>  __nldbl___vsprintf_chk (char *string, int flag, size_t slen, const char *fmt,
>   va_list ap)
>  {
> +  if (slen == 0)
> +    __chk_fail ();
> +
>    int res;
>    __no_long_double = 1;
> -  res = __vsprintf_chk (string, flag, slen, fmt, ap);
> +  res = __vsprintf_internal (string, slen, fmt, ap,
> +     (flag > 0) ? PRINTF_FORTIFY : 0);
>    __no_long_double = 0;
>    return res;
>  }
> @@ -635,9 +643,13 @@ attribute_compat_text_section
>  __nldbl___vswprintf_chk (wchar_t *string, size_t maxlen, int flag, size_t slen,
>   const wchar_t *fmt, va_list ap)
>  {
> +  if (__glibc_unlikely (slen < maxlen))
> +    __chk_fail ();
> +
>    int res;
>    __no_long_double = 1;
> -  res = __vswprintf_chk (string, maxlen, flag, slen, fmt, ap);
> +  res = __vswprintf_internal (string, maxlen, fmt, ap,
> +      (flag > 0) ? PRINTF_FORTIFY : 0);
>    __no_long_double = 0;
>    return res;
>  }
> @@ -670,7 +682,8 @@ __nldbl___vasprintf_chk (char **ptr, int flag, const char *fmt, va_list arg)
>  {
>    int res;
>    __no_long_double = 1;
> -  res = __vasprintf_chk (ptr, flag, fmt, arg);
> +  res = __vasprintf_internal (ptr, fmt, arg,
> +      (flag > 0) ? PRINTF_FORTIFY : 0);
>    __no_long_double = 0;
>    return res;
>  }
> @@ -696,7 +709,7 @@ __nldbl___vdprintf_chk (int d, int flag, const char *fmt, va_list arg)
>  {
>    int res;
>    set_no_long_double ();
> -  res = __vdprintf_chk (d, flag, fmt, arg);
> +  res = __vdprintf_internal (d, fmt, arg, (flag > 0) ? PRINTF_FORTIFY : 0);
>    clear_no_long_double ();
>    return res;
>  }
> @@ -723,7 +736,8 @@ __nldbl___obstack_vprintf_chk (struct obstack *obstack, int flag,
>  {
>    int res;
>    __no_long_double = 1;
> -  res = __obstack_vprintf_chk (obstack, flag, fmt, arg);
> +  res = __obstack_vprintf_internal (obstack, fmt, arg,
> +    (flag > 0) ? PRINTF_FORTIFY : 0);
>    __no_long_double = 0;
>    return res;
>  }

Ok.

> diff --git a/sysdeps/nptl/stdio-lock.h b/sysdeps/nptl/stdio-lock.h
> index 5b9782452f..1d6a81c5bf 100644
> --- a/sysdeps/nptl/stdio-lock.h
> +++ b/sysdeps/nptl/stdio-lock.h
> @@ -94,15 +94,8 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t;
>   __attribute__((cleanup (_IO_acquire_lock_fct)))      \
>   = (_fp);      \
>      _IO_flockfile (_IO_acquire_lock_file);
> -#  define _IO_acquire_lock_clear_flags2(_fp) \
> -  do {      \
> -    FILE *_IO_acquire_lock_file      \
> - __attribute__((cleanup (_IO_acquire_lock_clear_flags2_fct)))      \
> - = (_fp);      \
> -    _IO_flockfile (_IO_acquire_lock_file);
>  # else
>  #  define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled
> -#  define _IO_acquire_lock_clear_flags2(_fp) _IO_acquire_lock (_fp)
>  # endif
>  # define _IO_release_lock(_fp) ; } while (0)
>  
>

Ok.


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 8/8] Use PRINTF_LDBL_IS_DBL instead of __ldbl_is_dbl.

Adhemerval Zanella-2
In reply to this post by Gabriel F. T. Gomes-2


On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:

> From: Zack Weinberg <[hidden email]>
>
> Changed since v1:
>
>   - Removed libc_hidden_def and libc_hidden_proto from all of the
>     __nldbl_*printf* function, since they are no longer called from
>     within glibc, only from nldbl-*printf*.c files (libnldbl_nonshared.a)
>
> -- 8< --
> After all that prep work, nldbl-compat.c can now use PRINTF_LDBL_IS_DBL
> instead of __no_long_double to control the behavior of printf-like
> functions; this is the last thing we needed __no_long_double for, so it
> can go away entirely.
>
> Tested for powerpc and powerpc64le.

LGTM with two points below.

>
> 2018-10-25  Zack Weinberg  <[hidden email]>
>    Gabriel F. T. Gomes  <[hidden email]>
>
> * stdio-common/vfprintf-internal.c
> (__vfprintf_internal, __vfwprintf_internal): Don't use __ldbl_is_dbl.
> * sysdeps/generic/math_ldbl_opt.h: Remove __ldbl_is_dbl.
> * sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h: Remove __ldbl_is_dbl
> and __no_long_double.
> * sysdeps/ieee754/ldbl-opt/math_ldbl_opt.c: Remove file.
> * sysdeps/ieee754/ldbl-opt/Makefile (routines): Remove math_ldbl_opt.
> * sysdeps/ieee754/ldbl-opt/nldbl-compat.c: Remove
> libc_hidden_proto and libc_hidden_def from all __nldbl_*printf*
> and __nldbl_*syslog* functions.
> (__nldbl_cleanup, set_no_long_double, clear_no_long_double): Remove.
> (__nldbl___asprintf, __nldbl_dprintf, __nldbl_fprintf)
> (__nldbl_fwprintf, __nldbl_printf, __nldbl_sprintf)
> (__nldbl_vfprintf, __nldbl___vsprintf, __nldbl_obstack_vprintf)
> (__ndlbl_obstack_printf, __nldbl_snprintf, __nldbl_swprintf)
> (__nldbl_vasprintf, __nldbl_vdprintf, __nldbl_vfwprintf)
> (__nldbl_vprintf, __nldbl_vsnprintf, __ndlbl_vswprintf)
> (__nldbl_vwprintf, __nldbl_wprintf):
> Directly call the appropriate __v*printf_internal routine, passing
> PRINTF_LDBL_IS_DBL.  Do not mess with __no_long_double. Normalize
> variable names.
> (__nldbl___fprintf_chk, __nldbl___fwprintf_chk)
> (__nldbl___printf_chk, __nldbl___snprintf_chk)
> (__nldbl___sprintf_chk, __nldbl___swprintf_chk)
> (__nldbl___vfprintf_chk, __nldbl___vfwprintf_chk)
> (__nldbl___vprintf_chk, __nldbl___vsnprintf_chk)
> (__nldbl___vsprintf_chk, __nldbl___vswprintf_chk)
> (__nldbl___vwprintf_chk, __nldbl___wprintf_chk)
> (__nldbl___vasprintf_chk, __nldbl___asprintf_chk)
> (__nldbl___vdprintf_chk, __nldbl___dprintf_chk)
> (__nldbl___obstack_vprintf_chk, __nldbl___obstack_printf_chk):
> Likewise, and also pass PRINTF_FORTIFY when appropriate.
> (__nldbl_syslog, __nldbl_vsyslog):
> Directly call __vsyslog_internal, passing PRINTF_LDBL_IS_DBL.
> (__nldbl_syslog_chk): Likewise, and also pass PRINTF_FORTIFY when
> appropriate.
> (__nldbl_vsyslog_chk): Likewise, and also pass PRINTF_FORTIFY when
> appropriate.
>
> Signed-off-by: Zack Weinberg <[hidden email]>
> Signed-off-by: Gabriel F. T. Gomes <[hidden email]>

We don't use DCO, but copyright assignments.

> ---
>  stdio-common/vfprintf-internal.c         |   4 -
>  sysdeps/generic/math_ldbl_opt.h          |   1 -
>  sysdeps/ieee754/ldbl-opt/Makefile        |   2 +-
>  sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h |   4 -
>  sysdeps/ieee754/ldbl-opt/nldbl-compat.c  | 504 +++++++++++++++----------------
>  5 files changed, 238 insertions(+), 277 deletions(-)
>
> diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> index b5ae868371..cf7e858e35 100644
> --- a/stdio-common/vfprintf-internal.c
> +++ b/stdio-common/vfprintf-internal.c
> @@ -1282,10 +1282,6 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap, unsigned int mode_flags)
>       0 if unknown.  */
>    int readonly_format = 0;
>  
> -  /* Temporarily honor environmental settings.  */
> -  if (__ldbl_is_dbl)
> -    mode_flags |= PRINTF_LDBL_IS_DBL;
> -
>    /* Orient the stream.  */
>  #ifdef ORIENT
>    ORIENT;

Ok.

> diff --git a/sysdeps/generic/math_ldbl_opt.h b/sysdeps/generic/math_ldbl_opt.h
> index 92f670dff7..fbd2c82e2f 100644
> --- a/sysdeps/generic/math_ldbl_opt.h
> +++ b/sysdeps/generic/math_ldbl_opt.h
> @@ -15,4 +15,3 @@
>  #define ldbl_weak_alias(name, aliasname) weak_alias (name, aliasname)
>  #define ldbl_compat_symbol(lib, local, symbol, version) \
>    compat_symbol (lib, local, symbol, version)
> -#define __ldbl_is_dbl 0
> diff --git a/sysdeps/ieee754/ldbl-opt/Makefile b/sysdeps/ieee754/ldbl-opt/Makefile
> index 6854413fa3..64fdb8cb9e 100644
> --- a/sysdeps/ieee754/ldbl-opt/Makefile
> +++ b/sysdeps/ieee754/ldbl-opt/Makefile
> @@ -8,7 +8,7 @@ endif
>  
>  ifeq ($(subdir),math)
>  libm-routines += s_nexttowardfd
> -routines += math_ldbl_opt nldbl-compat
> +routines += nldbl-compat
>  
>  extra-libs += libnldbl
>  libnldbl-calls = asprintf dprintf fprintf fscanf fwprintf fwscanf iovfscanf \

Ok.

> diff --git a/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h b/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
> index 4d2f3c7be2..dbd52d6d22 100644
> --- a/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
> +++ b/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
> @@ -41,8 +41,4 @@
>  #endif
>  
>  #ifndef __ASSEMBLER__
> -/* Set temporarily to non-zero if long double should be considered
> -   the same as double.  */
> -extern __thread int __no_long_double attribute_tls_model_ie attribute_hidden;
> -# define __ldbl_is_dbl __builtin_expect (__no_long_double, 0)
>  #endif

Why not remove the __ASSEMBLER #if/#endif altogether?

> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> index 59b2c9fcdd..f6dd81759d 100644
> --- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> @@ -30,43 +30,15 @@
>  
>  #include "nldbl-compat.h"
>  
> -libc_hidden_proto (__nldbl_vfprintf)
>  libc_hidden_proto (__nldbl_vsscanf)
> -libc_hidden_proto (__nldbl_vsprintf)
>  libc_hidden_proto (__nldbl_vfscanf)
>  libc_hidden_proto (__nldbl_vfwscanf)
> -libc_hidden_proto (__nldbl_vdprintf)
>  libc_hidden_proto (__nldbl_vswscanf)
> -libc_hidden_proto (__nldbl_vfwprintf)
> -libc_hidden_proto (__nldbl_vswprintf)
> -libc_hidden_proto (__nldbl_vsnprintf)
> -libc_hidden_proto (__nldbl_vasprintf)
> -libc_hidden_proto (__nldbl_obstack_vprintf)
> -libc_hidden_proto (__nldbl___vfwprintf_chk)
> -libc_hidden_proto (__nldbl___vsnprintf_chk)
> -libc_hidden_proto (__nldbl___vfprintf_chk)
> -libc_hidden_proto (__nldbl___vsyslog_chk)
> -libc_hidden_proto (__nldbl___vsprintf_chk)
> -libc_hidden_proto (__nldbl___vswprintf_chk)
> -libc_hidden_proto (__nldbl___vasprintf_chk)
> -libc_hidden_proto (__nldbl___vdprintf_chk)
> -libc_hidden_proto (__nldbl___obstack_vprintf_chk)
>  libc_hidden_proto (__nldbl___isoc99_vsscanf)
>  libc_hidden_proto (__nldbl___isoc99_vfscanf)
>  libc_hidden_proto (__nldbl___isoc99_vswscanf)
>  libc_hidden_proto (__nldbl___isoc99_vfwscanf)
>  
> -static void
> -__nldbl_cleanup (void *arg)
> -{
> -  __no_long_double = 0;
> -}
> -
> -#define set_no_long_double() \
> -  __libc_cleanup_push (__nldbl_cleanup, NULL); __no_long_double = 1
> -#define clear_no_long_double() \
> -  __no_long_double = 0; __libc_cleanup_pop (0)
> -
>  /* Compatibility with IEEE double as long double.
>     IEEE quad long double is used by default for most programs, so
>     we don't need to split this into one file per function for the

Ok.

> @@ -76,14 +48,14 @@ int
>  attribute_compat_text_section
>  __nldbl___asprintf (char **string_ptr, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vasprintf (string_ptr, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vasprintf_internal (string_ptr, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  weak_alias (__nldbl___asprintf, __nldbl_asprintf)
>  

Ok.

> @@ -91,28 +63,28 @@ int
>  attribute_compat_text_section
>  __nldbl_dprintf (int d, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vdprintf (d, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vdprintf_internal (d, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_fprintf (FILE *stream, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vfprintf (stream, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfprintf_internal (stream, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  weak_alias (__nldbl_fprintf, __nldbl__IO_fprintf)
>  

Ok.

> @@ -120,28 +92,28 @@ int
>  attribute_compat_text_section weak_function
>  __nldbl_fwprintf (FILE *stream, const wchar_t *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vfwprintf (stream, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfwprintf_internal (stream, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_printf (const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vfprintf (stdout, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfprintf_internal (stdout, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  strong_alias (__nldbl_printf, __nldbl__IO_printf)
>  

Ok.

> @@ -149,14 +121,14 @@ int
>  attribute_compat_text_section
>  __nldbl_sprintf (char *s, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vsprintf (s, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vsprintf_internal (s, -1, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  strong_alias (__nldbl_sprintf, __nldbl__IO_sprintf)
>  

Ok.

> @@ -164,123 +136,93 @@ int
>  attribute_compat_text_section
>  __nldbl_vfprintf (FILE *s, const char *fmt, va_list ap)
>  {
> -  int done;
> -  set_no_long_double ();
> -  done = __vfprintf_internal (s, fmt, ap, 0);
> -  clear_no_long_double ();
> -  return done;
> +  return __vfprintf_internal (s, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl_vfprintf)
>  strong_alias (__nldbl_vfprintf, __nldbl__IO_vfprintf)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___vsprintf (char *string, const char *fmt, va_list ap)
>  {
> -  int done;
> -  __no_long_double = 1;
> -  done = __vsprintf_internal (string, -1, fmt, ap, 0);
> -  __no_long_double = 0;
> -  return done;
> +  return __vsprintf_internal (string, -1, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
>  strong_alias (__nldbl___vsprintf, __nldbl__IO_vsprintf)
>  weak_alias (__nldbl___vsprintf, __nldbl_vsprintf)
> -libc_hidden_def (__nldbl_vsprintf)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_obstack_vprintf (struct obstack *obstack, const char *fmt,
>   va_list ap)
>  {
> -  int done;
> -  __no_long_double = 1;
> -  done = __obstack_vprintf_internal (obstack, fmt, ap, 0);
> -  __no_long_double = 0;
> -  return done;
> +  return __obstack_vprintf_internal (obstack, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl_obstack_vprintf)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_obstack_printf (struct obstack *obstack, const char *fmt, ...)
>  {
> -  int result;
> +  int ret;
>    va_list ap;
>    va_start (ap, fmt);
> -  result = __nldbl_obstack_vprintf (obstack, fmt, ap);
> +  ret = __obstack_vprintf_internal (obstack, fmt, ap, PRINTF_LDBL_IS_DBL);
>    va_end (ap);
> -  return result;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section weak_function
>  __nldbl_snprintf (char *s, size_t maxlen, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vsnprintf (s, maxlen, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vsnprintf_internal (s, maxlen, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_swprintf (wchar_t *s, size_t n, const wchar_t *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vswprintf (s, n, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vswprintf_internal (s, n, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section weak_function
>  __nldbl_vasprintf (char **result_ptr, const char *fmt, va_list ap)
>  {
> -  int res;
> -  __no_long_double = 1;
> -  res = __vasprintf_internal (result_ptr, fmt, ap, 0);
> -  __no_long_double = 0;
> -  return res;
> +  return __vasprintf_internal (result_ptr, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl_vasprintf)
>  

Ok.

>  int
>  attribute_compat_text_section
> -__nldbl_vdprintf (int d, const char *fmt, va_list arg)
> +__nldbl_vdprintf (int d, const char *fmt, va_list ap)
>  {
> -  int res;
> -  set_no_long_double ();
> -  res = __vdprintf_internal (d, fmt, arg, 0);
> -  clear_no_long_double ();
> -  return res;
> +  return __vdprintf_internal (d, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl_vdprintf)
>  

Ok.

>  int
>  attribute_compat_text_section weak_function
>  __nldbl_vfwprintf (FILE *s, const wchar_t *fmt, va_list ap)
>  {
> -  int res;
> -  set_no_long_double ();
> -  res = __vfwprintf_internal (s, fmt, ap, 0);
> -  clear_no_long_double ();
> -  return res;
> +  return __vfwprintf_internal (s, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl_vfwprintf)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_vprintf (const char *fmt, va_list ap)
>  {
> -  return __nldbl_vfprintf (stdout, fmt, ap);
> +  return __vfprintf_internal (stdout, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
>  

Ok.

>  int
> @@ -288,13 +230,8 @@ attribute_compat_text_section
>  __nldbl_vsnprintf (char *string, size_t maxlen, const char *fmt,
>     va_list ap)
>  {
> -  int res;
> -  __no_long_double = 1;
> -  res = __vsnprintf_internal (string, maxlen, fmt, ap, 0);
> -  __no_long_double = 0;
> -  return res;
> +  return __vsnprintf_internal (string, maxlen, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl_vsnprintf)
>  weak_alias (__nldbl_vsnprintf, __nldbl___vsnprintf)
>  

Ok.

>  int
> @@ -302,33 +239,28 @@ attribute_compat_text_section weak_function
>  __nldbl_vswprintf (wchar_t *string, size_t maxlen, const wchar_t *fmt,
>     va_list ap)
>  {
> -  int res;
> -  __no_long_double = 1;
> -  res = __vswprintf_internal (string, maxlen, fmt, ap, 0);
> -  __no_long_double = 0;
> -  return res;
> +  return __vswprintf_internal (string, maxlen, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl_vswprintf)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_vwprintf (const wchar_t *fmt, va_list ap)
>  {
> -  return __nldbl_vfwprintf (stdout, fmt, ap);
> +  return __vfwprintf_internal (stdout, fmt, ap, PRINTF_LDBL_IS_DBL);
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl_wprintf (const wchar_t *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl_vfwprintf (stdout, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfwprintf_internal (stdout, fmt, ap, PRINTF_LDBL_IS_DBL);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_29)
> @@ -491,42 +423,51 @@ int
>  attribute_compat_text_section
>  __nldbl___fprintf_chk (FILE *stream, int flag, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vfprintf_chk (stream, flag, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfprintf_internal (stream, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }

Ok.

>  
>  int
>  attribute_compat_text_section
>  __nldbl___fwprintf_chk (FILE *stream, int flag, const wchar_t *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vfwprintf_chk (stream, flag, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfwprintf_internal (stream, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___printf_chk (int flag, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vfprintf_chk (stdout, flag, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfprintf_internal (stdout, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
> @@ -534,74 +475,94 @@ attribute_compat_text_section
>  __nldbl___snprintf_chk (char *s, size_t maxlen, int flag, size_t slen,
>   const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  if (__glibc_unlikely (slen < maxlen))
> +    __chk_fail ();
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vsnprintf_chk (s, maxlen, flag, slen, fmt, arg);
> -  va_end (arg);
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  return done;
> +  va_start (ap, fmt);
> +  ret = __vsnprintf_internal (s, maxlen, fmt, ap, mode);
> +  va_end (ap);
> +
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___sprintf_chk (char *s, int flag, size_t slen, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  if (slen == 0)
> +    __chk_fail ();
> +
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vsprintf_chk (s, flag, slen, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vsprintf_internal (s, slen, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
> -__nldbl___swprintf_chk (wchar_t *s, size_t n, int flag, size_t slen,
> +__nldbl___swprintf_chk (wchar_t *s, size_t maxlen, int flag, size_t slen,
>   const wchar_t *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  if (__glibc_unlikely (slen < maxlen))
> +    __chk_fail ();
> +
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vswprintf_chk (s, n, flag, slen, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vswprintf_internal (s, maxlen, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___vfprintf_chk (FILE *s, int flag, const char *fmt, va_list ap)
>  {
> -  int res;
> -  set_no_long_double ();
> -  res = __vfprintf_internal (s, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
> -  clear_no_long_double ();
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vfprintf_internal (s, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___vfprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___vfwprintf_chk (FILE *s, int flag, const wchar_t *fmt, va_list ap)
>  {
> -  int res;
> -  set_no_long_double ();
> -  res = __vfwprintf_internal (s, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
> -  clear_no_long_double ();
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vfwprintf_internal (s, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___vfwprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___vprintf_chk (int flag, const char *fmt, va_list ap)
>  {
> -  return __nldbl___vfprintf_chk (stdout, flag, fmt, ap);
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vfprintf_internal (stdout, fmt, ap, mode);
>  }
>  

Ok.

>  int
> @@ -612,14 +573,12 @@ __nldbl___vsnprintf_chk (char *string, size_t maxlen, int flag, size_t slen,
>    if (__glibc_unlikely (slen < maxlen))
>      __chk_fail ();
>  
> -  int res;
> -  __no_long_double = 1;
> -  res = __vsnprintf_internal (string, maxlen, fmt, ap,
> -      (flag > 0) ? PRINTF_FORTIFY : 0);
> -  __no_long_double = 0;
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vsnprintf_internal (string, maxlen, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___vsnprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
> @@ -629,14 +588,12 @@ __nldbl___vsprintf_chk (char *string, int flag, size_t slen, const char *fmt,
>    if (slen == 0)
>      __chk_fail ();
>  
> -  int res;
> -  __no_long_double = 1;
> -  res = __vsprintf_internal (string, slen, fmt, ap,
> -     (flag > 0) ? PRINTF_FORTIFY : 0);
> -  __no_long_double = 0;
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vsprintf_internal (string, slen, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___vsprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
> @@ -646,116 +603,125 @@ __nldbl___vswprintf_chk (wchar_t *string, size_t maxlen, int flag, size_t slen,
>    if (__glibc_unlikely (slen < maxlen))
>      __chk_fail ();
>  
> -  int res;
> -  __no_long_double = 1;
> -  res = __vswprintf_internal (string, maxlen, fmt, ap,
> -      (flag > 0) ? PRINTF_FORTIFY : 0);
> -  __no_long_double = 0;
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vswprintf_internal (string, maxlen, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___vswprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___vwprintf_chk (int flag, const wchar_t *fmt, va_list ap)
>  {
> -  return __nldbl___vfwprintf_chk (stdout, flag, fmt, ap);
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vfwprintf_internal (stdout, fmt, ap, mode);
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___wprintf_chk (int flag, const wchar_t *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vfwprintf_chk (stdout, flag, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vfwprintf_internal (stdout, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
> -__nldbl___vasprintf_chk (char **ptr, int flag, const char *fmt, va_list arg)
> +__nldbl___vasprintf_chk (char **ptr, int flag, const char *fmt, va_list ap)
>  {
> -  int res;
> -  __no_long_double = 1;
> -  res = __vasprintf_internal (ptr, fmt, arg,
> -      (flag > 0) ? PRINTF_FORTIFY : 0);
> -  __no_long_double = 0;
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vasprintf_internal (ptr, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___vasprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___asprintf_chk (char **ptr, int flag, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vasprintf_chk (ptr, flag, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vasprintf_internal (ptr, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
> -__nldbl___vdprintf_chk (int d, int flag, const char *fmt, va_list arg)
> +__nldbl___vdprintf_chk (int d, int flag, const char *fmt, va_list ap)
>  {
> -  int res;
> -  set_no_long_double ();
> -  res = __vdprintf_internal (d, fmt, arg, (flag > 0) ? PRINTF_FORTIFY : 0);
> -  clear_no_long_double ();
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __vdprintf_internal (d, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___vdprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___dprintf_chk (int d, int flag, const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___vdprintf_chk (d, flag, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __vdprintf_internal (d, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___obstack_vprintf_chk (struct obstack *obstack, int flag,
> -       const char *fmt, va_list arg)
> +       const char *fmt, va_list ap)
>  {
> -  int res;
> -  __no_long_double = 1;
> -  res = __obstack_vprintf_internal (obstack, fmt, arg,
> -    (flag > 0) ? PRINTF_FORTIFY : 0);
> -  __no_long_double = 0;
> -  return res;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
> +
> +  return __obstack_vprintf_internal (obstack, fmt, ap, mode);
>  }
> -libc_hidden_def (__nldbl___obstack_vprintf_chk)
>  

Ok.

>  int
>  attribute_compat_text_section
>  __nldbl___obstack_printf_chk (struct obstack *obstack, int flag,
>        const char *fmt, ...)
>  {
> -  va_list arg;
> -  int done;
> +  va_list ap;
> +  int ret;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -  va_start (arg, fmt);
> -  done = __nldbl___obstack_vprintf_chk (obstack, flag, fmt, arg);
> -  va_end (arg);
> +  va_start (ap, fmt);
> +  ret = __obstack_vprintf_internal (obstack, fmt, ap, mode);
> +  va_end (ap);
>  
> -  return done;
> +  return ret;
>  }
>  

Ok.

>  extern __typeof (printf_size) __printf_size;
> @@ -837,18 +803,28 @@ __nldbl_syslog (int pri, const char *fmt, ...)
>  {
>    va_list ap;
>    va_start (ap, fmt);
> -  __nldbl___vsyslog_chk (pri, -1, fmt, ap);
> +  __vsyslog_internal (pri, fmt, ap, PRINTF_LDBL_IS_DBL);
>    va_end (ap);
>  }
>  
> +void
> +attribute_compat_text_section
> +__nldbl_vsyslog (int pri, const char *fmt, va_list ap)
> +{
> +  __vsyslog_internal (pri, fmt, ap, PRINTF_LDBL_IS_DBL);
> +}
> +

Ok.

>  void
>  attribute_compat_text_section
>  __nldbl___syslog_chk (int pri, int flag, const char *fmt, ...)
>  {
>    va_list ap;
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
>    va_start (ap, fmt);
> -  __nldbl___vsyslog_chk (pri, flag, fmt, ap);
> +  __vsyslog_internal (pri, fmt, ap, mode);
>    va_end(ap);
>  }
>  

Ok.

> @@ -856,17 +832,11 @@ void
>  attribute_compat_text_section
>  __nldbl___vsyslog_chk (int pri, int flag, const char *fmt, va_list ap)
>  {
> -  set_no_long_double ();
> -  __vsyslog_internal (pri, fmt, ap, (flag > 0) ? PRINTF_FORTIFY : 0);
> -  clear_no_long_double ();
> -}
> -libc_hidden_def (__nldbl___vsyslog_chk)
> +  unsigned int mode = PRINTF_LDBL_IS_DBL;
> +  if (flag > 0)
> +    mode |= PRINTF_FORTIFY;
>  
> -void
> -attribute_compat_text_section
> -__nldbl_vsyslog (int pri, const char *fmt, va_list ap)
> -{
> -  __nldbl___vsyslog_chk (pri, -1, fmt, ap);
> +  __vsyslog_internal (pri, fmt, ap, mode);
>  }
>  
>  int
> -- 2.14.5
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.

Adhemerval Zanella-2
In reply to this post by Gabriel F. T. Gomes-2


On 29/10/2018 09:24, Gabriel F. T. Gomes wrote:

> On Mon, 29 Oct 2018, Gabriel F. T. Gomes wrote:
>
>> From: Zack Weinberg <[hidden email]>
>>
>> int
>> -__vdprintf_chk (int d, int flags, const char *format, va_list arg)
>> +__vdprintf_chk (int d, int flag, const char *format, va_list ap)
>> {
>>
>> [...]
>>
>> -  done = __vfprintf_internal (&tmpfil.file, format, arg, 0);
>>
>> [...]
>>
>> -  return done;
>> +  return __vdprintf_internal (d, format, ap, mode);
>
> While reviewing the first version of this patch, I noticed that it could
> have an effect on bug 20231, because, after this patch, __vdprintf_chk
> will call __vdprintf_internal, which has the extra check for EOF (as
> reported in the bug).  However, the bug is marked as unconfirmed, and I
> was unable to reproduce it with the following test case:
>
>     #include <stdio.h>
>     #include <stdarg.h>
>     #include <sys/types.h>
>     #include <sys/stat.h>
>     #include <fcntl.h>
>     #include <unistd.h>
>     int
>     main (void)
>     {
>       int fd, libret, sysret;
>       va_list ap;
>       fd = open ("/tmp/blablabla", O_RDWR | O_TRUNC | O_CREAT, S_IRWXU);
>       if (fd == -1)
>         perror (NULL);
>       sysret = close (fd);
>       if (sysret)
>         perror (NULL);
>       libret = vdprintf (fd, "blablabla", ap);
>       if (libret != EOF)
>         printf ("Bug 20231 reproduced\n");
>       return 0;
>     }
>
> Maybe the test case is wrong, in which case I could fix it, then mark this
> patch as solving bug 20231.  On the other hand, if the test case is
> correct, we could just close bug 20231.
>

The BZ#20231 described issue does not happen, vdprintf_chk indeed returns EOF
when trying to write on a closed file. It will currently call on master:

  _IO_new_file_seekoff
  \_ _IO_new_file_attach
     \_ __GI___vdprintf_chk
        \_ _IO_file_seek
           \_ __lseek64

And the __lseek64 will return -1/EBADF and thus _IO_new_file_attach will fail.

The code difference with default vdprintf is, in fact, BZ#11319, where vdprintf_chk
does not return an error when an output error occurs (the same example reported on
BZ#11319 fails with -D_FORTIFY_SOURCE=2 -O2).

I reopened BZ#11319 noting the fortified also needs fixing and the good thing is
the 'Add __v*printf_internal with flags arguments.' [1] refactor fixes it by
using a common correct function for both fortified and non-fortified vfprintf.
Please add a note that this patch fixes BZ#11319.

[1] https://sourceware.org/ml/libc-alpha/2018-10/msg00616.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 5/8] Add __v*printf_internal with flags arguments.

Adhemerval Zanella-2
In reply to this post by Adhemerval Zanella-2


On 07/11/2018 16:53, Adhemerval Zanella wrote:

>
>
> On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:
>> From: Zack Weinberg <[hidden email]>
>> [...]
>> There are a lot more printf variants than there are scanf variants,
>> and the code for setting up and tearing down their custom FILE
>> variants around the call to __vf(w)printf is more complicated and
>> variable.  Therefore, I have added _internal versions of all the
>> v*printf variants, rather than introducing helper routines so that
>> they can all directly call __vf(w)printf_internal, as was done with
>> scanf.
>>
>> As with the scanf changes, in this patch the _internal functions still
>> look at the environmental mode bits and all callers pass 0 for the
>> flags parameter.
>>
>> Several of the affected public functions had _IO_ name aliases that
>> were not exported (but, in one case, appeared in libio.h anyway);
>> I was originally planning to leave them as aliases to avoid having
>> to touch internal callers, but it turns out ldbl_*_alias only work
>> for exported symbols, so they've all been removed instead.  It also
>> turns out there were hardly any internal callers.  _IO_vsprintf and
>> _IO_vfprintf *are* exported, so those two stick around.

Please add a note that it fixes BZ#11319 by unifing both fortified and
non-fortified vdprintf initialization.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/8] Use STRFMON_LDBL_IS_DBL instead of __ldbl_is_dbl.

Gabriel F. T. Gomes-2
In reply to this post by Adhemerval Zanella-2
On Wed, 07 Nov 2018, Adhemerval Zanella wrote:

>On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:
>> From: Zack Weinberg <[hidden email]>
>>
>> +extern ssize_t
>> +__vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
>> +       const char *format, va_list ap,
>> +       unsigned int flags)
>> +  attribute_hidden;
>> +
>> +/* Flags for __vstrfmon_l_internal.  */
>> +#define STRFMON_LDBL_IS_DBL 0x0001  
>
>Please extend the flag comment to describe what it does.

With the proposed text (below) and DCO's signed-off-by statements removed
from the commit message, is it OK for master?

+/* Flags for __vstrfmon_l_internal.
+
+   STRFMON_LDBL_IS_DBL is a one-bit mask for the flags parameter that
+   indicates whether long double values are to be handled as having the
+   same format as double, in which case the flag should be set to one,
+   or as another format, otherwise.  */
+#define STRFMON_LDBL_IS_DBL 0x0001
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 2/8] Add __vfscanf_internal and __vfwscanf_internal with flags arguments.

Gabriel F. T. Gomes-2
In reply to this post by Adhemerval Zanella-2
On Wed, 07 Nov 2018, Adhemerval Zanella wrote:

>On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:
>
>> Additional note for review:
>>
>>   - Reviewing the changes from vfscanf.c to vfscanf-internal.c in the
>>     original patch would be vey hard, because git doesn't detect the
>>     filename change.  To make review a little easier, I did as Zack did
>>     and manually edited the diff.  I'll reply to this thread and attach
>>     the original patch if someone wants to apply it.
>>     (ping me if I forget it)  
>
>I would advise to avoid it, it is error-prone and we do have tools to
>handle it.  One option is to use -C and -M git options with a higher s
>imilarity index, for instance on this patch using:
>
>git format-patch -M90% -C -1
>
>it generates the expected result:

It does, indeed.  Thanks for the explanation.  I'll use it from now on.

>> Signed-off-by: Zack Weinberg <[hidden email]>
>> Signed-off-by: Gabriel F. T. Gomes <[hidden email]>  
>
>As Florian has said, we don't use DCO, but copyright assignments.

Ack, removed.

>> +/* Flags for __vfscanf_internal and __vfwscanf_internal.  */
>> +#define SCANF_LDBL_IS_DBL 0x0001
>> +#define SCANF_ISOC99_A    0x0002  
>
>As before, please describe what actually the flag does.

Ack, how about the following text?

+/* Flags for __vfscanf_internal and __vfwscanf_internal.
+
+   SCANF_LDBL_IS_DBL indicates whether long double values are to be
+   handled as having the same format as double, in which case the flag
+   should be set to one, or as another format, otherwise.
+
+   SCANF_ISOC99_A, when set to one, indicates that the ISO C99 or POSIX
+   behavior of the scanf functions is to be used, i.e. automatic
+   allocation for input strings with %as, %aS and %a[, a GNU extension,
+   is disabled. This is the behavior that the __isoc99_scanf family of
+   functions use.  When the flag is set to zero, automatic allocation is
+   enabled.  */
+#define SCANF_LDBL_IS_DBL 0x0001
+#define SCANF_ISOC99_A    0x0002

>> +#define LDBL_DISTINCT (__glibc_likely ((mode_flags & SCANF_LDBL_IS_DBL) == 0))
>> +#define USE_ISOC99_A  (__glibc_likely (mode_flags & SCANF_ISOC99_A))  
>
>Do we really gain anything using these defines? I tend to frown on macros
>that use internal named variables instead of set them as arguments. Also
>this is quite short, I think it is more readable to just use the comparison
>directly.

I don't have a strong opinion about it, but it does look good with the
comparison directly where it is used.

Fixed as suggested.

>> +/* Same as compat_symbol, ldbl_compat_symbol is not to be used outside
>> +   '#if SHLIB_COMPAT' statement and should fail if it is.  */
>> +# define ldbl_compat_symbol(lib, local, symbol, version) ...  
>
>Why not use an _Static_assert(0, "ldbl_compat_symbol should be used inside SHLIB_COMPAT")?

No reason other than I wasn't aware of _Static_assert.

Fixed as suggested.


I'll send a new version when I finish reviewing the whole patch set.

Thanks for the careful review.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.

Gabriel F. T. Gomes-2
In reply to this post by Adhemerval Zanella-2
On Thu, 08 Nov 2018, Adhemerval Zanella wrote:

>On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:
>
>> Signed-off-by: Zack Weinberg <[hidden email]>
>> Signed-off-by: Gabriel F. T. Gomes <[hidden email]>  
>
>We don't use DCO, but copyright assignments.

Ack, fixed.

>> --- /dev/null
>> +++ b/debug/vobprintf_chk.c
>> @@ -0,0 +1,32 @@
>> +/* Print output of stream to given obstack.
>> +   Copyright (C) 1996-2018 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +   Contributed by Ulrich Drepper <[hidden email]>, 1996.  
>
>I think it is better to just set Copyright to 2018 without 'Contributed by'
>(this implementation is quite different than the original one).

Ack, fixed.

>> -___sprintf_chk (char *s, int flags, size_t slen, const char *format, ...)
>> +___sprintf_chk (char *s, int flag, size_t slen, const char *format, ...)
>> [...]
>> +  if (slen == 0)
>> +    __chk_fail ();  
>
>Maybe a __glibc_unlikely here?

Looks right to me, added.  Please see comments below.

>>  __nldbl___vsprintf_chk (char *string, int flag, size_t slen, const char *fmt,
>>   va_list ap)
>>  {
>> +  if (slen == 0)
>> +    __chk_fail ();

I added __glibc_unlikely here, too.

>>  int
>> -___vsprintf_chk (char *s, int flags, size_t slen, const char *format,
>> - va_list args)
>> +___vsprintf_chk (char *s, int flag, size_t slen, const char *format,
>> + va_list ap)
>> [...]
>>    if (slen == 0)
>>      __chk_fail ();

This is an unchanged hunk (brought in by diff context), but may I also add
__glibc_unlikely to it along with the other changes?

These are the only ocurrences of slen == 0 (__nldbl___sprintf_chk calls
__nldbl___vsprintf_chk, so it doesn't check slen on its own).
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.

Florian Weimer-5
In reply to this post by Adhemerval Zanella-2
* Adhemerval Zanella:

>> +  if (slen == 0)
>> +    __chk_fail ();
>
> Maybe a __glibc_unlikely here?

I don't think this is required for paths which lead to a noreturn
function.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 5/8] Add __v*printf_internal with flags arguments.

Gabriel F. T. Gomes-2
In reply to this post by Adhemerval Zanella-2
On Thu, 08 Nov 2018, Adhemerval Zanella wrote:

>Please add a note that it fixes BZ#11319 by unifing both fortified and
>non-fortified vdprintf initialization.

I'll add this note to patch 7/8, since the fix is there.  Thanks for
checking and closing bug 20231, and for reopening 11319 and pointing it
out to me.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/8] Use STRFMON_LDBL_IS_DBL instead of __ldbl_is_dbl.

Adhemerval Zanella-2
In reply to this post by Gabriel F. T. Gomes-2


On 13/11/2018 10:08, Gabriel F. T. Gomes wrote:

> On Wed, 07 Nov 2018, Adhemerval Zanella wrote:
>> On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:
>>> From: Zack Weinberg <[hidden email]>
>>>
>>> +extern ssize_t
>>> +__vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
>>> +       const char *format, va_list ap,
>>> +       unsigned int flags)
>>> +  attribute_hidden;
>>> +
>>> +/* Flags for __vstrfmon_l_internal.  */
>>> +#define STRFMON_LDBL_IS_DBL 0x0001  
>>
>> Please extend the flag comment to describe what it does.
>
> With the proposed text (below) and DCO's signed-off-by statements removed
> from the commit message, is it OK for master?
>
> +/* Flags for __vstrfmon_l_internal.
> +
> +   STRFMON_LDBL_IS_DBL is a one-bit mask for the flags parameter that
> +   indicates whether long double values are to be handled as having the
> +   same format as double, in which case the flag should be set to one,
> +   or as another format, otherwise.  */
> +#define STRFMON_LDBL_IS_DBL 0x0001
>

LGTM, thanks.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.

Adhemerval Zanella-2
In reply to this post by Florian Weimer-5


On 15/11/2018 08:24, Florian Weimer wrote:

> * Adhemerval Zanella:
>
>>> +  if (slen == 0)
>>> +    __chk_fail ();
>>
>> Maybe a __glibc_unlikely here?
>
> I don't think this is required for paths which lead to a noreturn
> function.
>
> Thanks,
> Florian
>

Right, so I ok with current change as is.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.

Adhemerval Zanella-2
In reply to this post by Gabriel F. T. Gomes-2


On 15/11/2018 07:08, Gabriel F. T. Gomes wrote:

> On Thu, 08 Nov 2018, Adhemerval Zanella wrote:
>
>> On 29/10/2018 09:16, Gabriel F. T. Gomes wrote:
>>
>>> Signed-off-by: Zack Weinberg <[hidden email]>
>>> Signed-off-by: Gabriel F. T. Gomes <[hidden email]>  
>>
>> We don't use DCO, but copyright assignments.
>
> Ack, fixed.
>
>>> --- /dev/null
>>> +++ b/debug/vobprintf_chk.c
>>> @@ -0,0 +1,32 @@
>>> +/* Print output of stream to given obstack.
>>> +   Copyright (C) 1996-2018 Free Software Foundation, Inc.
>>> +   This file is part of the GNU C Library.
>>> +   Contributed by Ulrich Drepper <[hidden email]>, 1996.  
>>
>> I think it is better to just set Copyright to 2018 without 'Contributed by'
>> (this implementation is quite different than the original one).
>
> Ack, fixed.
>
>>> -___sprintf_chk (char *s, int flags, size_t slen, const char *format, ...)
>>> +___sprintf_chk (char *s, int flag, size_t slen, const char *format, ...)
>>> [...]
>>> +  if (slen == 0)
>>> +    __chk_fail ();  
>>
>> Maybe a __glibc_unlikely here?
>
> Looks right to me, added.  Please see comments below.
>
>>>  __nldbl___vsprintf_chk (char *string, int flag, size_t slen, const char *fmt,
>>>   va_list ap)
>>>  {
>>> +  if (slen == 0)
>>> +    __chk_fail ();
>
> I added __glibc_unlikely here, too.
>
>>>  int
>>> -___vsprintf_chk (char *s, int flags, size_t slen, const char *format,
>>> - va_list args)
>>> +___vsprintf_chk (char *s, int flag, size_t slen, const char *format,
>>> + va_list ap)
>>> [...]
>>>    if (slen == 0)
>>>      __chk_fail ();
>
> This is an unchanged hunk (brought in by diff context), but may I also add
> __glibc_unlikely to it along with the other changes?
>
> These are the only ocurrences of slen == 0 (__nldbl___sprintf_chk calls
> __nldbl___vsprintf_chk, so it doesn't check slen on its own).
>

As Florian has pointed out I also think this is not really necessary.
So I am ok with current changes.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/8] Use STRFMON_LDBL_IS_DBL instead of __ldbl_is_dbl.

Gabriel F. T. Gomes-2
In reply to this post by Adhemerval Zanella-2
On Thu, 15 Nov 2018, Adhemerval Zanella wrote:

>On 13/11/2018 10:08, Gabriel F. T. Gomes wrote:
>>
>> +/* Flags for __vstrfmon_l_internal.
>> +
>> +   STRFMON_LDBL_IS_DBL is a one-bit mask for the flags parameter that
>> +   indicates whether long double values are to be handled as having the
>> +   same format as double, in which case the flag should be set to one,
>> +   or as another format, otherwise.  */
>> +#define STRFMON_LDBL_IS_DBL 0x0001
>>  
>
>LGTM, thanks.

Thanks, now committed with this change.
12