Re: nano printf + powerpc gcc

[Eric Blake <eblake at redhat dot com>]

On 01/29/2018 02:10 PM, Alexey Neyman wrote:
> Wasn't the purpose of passing down `va_list *` to allow the callee to
> modify the current state of the varargs parser in the caller? As far as
> I understand, with the va_copy the callee (_printf_float) will get the
> arguments from the beginning of the varargs, disregarding the arguments
> already fetched by the caller (_VFPRINTF_R).

va_copy() is required to preserve the current state of the original:

   va_copy()
       The va_copy() macro copies the (previously initialized) variable
argu‐
       ment  list  src to dest.  The behavior is as if va_start() were
applied
       to dest with the same last argument, followed by  the  same
number  of
       va_arg() invocations that was used to reach the current state of src.

As long as all operations on the va_list are done through the copy, and
not some on the original while others are on the copy, then we should be
fine, and that appears to be the case with the proposed patch (the copy
is done prior to any access to ap, and all uses of ap were changed to
instead be ap_copy; we aren't worried about the caller of _VFPRINTF_R
having consumed any prior arguments or needing to consume any further
arguments; the worry is only about _printf_float being able to affect
_VFPRINTF_R which is the case).

Still, rather than making sure we changed all uses of ap to ap_copy
within _VFPRINTF_R, it may be simpler to rename the function parameter
and do the copy as soon as possible, so that you then use the preferred
name through the rest of the function, as in:

diff --git i/newlib/libc/stdio/nano-vfprintf.c
w/newlib/libc/stdio/nano-vfprintf.c
index ed7316a77..397894f31 100644
--- i/newlib/libc/stdio/nano-vfprintf.c
+++ w/newlib/libc/stdio/nano-vfprintf.c
@@ -169,15 +169,6 @@ static char *rcsid = "$Id$";
 #include "nano-vfprintf_local.h"


-/* GCC PR 14577 at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557 */
-#if __STDC_VERSION__ >= 201112L
-#define va_ptr(ap) _Generic(&(ap), va_list *: &(ap), default: (va_list
*)(ap))
-#elif __GNUC__ >= 4
-#define va_ptr(ap)
__builtin_choose_expr(__builtin_types_compatible_p(__typeof__(&(ap)),
va_list *), &(ap), (va_list *)(ap))
-#else
-#define va_ptr(ap) (sizeof(ap) == sizeof(va_list) ? (va_list *)&(ap) :
(va_list *)(ap))
-#endif
-
 /* The __ssputs_r function is shared between all versions of vfprintf
    and vfwprintf.  */
 #ifdef STRING_ONLY
@@ -478,13 +469,14 @@ int
 _VFPRINTF_R (struct _reent *data,
        FILE * fp,
        const char *fmt0,
-       va_list ap)
+       va_list ap_orig)
 {
   register char *fmt;	/* Format string.  */
   register int n, m;	/* Handy integers (short term usage).  */
   register char *cp;	/* Handy char pointer (short term usage).  */
   const char *flag_chars;
   struct _prt_data_t prt_data;	/* All data for decoding format string.  */
+  va_list ap;

   /* Output function pointer.  */
   int (*pfunc)(struct _reent *, FILE *, const char *, size_t len);
@@ -522,6 +514,10 @@ _VFPRINTF_R (struct _reent *data,
   prt_data.blank = ' ';
   prt_data.zero = '0';

+  /* Operate on a copy of va_orig so we can take the address of ap
+   * regardless of whether va_list is an array type */
+  va_copy (ap, ap_orig);
+
   /* Scan the format for conversions (`%' character).  */
   for (;;)
     {
@@ -636,12 +632,12 @@ _VFPRINTF_R (struct _reent *data,
 	    }
 	  else
 	    {
-	      n = _printf_float (data, &prt_data, fp, pfunc, va_ptr(ap));
+	      n = _printf_float (data, &prt_data, fp, pfunc, &ap);
 	    }
 	}
       else
 #endif
-	n = _printf_i (data, &prt_data, fp, pfunc, va_ptr(ap));
+	n = _printf_i (data, &prt_data, fp, pfunc, &ap);

       if (n == -1)
 	goto error;
@@ -654,6 +650,7 @@ error:
 #ifndef STRING_ONLY
   _newlib_flockfile_end (fp);
 #endif
+  va_end (ap);
   return (__sferror (fp) ? EOF : prt_data.ret);
 }



-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature