This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: nano printf + powerpc gcc


Okay, attached is a quick simulation - using a similar code that uses va_arg in the caller then either passes a copy of the va_list to a callee, or passes it by pointer.

When compiled with -DUSE_VA_POINTERS, either for x86_64 or for i386, it prints correct values from both f1/f2 for all ten iterations:

f2#0: 5
f1#0: 0.500000
f2#1: 5
f1#1: 0.500000
f2#2: 5
f1#2: 0.500000
f2#3: 5
f1#3: 0.500000
f2#4: 5
f1#4: 0.500000
f2#5: 5
f1#5: 0.500000
f2#6: 5
f1#6: 0.500000
f2#7: 5
f1#7: 0.500000
f2#8: 5
f1#8: 0.500000
f2#9: 5
f1#9: 0.500000

However, when passing a va_copy'ed va_list to the callee, it starts to print junk. For x86_64, this happens when it runs out of registers to pass the values - look at the last 3 lines:

f2#0: 5
f1#0: 0.500000
f2#1: 5
f1#1: 0.500000
f2#2: 5
f1#2: 0.500000
f2#3: 5
f1#3: 0.500000
f2#4: 5
f1#4: 0.500000
f2#5: 5
f1#5: 0.500000
f2#6: 5
f1#6: 0.500000
f2#7: 5
f1#7: 0.500000
f2#8: 5
f1#8: 0.000000
f2#9: 0
f1#9: 0.000000

It becomes even more pronounced on i386 where both integers and floating point arguments are placed into the stack:

f2#0: 5
f1#0: 0.000000
f2#1: 0
f1#1: 0.000000
f2#2: 1071644672
f1#2: 0.000000
f2#3: 5
f1#3: 0.000000
f2#4: 0
f1#4: 0.000000
f2#5: 1071644672
f1#5: 0.000000
f2#6: 5
f1#6: 0.000000
f2#7: 0
f1#7: 0.000000
f2#8: 1071644672
f1#8: 0.000000
f2#9: 5
f1#9: 0.000000

So no, va_copy-based approach does not work where the callee needs to *modify* the current state of the va_list in the caller.

Regards,
Alexey.


On 01/29/2018 12:45 PM, Jon Beniston wrote:
Hi Alexey,

That works okay for me.

Cheers,
Jon

-----Original Message-----
From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org] On Behalf Of Alexey Neyman
Sent: 29 January 2018 20:10
To: Eric Blake; Jon Beniston; 'Alexander Fedotov'; 'Andre Vieira (lists)'
Cc: newlib@sourceware.org
Subject: Re: nano printf + powerpc gcc

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).

I'd suggest to verify the corner cases where floating point arguments are intermixed with integer arguments and both exceed the number of the registers available for passing them - causing both to spill into the stack. On PPC, that would be more than 8 integer and 8 floating point arguments, as far as I remember the ABI. Something like:

          printf("%u %f %u %f %u %f %u %f %u %f %u %f %u %f %u %f %u %f %u %f\n",
                          5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5,
                          5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5);

Regards,
Alexey.


On 01/29/2018 07:48 AM, Eric Blake wrote:
On 01/29/2018 05:56 AM, Jon Beniston wrote:
Hi,

Is this patch what is recommended? Seems to fix it for me (but only very briefly tested on my target).
@@ -485,6 +475,7 @@ _VFPRINTF_R (struct _reent *data,
     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_copy;
/* Output function pointer. */
     int (*pfunc)(struct _reent *, FILE *, const char *, size_t len);
@@ -522,6 +513,8 @@ _VFPRINTF_R (struct _reent *data,
     prt_data.blank = ' ';
     prt_data.zero = '0';
+ va_copy (ap_copy, ap);
+
     /* Scan the format for conversions (`%' character).  */
     for (;;)
       {
@@ -577,7 +570,7 @@ _VFPRINTF_R (struct _reent *data,
   	   *	-- ANSI X3J11
   	   * They don't exclude field widths read from args.
   	   */
-	  prt_data.width = GET_ARG (n, ap, int);
+	  prt_data.width = GET_ARG (n, ap_copy, int);
...
   	  else
-	    {
-	      n = _printf_float (data, &prt_data, fp, pfunc, va_ptr(ap));
-	    }
+            n = _printf_float (data, &prt_data, fp, pfunc,
+ &ap_copy);
Maybe a comment why the copy is needed is still in order, but yes,
this matches what I was thinking.




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


static void f1(int i,
#if defined(USE_VA_POINTERS)
		va_list *p_ap
#else
		va_list ap
#endif
		)
{
	double x;

#if defined(USE_VA_POINTERS)
	x = va_arg(*p_ap, double);
#else
	x = va_arg(ap, double);
#endif
	printf("%s#%u: %f\n", __func__, i, x);
}

static void f2(va_list ap)
{
	int i, x;
#if !defined(USE_VA_POINTERS)
	va_list ap2;

	va_copy(ap2, ap);
#endif
	for (i = 0; i < 10; i++) {
		x = va_arg(ap, int);
		printf("%s#%u: %u\n", __func__, i, x);
#if defined(USE_VA_POINTERS)
#if defined(__x86_64__)
		f1(i, (va_list *)ap);
#elif defined(__i386__)
		f1(i, &ap);
#else
#error How to pass va_list pointers on this architecture?
#endif
#else
		f1(i, ap2);
#endif
	}
}

void fx(int dummy, ...)
{
	va_list ap;

	va_start(ap, dummy);
	f2(ap);
	va_end(ap);
}

int main(void)
{
	fx(0, 5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5,
			5, 0.5, 5, 0.5, 5, 0.5, 5, 0.5);
	return 0;
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]