nano printf + powerpc gcc

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

nano printf + powerpc gcc

Alexander Fedotov
Hi

I'm experienced a strange printf() for float variables behaviour with
Nano version while regular is fine. It just prins out a garbage (even
with enabled -u _printf_float). After some digging I've managed to
track down issue until this line:

n = _printf_float (data, &prt_data, fp, pfunc, &ap);


Moreover there are number of warnings also in build log:

src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
function '_svfprintf_r':
../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
warning: passing argument 5 of '_printf_float' from incompatible
pointer type
        n = _printf_float (data, &prt_data, fp, pfunc, &ap);
                                                                       ^
In file included from
../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
note: expected 'struct __va_list_tag (*)[1]' but argument is of type
'struct __va_list_tag **'
 _printf_float (struct _reent *data,
 ^
../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
warning: passing argument 5 of '_printf_i' from incompatible pointer
type
  n = _printf_i (data, &prt_data, fp, pfunc, &ap);
                                                            ^
In file included from
../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
note: expected 'struct __va_list_tag (*)[1]' but argument is of type
'struct __va_list_tag **'
 _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,


It looks like nano written without taking in mind such targets. Here I
found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
that's working.

I'm attaching a patch.

Alex

0001-fix-incompatible-pointer-type-for-va_list-in-nano-ve.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Corinna Vinschen
On Dec 25 16:39, Alexander Fedotov wrote:

> Hi
>
> I'm experienced a strange printf() for float variables behaviour with
> Nano version while regular is fine. It just prins out a garbage (even
> with enabled -u _printf_float). After some digging I've managed to
> track down issue until this line:
>
> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>
>
> Moreover there are number of warnings also in build log:
>
> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
> function '_svfprintf_r':
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
> warning: passing argument 5 of '_printf_float' from incompatible
> pointer type
>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>                                                                        ^
> In file included from
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
> 'struct __va_list_tag **'
>  _printf_float (struct _reent *data,
>  ^
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
> warning: passing argument 5 of '_printf_i' from incompatible pointer
> type
>   n = _printf_i (data, &prt_data, fp, pfunc, &ap);
>                                                             ^
> In file included from
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
> 'struct __va_list_tag **'
>  _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,
>
>
> It looks like nano written without taking in mind such targets. Here I
> found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
> that's working.
>
> I'm attaching a patch.
>
> Alex
Pushed.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer
Red Hat

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Andre Vieira (lists)
Hi Alexander,

This patch is causing our arm-none-eabi-gcc newlib-nano builds to fail.

It seems the _Generic isn't picking up the right type when ap is of type
va_list. We get the following error:
newlib/libc/stdio/nano-vfscanf.c:430:2: error: cannot convert to a
pointer type
ret = _scanf_chars (rptr, &scan_data, fp, va_ptr(ap));
^~~
newlib/libc/stdio/nano-vfscanf.c:432:2: error: cannot convert to a
pointer type
ret = _scanf_i (rptr, &scan_data, fp, va_ptr(ap));
^~~
newlib/libc/stdio/nano-vfscanf.c:435:2: error: cannot convert to a
pointer type
ret = _scanf_float (rptr, &scan_data, fp, va_ptr(ap));


A reduced test-case, see:

#include <stdio.h>
extern void bar(va_list *ap);
void foo(va_list ap) {
    bar(_Generic(&(ap), va_list *: &(ap), default: (va_list *)(ap)));
}

Note that va_list for our toolchain is defined as gcc's __builtin_va_list.

I have tried your other definitions of va_ptr too and none of them seem
to work. How do you suggest we proceed?

Cheers,
Andre



On 08/01/18 10:07, Corinna Vinschen wrote:

> On Dec 25 16:39, Alexander Fedotov wrote:
>> Hi
>>
>> I'm experienced a strange printf() for float variables behaviour with
>> Nano version while regular is fine. It just prins out a garbage (even
>> with enabled -u _printf_float). After some digging I've managed to
>> track down issue until this line:
>>
>> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>
>>
>> Moreover there are number of warnings also in build log:
>>
>> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
>> function '_svfprintf_r':
>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
>> warning: passing argument 5 of '_printf_float' from incompatible
>> pointer type
>>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>                                                                        ^
>> In file included from
>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>> 'struct __va_list_tag **'
>>  _printf_float (struct _reent *data,
>>  ^
>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
>> warning: passing argument 5 of '_printf_i' from incompatible pointer
>> type
>>   n = _printf_i (data, &prt_data, fp, pfunc, &ap);
>>                                                             ^
>> In file included from
>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>> 'struct __va_list_tag **'
>>  _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,
>>
>>
>> It looks like nano written without taking in mind such targets. Here I
>> found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
>> that's working.
>>
>> I'm attaching a patch.
>>
>> Alex
>
> Pushed.
>
>
> Thanks,
> Corinna
>

Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Alexander Fedotov
Hello Andre
Yes indeed this fix unfortunately broke an ARM port. I have no idea
why this builtins doesn't work on gcc for arm. Best way I believe
right now is to revert this commit.

Alex

On Tue, Jan 23, 2018 at 5:04 PM, Andre Vieira (lists)
<[hidden email]> wrote:

> Hi Alexander,
>
> This patch is causing our arm-none-eabi-gcc newlib-nano builds to fail.
>
> It seems the _Generic isn't picking up the right type when ap is of type
> va_list. We get the following error:
> newlib/libc/stdio/nano-vfscanf.c:430:2: error: cannot convert to a
> pointer type
> ret = _scanf_chars (rptr, &scan_data, fp, va_ptr(ap));
> ^~~
> newlib/libc/stdio/nano-vfscanf.c:432:2: error: cannot convert to a
> pointer type
> ret = _scanf_i (rptr, &scan_data, fp, va_ptr(ap));
> ^~~
> newlib/libc/stdio/nano-vfscanf.c:435:2: error: cannot convert to a
> pointer type
> ret = _scanf_float (rptr, &scan_data, fp, va_ptr(ap));
>
>
> A reduced test-case, see:
>
> #include <stdio.h>
> extern void bar(va_list *ap);
> void foo(va_list ap) {
>     bar(_Generic(&(ap), va_list *: &(ap), default: (va_list *)(ap)));
> }
>
> Note that va_list for our toolchain is defined as gcc's __builtin_va_list.
>
> I have tried your other definitions of va_ptr too and none of them seem
> to work. How do you suggest we proceed?
>
> Cheers,
> Andre
>
>
>
> On 08/01/18 10:07, Corinna Vinschen wrote:
>> On Dec 25 16:39, Alexander Fedotov wrote:
>>> Hi
>>>
>>> I'm experienced a strange printf() for float variables behaviour with
>>> Nano version while regular is fine. It just prins out a garbage (even
>>> with enabled -u _printf_float). After some digging I've managed to
>>> track down issue until this line:
>>>
>>> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>>
>>>
>>> Moreover there are number of warnings also in build log:
>>>
>>> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
>>> function '_svfprintf_r':
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
>>> warning: passing argument 5 of '_printf_float' from incompatible
>>> pointer type
>>>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>>                                                                        ^
>>> In file included from
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>>> 'struct __va_list_tag **'
>>>  _printf_float (struct _reent *data,
>>>  ^
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
>>> warning: passing argument 5 of '_printf_i' from incompatible pointer
>>> type
>>>   n = _printf_i (data, &prt_data, fp, pfunc, &ap);
>>>                                                             ^
>>> In file included from
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>>> 'struct __va_list_tag **'
>>>  _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,
>>>
>>>
>>> It looks like nano written without taking in mind such targets. Here I
>>> found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
>>> that's working.
>>>
>>> I'm attaching a patch.
>>>
>>> Alex
>>
>> Pushed.
>>
>>
>> Thanks,
>> Corinna
>>
>



--
Best regards,
AF
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Andre Vieira (lists)
In reply to this post by Andre Vieira (lists)
Hi,

I looked a bit further into this and I can't reproduce this behavior for
the i386 backend. So I had a look through GCC's parser implementation
for _Generic and it seems GCC evaluates the expressions in each matching
case, that is to say it will also parse the default case. In arm, the
type of va_list is a struct, since it is invalid to cast structs to
pointers, the c-parser will error out when it parses this cast in the
default case. For an i386 gcc, and I'm guessing your case is the same,
the type of va_list can be cast to a pointer type, so it parses it
without errors.

I don't know if parsing the expression of default if another case has
matched is intended behavior. I will investigate this further and if I
can convince myself this is wrong behavior I'll chase it with upstream gcc.

On the other hand, I believe we can leave out the cast here, since the
prototype for the function taking this as a parameter should be
explicitly trying to cast it if that gets picked anyway no?

I tried it with a pet example and it compiles if I leave out the cast
for the 'default' case on all three implementations of va_ptr.


Cheers,
Andre

On 23/01/18 14:04, Andre Vieira (lists) wrote:

> Hi Alexander,
>
> This patch is causing our arm-none-eabi-gcc newlib-nano builds to fail.
>
> It seems the _Generic isn't picking up the right type when ap is of type
> va_list. We get the following error:
> newlib/libc/stdio/nano-vfscanf.c:430:2: error: cannot convert to a
> pointer type
> ret = _scanf_chars (rptr, &scan_data, fp, va_ptr(ap));
> ^~~
> newlib/libc/stdio/nano-vfscanf.c:432:2: error: cannot convert to a
> pointer type
> ret = _scanf_i (rptr, &scan_data, fp, va_ptr(ap));
> ^~~
> newlib/libc/stdio/nano-vfscanf.c:435:2: error: cannot convert to a
> pointer type
> ret = _scanf_float (rptr, &scan_data, fp, va_ptr(ap));
>
>
> A reduced test-case, see:
>
> #include <stdio.h>
> extern void bar(va_list *ap);
> void foo(va_list ap) {
>     bar(_Generic(&(ap), va_list *: &(ap), default: (va_list *)(ap)));
> }
>
> Note that va_list for our toolchain is defined as gcc's __builtin_va_list.
>
> I have tried your other definitions of va_ptr too and none of them seem
> to work. How do you suggest we proceed?
>
> Cheers,
> Andre
>
>
>
> On 08/01/18 10:07, Corinna Vinschen wrote:
>> On Dec 25 16:39, Alexander Fedotov wrote:
>>> Hi
>>>
>>> I'm experienced a strange printf() for float variables behaviour with
>>> Nano version while regular is fine. It just prins out a garbage (even
>>> with enabled -u _printf_float). After some digging I've managed to
>>> track down issue until this line:
>>>
>>> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>>
>>>
>>> Moreover there are number of warnings also in build log:
>>>
>>> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
>>> function '_svfprintf_r':
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
>>> warning: passing argument 5 of '_printf_float' from incompatible
>>> pointer type
>>>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>>                                                                        ^
>>> In file included from
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>>> 'struct __va_list_tag **'
>>>  _printf_float (struct _reent *data,
>>>  ^
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
>>> warning: passing argument 5 of '_printf_i' from incompatible pointer
>>> type
>>>   n = _printf_i (data, &prt_data, fp, pfunc, &ap);
>>>                                                             ^
>>> In file included from
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>>> 'struct __va_list_tag **'
>>>  _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,
>>>
>>>
>>> It looks like nano written without taking in mind such targets. Here I
>>> found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
>>> that's working.
>>>
>>> I'm attaching a patch.
>>>
>>> Alex
>>
>> Pushed.
>>
>>
>> Thanks,
>> Corinna
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Andre Vieira (lists)
In reply to this post by Alexander Fedotov
Sorry for my other email not replying to this, when I started writing it
I hadn't seen your email yet.

Anyhow, I suggested a change to your patch there at the end, if you
could give that a try to see if it works for you too, then we could (at
least for now) maybe get away with it.

Cheers,
Andre

On 24/01/18 16:02, Alexander Fedotov wrote:

> Hello Andre
> Yes indeed this fix unfortunately broke an ARM port. I have no idea
> why this builtins doesn't work on gcc for arm. Best way I believe
> right now is to revert this commit.
>
> Alex
>
> On Tue, Jan 23, 2018 at 5:04 PM, Andre Vieira (lists)
> <[hidden email]> wrote:
>> Hi Alexander,
>>
>> This patch is causing our arm-none-eabi-gcc newlib-nano builds to fail.
>>
>> It seems the _Generic isn't picking up the right type when ap is of type
>> va_list. We get the following error:
>> newlib/libc/stdio/nano-vfscanf.c:430:2: error: cannot convert to a
>> pointer type
>> ret = _scanf_chars (rptr, &scan_data, fp, va_ptr(ap));
>> ^~~
>> newlib/libc/stdio/nano-vfscanf.c:432:2: error: cannot convert to a
>> pointer type
>> ret = _scanf_i (rptr, &scan_data, fp, va_ptr(ap));
>> ^~~
>> newlib/libc/stdio/nano-vfscanf.c:435:2: error: cannot convert to a
>> pointer type
>> ret = _scanf_float (rptr, &scan_data, fp, va_ptr(ap));
>>
>>
>> A reduced test-case, see:
>>
>> #include <stdio.h>
>> extern void bar(va_list *ap);
>> void foo(va_list ap) {
>>     bar(_Generic(&(ap), va_list *: &(ap), default: (va_list *)(ap)));
>> }
>>
>> Note that va_list for our toolchain is defined as gcc's __builtin_va_list.
>>
>> I have tried your other definitions of va_ptr too and none of them seem
>> to work. How do you suggest we proceed?
>>
>> Cheers,
>> Andre
>>
>>
>>
>> On 08/01/18 10:07, Corinna Vinschen wrote:
>>> On Dec 25 16:39, Alexander Fedotov wrote:
>>>> Hi
>>>>
>>>> I'm experienced a strange printf() for float variables behaviour with
>>>> Nano version while regular is fine. It just prins out a garbage (even
>>>> with enabled -u _printf_float). After some digging I've managed to
>>>> track down issue until this line:
>>>>
>>>> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>>>
>>>>
>>>> Moreover there are number of warnings also in build log:
>>>>
>>>> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
>>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
>>>> function '_svfprintf_r':
>>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
>>>> warning: passing argument 5 of '_printf_float' from incompatible
>>>> pointer type
>>>>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>>>>                                                                        ^
>>>> In file included from
>>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
>>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>>>> 'struct __va_list_tag **'
>>>>  _printf_float (struct _reent *data,
>>>>  ^
>>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
>>>> warning: passing argument 5 of '_printf_i' from incompatible pointer
>>>> type
>>>>   n = _printf_i (data, &prt_data, fp, pfunc, &ap);
>>>>                                                             ^
>>>> In file included from
>>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
>>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>>>> 'struct __va_list_tag **'
>>>>  _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,
>>>>
>>>>
>>>> It looks like nano written without taking in mind such targets. Here I
>>>> found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
>>>> that's working.
>>>>
>>>> I'm attaching a patch.
>>>>
>>>> Alex
>>>
>>> Pushed.
>>>
>>>
>>> Thanks,
>>> Corinna
>>>
>>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Alexander Fedotov
Sure, I can check fix on my side.

Other way is to fix original issue from where this workaround comes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557. But seems nobody cares
it.

Alex

On Wed, 24 Jan 2018 at 20:46, Andre Vieira (lists) <
[hidden email]> wrote:

> Sorry for my other email not replying to this, when I started writing it
> I hadn't seen your email yet.
>
> Anyhow, I suggested a change to your patch there at the end, if you
> could give that a try to see if it works for you too, then we could (at
> least for now) maybe get away with it.
>
> Cheers,
> Andre
>
> On 24/01/18 16:02, Alexander Fedotov wrote:
> > Hello Andre
> > Yes indeed this fix unfortunately broke an ARM port. I have no idea
> > why this builtins doesn't work on gcc for arm. Best way I believe
> > right now is to revert this commit.
> >
> > Alex
> >
> > On Tue, Jan 23, 2018 at 5:04 PM, Andre Vieira (lists)
> > <[hidden email]> wrote:
> >> Hi Alexander,
> >>
> >> This patch is causing our arm-none-eabi-gcc newlib-nano builds to fail.
> >>
> >> It seems the _Generic isn't picking up the right type when ap is of type
> >> va_list. We get the following error:
> >> newlib/libc/stdio/nano-vfscanf.c:430:2: error: cannot convert to a
> >> pointer type
> >> ret = _scanf_chars (rptr, &scan_data, fp, va_ptr(ap));
> >> ^~~
> >> newlib/libc/stdio/nano-vfscanf.c:432:2: error: cannot convert to a
> >> pointer type
> >> ret = _scanf_i (rptr, &scan_data, fp, va_ptr(ap));
> >> ^~~
> >> newlib/libc/stdio/nano-vfscanf.c:435:2: error: cannot convert to a
> >> pointer type
> >> ret = _scanf_float (rptr, &scan_data, fp, va_ptr(ap));
> >>
> >>
> >> A reduced test-case, see:
> >>
> >> #include <stdio.h>
> >> extern void bar(va_list *ap);
> >> void foo(va_list ap) {
> >>     bar(_Generic(&(ap), va_list *: &(ap), default: (va_list *)(ap)));
> >> }
> >>
> >> Note that va_list for our toolchain is defined as gcc's
> __builtin_va_list.
> >>
> >> I have tried your other definitions of va_ptr too and none of them seem
> >> to work. How do you suggest we proceed?
> >>
> >> Cheers,
> >> Andre
> >>
> >>
> >>
> >> On 08/01/18 10:07, Corinna Vinschen wrote:
> >>> On Dec 25 16:39, Alexander Fedotov wrote:
> >>>> Hi
> >>>>
> >>>> I'm experienced a strange printf() for float variables behaviour with
> >>>> Nano version while regular is fine. It just prins out a garbage (even
> >>>> with enabled -u _printf_float). After some digging I've managed to
> >>>> track down issue until this line:
> >>>>
> >>>> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
> >>>>
> >>>>
> >>>> Moreover there are number of warnings also in build log:
> >>>>
> >>>> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
> >>>> function '_svfprintf_r':
> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
> >>>> warning: passing argument 5 of '_printf_float' from incompatible
> >>>> pointer type
> >>>>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
> >>>>
>   ^
> >>>> In file included from
> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
> >>>>
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
> >>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
> >>>> 'struct __va_list_tag **'
> >>>>  _printf_float (struct _reent *data,
> >>>>  ^
> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
> >>>> warning: passing argument 5 of '_printf_i' from incompatible pointer
> >>>> type
> >>>>   n = _printf_i (data, &prt_data, fp, pfunc, &ap);
> >>>>                                                             ^
> >>>> In file included from
> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
> >>>>
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
> >>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
> >>>> 'struct __va_list_tag **'
> >>>>  _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,
> >>>>
> >>>>
> >>>> It looks like nano written without taking in mind such targets. Here I
> >>>> found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
> >>>> that's working.
> >>>>
> >>>> I'm attaching a patch.
> >>>>
> >>>> Alex
> >>>
> >>> Pushed.
> >>>
> >>>
> >>> Thanks,
> >>> Corinna
> >>>
> >>
> >
> >
> >
>
> --
Best regards,
AF
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Craig Howland
In reply to this post by Andre Vieira (lists)
On 01/24/2018 11:46 AM, Andre Vieira (lists) wrote:

> Hi,
>
> I looked a bit further into this and I can't reproduce this behavior for
> the i386 backend. So I had a look through GCC's parser implementation
> for _Generic and it seems GCC evaluates the expressions in each matching
> case, that is to say it will also parse the default case. In arm, the
> type of va_list is a struct, since it is invalid to cast structs to
> pointers, the c-parser will error out when it parses this cast in the
> default case. For an i386 gcc, and I'm guessing your case is the same,
> the type of va_list can be cast to a pointer type, so it parses it
> without errors.
>
> I don't know if parsing the expression of default if another case has
> matched is intended behavior. I will investigate this further and if I
> can convince myself this is wrong behavior I'll chase it with upstream gcc.
>
The final draft of the C11 standard, which I assume was not changed in the final
release, does say that only the chosen expression is evaluated.  (Which is as it
must be.  If it is choosing function names, for example, it can hardly evaluate
a non-chosen option, as that means calling the function.  The example they give
is actually choosing a function.)  A cast is an expression, so it appears that
you do need to chase it with upstream GCC.  It obviously has to parse each
selection, but it must only evaluate one.
Craig

Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Brian Inglis
On 2018-01-24 11:25, Craig Howland wrote:

> On 01/24/2018 11:46 AM, Andre Vieira (lists) wrote:
>> I looked a bit further into this and I can't reproduce this behavior for
>> the i386 backend. So I had a look through GCC's parser implementation for
>> _Generic and it seems GCC evaluates the expressions in each matching case,
>> that is to say it will also parse the default case. In arm, the type of
>> va_list is a struct, since it is invalid to cast structs to pointers, the
>> c-parser will error out when it parses this cast in the default case. For
>> an i386 gcc, and I'm guessing your case is the same, the type of va_list
>> can be cast to a pointer type, so it parses it without errors.
>>
>> I don't know if parsing the expression of default if another case has
>> matched is intended behavior. I will investigate this further and if I can
>> convince myself this is wrong behavior I'll chase it with upstream gcc.
>>
> The final draft of the C11 standard, which I assume was not changed in the
> final release, does say that only the chosen expression is evaluated.  (Which
> is as it must be.  If it is choosing function names, for example, it can
> hardly evaluate a non-chosen option, as that means calling the function.  The
> example they give is actually choosing a function.)  A cast is an expression,
> so it appears that you do need to chase it with upstream GCC.  It obviously
> has to parse each selection, but it must only evaluate one.
FYI for _Generic definition and issues see:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf        [CD]
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1650.htm        [DR original]
http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_423.htm        [DR discussion]
https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01054.html        [GCC response]
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1930.htm        [followup]

The official standard agrees with you at the end of #3, but see note (91 below;
selectively quoting from ISO/IEC 9899:2011[2012] thru TC3:
"
6 A generic selection is a primary expression. Its type and value depend on
the selected generic association, as detailed in the following subclause.
Forward references: declarations (6.7).

6.5.1.1 Generic selection

Syntax

1 generic-selection:
        _Generic ( assignment-expression , generic-assoc-list )
generic-assoc-list:
        generic-association
        generic-assoc-list , generic-association
generic-association:
        type-name : assignment-expression
        default : assignment-expression

91) Thus, an undeclared identifier is a violation of the syntax.

Constraints

2 A generic selection shall have no more than one default generic association.
The type name in a generic association shall specify a complete object type
other than a variably modified type. No two generic associations in the same
generic selection shall specify compatible types. The controlling expression of
a generic selection shall have type compatible with at most one of the types
named in its generic association list. If a generic selection has no default
generic association, its controlling expression shall have type compatible with
exactly one of the types named in its generic association list.

Semantics

3 The controlling expression of a generic selection is not evaluated. If a
generic selection has a generic association with a type name that is compatible
with the type of the controlling expression, then the result expression of the
generic selection is the expression in that generic association. Otherwise, the
result expression of the generic selection is the expression in the default
generic association. None of the expressions from any other generic association
of the generic selection is evaluated.

4 The type and value of a generic selection are identical to those of its
result expression. It is an lvalue, a function designator, or a void expression
if its result expression is, respectively, an lvalue, a function designator, or
a void expression.

5 EXAMPLE The cbrt type-generic macro could be implemented as follows:
#define cbrt(X) _Generic((X), \
                long double: cbrtl, \
                default: cbrt, \
                float: cbrtf \
        )(X)
"
--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Joseph Myers
In reply to this post by Craig Howland
On Wed, 24 Jan 2018, Craig Howland wrote:

> The final draft of the C11 standard, which I assume was not changed in the
> final release, does say that only the chosen expression is evaluated.  (Which

Evaluated means executed, with all the associated side-effects, at runtime
if the _Generic expression is executed at runtime.

All expressions - including the assignment-expressions that appear in the
_Generic syntax - must always meet the syntactic and semantic constraints
(for example, the constraints on valid casts).  This applies to
unevaluated expressions inside _Generic just as it does to unevaluated
expressions inside sizeof.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Alexander Fedotov
In reply to this post by Alexander Fedotov
Well if this requires a much of effort I would rather propose to to
wrap up these ifdef's by checking architecture.
Like #if defined(_PPC_) || defined (__x86_64__) || defined (__amd64__)
|| defined (__i386__) etc


On Wed, Jan 24, 2018 at 8:49 PM, Alexander Fedotov <[hidden email]> wrote:

> Sure, I can check fix on my side.
>
> Other way is to fix original issue from where this workaround comes
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557. But seems nobody cares
> it.
>
> Alex
>
> On Wed, 24 Jan 2018 at 20:46, Andre Vieira (lists)
> <[hidden email]> wrote:
>>
>> Sorry for my other email not replying to this, when I started writing it
>> I hadn't seen your email yet.
>>
>> Anyhow, I suggested a change to your patch there at the end, if you
>> could give that a try to see if it works for you too, then we could (at
>> least for now) maybe get away with it.
>>
>> Cheers,
>> Andre
>>
>> On 24/01/18 16:02, Alexander Fedotov wrote:
>> > Hello Andre
>> > Yes indeed this fix unfortunately broke an ARM port. I have no idea
>> > why this builtins doesn't work on gcc for arm. Best way I believe
>> > right now is to revert this commit.
>> >
>> > Alex
>> >
>> > On Tue, Jan 23, 2018 at 5:04 PM, Andre Vieira (lists)
>> > <[hidden email]> wrote:
>> >> Hi Alexander,
>> >>
>> >> This patch is causing our arm-none-eabi-gcc newlib-nano builds to fail.
>> >>
>> >> It seems the _Generic isn't picking up the right type when ap is of
>> >> type
>> >> va_list. We get the following error:
>> >> newlib/libc/stdio/nano-vfscanf.c:430:2: error: cannot convert to a
>> >> pointer type
>> >> ret = _scanf_chars (rptr, &scan_data, fp, va_ptr(ap));
>> >> ^~~
>> >> newlib/libc/stdio/nano-vfscanf.c:432:2: error: cannot convert to a
>> >> pointer type
>> >> ret = _scanf_i (rptr, &scan_data, fp, va_ptr(ap));
>> >> ^~~
>> >> newlib/libc/stdio/nano-vfscanf.c:435:2: error: cannot convert to a
>> >> pointer type
>> >> ret = _scanf_float (rptr, &scan_data, fp, va_ptr(ap));
>> >>
>> >>
>> >> A reduced test-case, see:
>> >>
>> >> #include <stdio.h>
>> >> extern void bar(va_list *ap);
>> >> void foo(va_list ap) {
>> >>     bar(_Generic(&(ap), va_list *: &(ap), default: (va_list *)(ap)));
>> >> }
>> >>
>> >> Note that va_list for our toolchain is defined as gcc's
>> >> __builtin_va_list.
>> >>
>> >> I have tried your other definitions of va_ptr too and none of them seem
>> >> to work. How do you suggest we proceed?
>> >>
>> >> Cheers,
>> >> Andre
>> >>
>> >>
>> >>
>> >> On 08/01/18 10:07, Corinna Vinschen wrote:
>> >>> On Dec 25 16:39, Alexander Fedotov wrote:
>> >>>> Hi
>> >>>>
>> >>>> I'm experienced a strange printf() for float variables behaviour with
>> >>>> Nano version while regular is fine. It just prins out a garbage (even
>> >>>> with enabled -u _printf_float). After some digging I've managed to
>> >>>> track down issue until this line:
>> >>>>
>> >>>> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>> >>>>
>> >>>>
>> >>>> Moreover there are number of warnings also in build log:
>> >>>>
>> >>>> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o
>> >>>> lib_a-nano-svfprintf.o
>> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
>> >>>> function '_svfprintf_r':
>> >>>>
>> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
>> >>>> warning: passing argument 5 of '_printf_float' from incompatible
>> >>>> pointer type
>> >>>>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>> >>>>
>> >>>> ^
>> >>>> In file included from
>> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>> >>>>
>> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:228:1:
>> >>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>> >>>> 'struct __va_list_tag **'
>> >>>>  _printf_float (struct _reent *data,
>> >>>>  ^
>> >>>>
>> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:650:45:
>> >>>> warning: passing argument 5 of '_printf_i' from incompatible pointer
>> >>>> type
>> >>>>   n = _printf_i (data, &prt_data, fp, pfunc, &ap);
>> >>>>                                                             ^
>> >>>> In file included from
>> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:169:0:
>> >>>>
>> >>>> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf_local.h:221:1:
>> >>>> note: expected 'struct __va_list_tag (*)[1]' but argument is of type
>> >>>> 'struct __va_list_tag **'
>> >>>>  _printf_i (struct _reent *data, struct _prt_data_t *pdata, FILE *fp,
>> >>>>
>> >>>>
>> >>>> It looks like nano written without taking in mind such targets. Here
>> >>>> I
>> >>>> found workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14557
>> >>>> that's working.
>> >>>>
>> >>>> I'm attaching a patch.
>> >>>>
>> >>>> Alex
>> >>>
>> >>> Pushed.
>> >>>
>> >>>
>> >>> Thanks,
>> >>> Corinna
>> >>>
>> >>
>> >
>> >
>> >
>>
> --
> Best regards,
> AF



--
Best regards,
AF
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Eric Blake-3
In reply to this post by Alexander Fedotov
On 12/25/2017 07:39 AM, Alexander Fedotov wrote:

> Hi
>
> I'm experienced a strange printf() for float variables behaviour with
> Nano version while regular is fine. It just prins out a garbage (even
> with enabled -u _printf_float). After some digging I've managed to
> track down issue until this line:
>
> n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>
>
> Moreover there are number of warnings also in build log:
>
> src_newlib/newlib/libc/stdio/nano-vfprintf.c -o lib_a-nano-svfprintf.o
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c: In
> function '_svfprintf_r':
> ../../../../../../src_newlib/newlib/libc/stdio/nano-vfprintf.c:645:55:
> warning: passing argument 5 of '_printf_float' from incompatible
> pointer type
>         n = _printf_float (data, &prt_data, fp, pfunc, &ap);
>                                                                        ^
One possibility for a workaround that I've seen used in other projects
is to do a va_copy() and pass a pointer to the copy rather than the
original ap passed in as a parameter.  Here's a thread from the qemu list:

https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00605.html
https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg00171.html

and where the va_copy() code is used in practice:

https://git.qemu.org/?p=qemu.git;a=blob;f=tests/libqtest.c;h=0ec8af292;hb=2077fef9#l472

in order to call a function taking va_list *.


 472 void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
 473 {
 474     va_list ap_copy;
...
 484
 485     /* Going through qobject ensures we escape strings properly.
 486      * This seemingly unnecessary copy is required in case va_list
 487      * is an array type.
 488      */
 489     va_copy(ap_copy, ap);
 490     qobj = qobject_from_jsonv(fmt, &ap_copy, &error_abort);
 491     va_end(ap_copy);
 492

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


signature.asc (632 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Alexander Fedotov
In reply to this post by Alexander Fedotov
Seems that Eric's proposal to use va_copy() is more straightforward
approach right now to bring back compatibility.
How do you think ? Is there any chances to fix expressions evaluation
it in GCC ?
Alex

On Thu, Jan 25, 2018 at 10:09 PM, Eric Blake <[hidden email]> wrote:

> On 01/25/2018 12:31 PM, Alexander Fedotov wrote:
>> Well if this requires a much of effort I would rather propose to to
>> wrap up these ifdef's by checking architecture.
>> Like #if defined(_PPC_) || defined (__x86_64__) || defined (__amd64__)
>> || defined (__i386__) etc
>
> See my proposal up-thread about an alternative workaround using
> va_copy() rather than _Generic(), where you would not need an #ifdef chain.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>



--
Best regards,
AF
Reply | Threaded
Open this post in threaded view
|

RE: nano printf + powerpc gcc

Jon Beniston-3
Hi,

Is this patch what is recommended? Seems to fix it for me (but only very briefly tested on my target).

Cheers,
Jon


nano-va.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Alexander Fedotov
Hi

Jon, I've tried you patch and it is working fine on PowerPC VLE. I'll
vote for this way.

Andre, could you check it on ARM ?

Alex

On Mon, Jan 29, 2018 at 2:56 PM, Jon Beniston <[hidden email]> wrote:
> Hi,
>
> Is this patch what is recommended? Seems to fix it for me (but only very briefly tested on my target).
>
> Cheers,
> Jon
>



--
Best regards,
AF
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Eric Blake-3
In reply to this post by Jon Beniston-3
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.

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


signature.asc (632 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Andre Vieira (lists)
This solves the build failure on Arm. Thank you!

Regards,
Andre Vieira

On 29/01/18 15:48, 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.
>

Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Craig Howland
In reply to this post by Brian Inglis
On 01/24/2018 07:44 PM, Brian Inglis wrote:

> On 2018-01-24 11:25, Craig Howland wrote:
>> On 01/24/2018 11:46 AM, Andre Vieira (lists) wrote:
>>> I looked a bit further into this and I can't reproduce this behavior for
>>> the i386 backend. So I had a look through GCC's parser implementation for
>>> _Generic and it seems GCC evaluates the expressions in each matching case,
>>> that is to say it will also parse the default case. In arm, the type of
>>> va_list is a struct, since it is invalid to cast structs to pointers, the
>>> c-parser will error out when it parses this cast in the default case. For
>>> an i386 gcc, and I'm guessing your case is the same, the type of va_list
>>> can be cast to a pointer type, so it parses it without errors.
>>>
>>> I don't know if parsing the expression of default if another case has
>>> matched is intended behavior. I will investigate this further and if I can
>>> convince myself this is wrong behavior I'll chase it with upstream gcc.
>>>
>> The final draft of the C11 standard, which I assume was not changed in the
>> final release, does say that only the chosen expression is evaluated.  (Which
>> is as it must be.  If it is choosing function names, for example, it can
>> hardly evaluate a non-chosen option, as that means calling the function.  The
>> example they give is actually choosing a function.)  A cast is an expression,
>> so it appears that you do need to chase it with upstream GCC.  It obviously
>> has to parse each selection, but it must only evaluate one.
> FYI for _Generic definition and issues see:
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf        [CD]
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1650.htm        [DR original]
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_423.htm        [DR discussion]
> https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01054.html        [GCC response]
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1930.htm        [followup]
>
> The official standard agrees with you at the end of #3, but see note (91 below;
> selectively quoting from ISO/IEC 9899:2011[2012] thru TC3:
> "
> 6 A generic selection is a primary expression. Its type and value depend on
> the selected generic association, as detailed in the following subclause.
> Forward references: declarations (6.7).
>
> 6.5.1.1 Generic selection
>
> Syntax
>
> 1 generic-selection:
> _Generic ( assignment-expression , generic-assoc-list )
> generic-assoc-list:
> generic-association
> generic-assoc-list , generic-association
> generic-association:
> type-name : assignment-expression
> default : assignment-expression
>
> 91) Thus, an undeclared identifier is a violation of the syntax.
>
> Constraints
>
> 2 A generic selection shall have no more than one default generic association.
> The type name in a generic association shall specify a complete object type
> other than a variably modified type. No two generic associations in the same
> generic selection shall specify compatible types. The controlling expression of
> a generic selection shall have type compatible with at most one of the types
> named in its generic association list. If a generic selection has no default
> generic association, its controlling expression shall have type compatible with
> exactly one of the types named in its generic association list.
>
> Semantics
>
> 3 The controlling expression of a generic selection is not evaluated. If a
> generic selection has a generic association with a type name that is compatible
> with the type of the controlling expression, then the result expression of the
> generic selection is the expression in that generic association. Otherwise, the
> result expression of the generic selection is the expression in the default
> generic association. None of the expressions from any other generic association
> of the generic selection is evaluated.
>
> 4 The type and value of a generic selection are identical to those of its
> result expression. It is an lvalue, a function designator, or a void expression
> if its result expression is, respectively, an lvalue, a function designator, or
> a void expression.
>
> 5 EXAMPLE The cbrt type-generic macro could be implemented as follows:
> #define cbrt(X) _Generic((X), \
> long double: cbrtl, \
> default: cbrt, \
> float: cbrtf \
> )(X)
> "
       Sorry for the delay in responding to this, but it took a while to have
enough time to read the given references.
       I believe that the given references don't apply to the case under
consideration.  The primary problem being discussed in them is qualifiers and
how they relate to rvalue versus lvalue for _Generic() evaluation, but
qualifiers don't come into play for the specific point under consideration here,
namely evaluation of an association other than the one chosen.  (That is, after
an association has been selected for evaluation, then the complications in DR
423 and the followup come into play.  Whether qualifiers are applied are not are
moot if the expression is not evaluated.)  So Andre's thought about chasing GCC
with it does still seem to be appropriate.
Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Alexey Neyman
In reply to this post by Eric Blake-3
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.
>

Reply | Threaded
Open this post in threaded view
|

Re: nano printf + powerpc gcc

Eric Blake-3
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


signature.asc (632 bytes) Download Attachment
12