[Bug stdio/18982] New: va_list and vprintf

[Bug stdio/18982] New: va_list and vprintf

[mx2927 at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=18982

            Bug ID: 18982
           Summary: va_list and vprintf
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: stdio
          Assignee: unassigned at sourceware dot org
          Reporter: mx2927 at gmail dot com
  Target Milestone: ---

I stumblued in the following problem with va_list and vprintf; consider the
following code sample:

// begin
#include <stdio.h>
#include <stdarg.h>


void do_print(char* fmt, ...)
{
  va_list ap;
  va_start(ap, fmt);


  vprintf(fmt, ap);
  vprintf(fmt, ap);

  va_end(ap);
}


int main()
{

  do_print("Hello %d, %d, %d\n", 1, 2, 3, 4, 5, 6);

  return 0;
}
// end

The libc manual says about vprintf:

"GNU C does not have such restrictions. You can safely continue to fetch
arguments from a va_list pointer after passing it to vprintf, and va_end is a
no-op. (Note, however, that subsequent va_arg calls will fetch the same
arguments which vprintf previously used.)" The restrictions mentioned are that
the va_list pointer becomes invalid after a call to vprintf:

http://www.gnu.org/software/libc/manual/html_node/Variable-Arguments-Output.html#Variable-Arguments-Output

glibc 2.20 on i686 (Fedora21) output of the above sample is:
Hello 1, 2, 3
Hello 1, 2, 3

Which is as expected according to the description from the manual.

glibc 2.20 on x86_64 (Fedora21) output of the above sample is:
Hello 1, 2, 3
Hello 4, 5, 6

which shows that the va_list pointer is not handled according to the
description from the manual. In fact if the example would provide only 3
variable arguments, the 32bit implementation would keep the va_list pointer
valid, while the 64 bit implementation would make the pointer invalid.

Besides the behaviour described in the manual, what I think is particularly
appreciable of the 32bit result is that it fulfils the C standard of arguments
being passed "by value", i.e. the va_list argument to vprintf is passed by
value, thus the original is not modified by vprintf.

Anyway, IMHO, I think that the behaviour should be the same on both
architectures, but even if by design the 32bit result would not be achieved on
64bit, at least the documentation should reflect the actual behaviour.

The problem applies to the family of vprintf functions: vprintf, vfprintf,
vsprintf and vsnprintf.

See also
https://sourceware.org/ml/libc-help/2015-08/msg00038.html
https://sourceware.org/ml/libc-help/2015-09/msg00011.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/18982] va_list and vprintf

[joseph at codesourcery dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=18982

--- Comment #1 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
As specified in the x86_64 ABI, va_list is an array type on x86_64.  
Thus, this is a documentation issue (failing to point out that when 
va_list is an array type, it is passed by reference).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/18982] va_list and vprintf

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=18982

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/18982] va_list and vprintf

