nano printf + powerpc gcc

[Alexey Neyman]

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