[mx2927 at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=18982

--- Comment #2 from M Manfredini <mx2927 at gmail dot com> ---
(In reply to joseph@codesourcery.com from comment #1)
> As specified in the x86_64 ABI, va_list is an array type on x86_64.  
> Thus, this is a documentation issue (failing to point out that when 
> va_list is an array type, it is passed by reference).

If it is going to be handled as a documentation issue, I think that it should
specify that vprintf and friends make va_list unusable after the call,
irrespective of the platform.
I.e. portability as the behavior that can reliably be expected on different
platforms.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/18982] va_list and vprintf

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=18982

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at sourceware dot org   |fweimer at redhat dot com

--- Comment #3 from Florian Weimer <fweimer at redhat dot com> ---
(In reply to joseph@codesourcery.com from comment #1)
> As specified in the x86_64 ABI, va_list is an array type on x86_64.  
> Thus, this is a documentation issue (failing to point out that when 
> va_list is an array type, it is passed by reference).

We could theoretically use va_copy in vprint and similar functions, and restore
it before returning.  But this has a performance impact, and I don't think it's
worth it.

So I'm in favor of a documentation change as well.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/18982] va_list and vprintf

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=18982

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  4e1665024a48da9c865e991aff03a5ad03c771aa (commit)
       via  6782806d8f6664d87d17bb30f8ce4e0c7c931e17 (commit)
       via  db8ad8fac3ebdb6c68964f4d24185c2bd2c83342 (commit)
       via  52fb79d6cdecb89a6f0375091e7c12ed79ae6760 (commit)
       via  f546f87c4ffb1642ffc96b8d614c329ed35252c3 (commit)
      from  213938ee8ad50156063c3b38c3d236c6f713eb4a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4e1665024a48da9c865e991aff03a5ad03c771aa

commit 4e1665024a48da9c865e991aff03a5ad03c771aa
Author: Florian Weimer <fweimer@redhat.com>
Date:   Sat Oct 17 12:07:04 2015 +0200

    sunrpc: Rewrite with explicit TLS access using __thread

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=6782806d8f6664d87d17bb30f8ce4e0c7c931e17

commit 6782806d8f6664d87d17bb30f8ce4e0c7c931e17
Author: Florian Weimer <fweimer@redhat.com>
Date:   Sat Oct 17 12:06:48 2015 +0200

    malloc: Rewrite with explicit TLS access using __thread

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=db8ad8fac3ebdb6c68964f4d24185c2bd2c83342

commit db8ad8fac3ebdb6c68964f4d24185c2bd2c83342
Author: Florian Weimer <fweimer@redhat.com>
Date:   Sat Oct 17 12:05:12 2015 +0200

    vfprintf: Rewrite printf_positional to use struct scratch_buffer

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=52fb79d6cdecb89a6f0375091e7c12ed79ae6760

commit 52fb79d6cdecb89a6f0375091e7c12ed79ae6760
Author: Florian Weimer <fweimer@redhat.com>
Date:   Sat Oct 17 12:02:37 2015 +0200

    Assume that SOCK_CLOEXEC is available and works

    This fixes (harmless) data races when accessing the various
    __have_sock_cloexec variables.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f546f87c4ffb1642ffc96b8d614c329ed35252c3

commit f546f87c4ffb1642ffc96b8d614c329ed35252c3
Author: Florian Weimer <fweimer@redhat.com>
Date:   Sat Oct 17 12:02:22 2015 +0200

    The va_list pointer is unspecified after a call to vfprintf [BZ #18982]

    This adjusts the documentation to the existing implementation.

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

Summary of changes:
 ChangeLog                                 |   64 +++++++++++++++++++++++++++
 NEWS                                      |    6 +-
 include/sys/socket.h                      |    8 ---
 include/unistd.h                          |    1 -
 malloc/arena.c                            |   48 ++++++++------------
 manual/memory.texi                        |    4 --
 manual/stdio.texi                         |   24 ++++------
 misc/syslog.c                             |   33 +-------------
 nis/ypclnt.c                              |   16 +------
 nscd/connections.c                        |   68 +----------------------------
 nscd/nscd_helper.c                        |   27 +-----------
 resolv/res_send.c                         |   49 ++-------------------
 socket/have_sock_cloexec.c                |    4 --
 stdio-common/vfprintf.c                   |   50 ++++-----------------
 sunrpc/clnt_udp.c                         |   36 +---------------
 sunrpc/rpc_thread.c                       |   22 +++------
 sysdeps/generic/malloc-machine.h          |    7 ---
 sysdeps/mach/hurd/malloc-machine.h        |   10 ----
 sysdeps/nptl/malloc-machine.h             |   10 ----
 sysdeps/unix/sysv/linux/kernel-features.h |    1 -
 20 files changed, 125 insertions(+), 363 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug stdio/18982] va_list and vprintf

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=18982

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |2.23

--- Comment #5 from Florian Weimer <fweimer at redhat dot com> ---
Fixed in 2.23.

-- 
You are receiving this mail because:
You are on the CC list for the bug.