Remove arm-specific formats from asm_fprintf

I have a release on file with the FSF, but don't have SVN write access.

Problem description:
asm_fprintf allows platforms to add support for new format specifiers by 
using the ASM_FPRINTF_EXTENSIONS macro.  ARM uses this to add support 
for %@ and %r specifiers.

However, it isn't enough to add these two items to the case statement in 
asm_fprintf (which is what ASM_FPRINTF_EXTENSIONS does).  Over in 
c-format.c, there is compile-time checking that is done against calls to 
asm_fprintf to validate the format string.  %@ and %r have been added to 
this checking (see asm_fprintf_char_table), but NOT in a 
platform-specific way.

This means that using %r or %@ will successfully pass the format 
checking on all platforms, but will ICE if used on non-ARM platforms 
since without ASM_FPRINTF_EXTENSIONS, there are no case statements in 
asm_fprintf to support them.

ChangeLog:
2014-07-23  David Wohlferd <dw@LimeGreenSocks.com>

         * doc/c-family/c-format.c: Add support for target macro
           ASM_FPRINTF_TABLE, remove arm-specific formats.
         * gcc/config/arm/arm.h: Use ASM_FPRINTF_TABLE for %@ and %r.
         * gcc/doc/tm.texi: Document new target macro.
         * gcc/doc/tm.texi.in: Ditto.
         * gcc/testsuite/gcc.dg/format/asm_fprintf-1.c: Make tests
           for %@ and %r arm-specific.

dw

On Wed, 23 Jul 2014, David Wohlferd wrote:

> 2014-07-23  David Wohlferd <dw@LimeGreenSocks.com>
> 
>         * doc/c-family/c-format.c: Add support for target macro
>           ASM_FPRINTF_TABLE, remove arm-specific formats.
>         * gcc/config/arm/arm.h: Use ASM_FPRINTF_TABLE for %@ and %r.
>         * gcc/doc/tm.texi: Document new target macro.
>         * gcc/doc/tm.texi.in: Ditto.
>         * gcc/testsuite/gcc.dg/format/asm_fprintf-1.c: Make tests
>           for %@ and %r arm-specific.

This patch would have the effect that only compilers for ARM target 
support the formats in asm_fprintf format checking.  That is, when 
building GCC, they would only be accepted when building GCC for ARM 
*host*.  But of course what you want is for them to be accepted when 
building GCC for ARM *target* but any host.  I.e., the support always 
needs to be compiled into c-format.c, and the decision whether to enable 
those formats needs to be taken by GCC when it is run, based on whether it 
is compiling a copy of GCC for ARM target (as opposed to GCC for some 
other target, or some other program altogether).

Existing configuration based on the GCC being compiled is in 
init_dynamic_asm_fprintf_info and other such functions.  I suppose you 
could have a __gcc_asm_fprintf_arm_formats__ identifier declared in some 
GCC header when building a compiler for ARM target (I'm supposing these 
formats are unlikely to be used in files from which tm.h includes could be 
removed any time soon).

Sorry for the late response.  I spent some time trying to understand 
your concerns here, but I'm not sure I got what you are saying:

1) Did you assume this patch would somehow disable these formats during 
cross builds, preventing the i386->arm xgcc from correctly using them?  
I don't believe that's true.  At worst this patch will cause additional 
compile warnings.

2) Or is it the additional compile warnings this patch introduces that 
are the problem? It seems to me that warnings SHOULD be generated when 
encountering platform-specific features from another platform.

Do we really want to try to teach every platform about the 
implementation details of all other platforms?  While we could try, I'm 
not convinced it's even possible.  At best, a build could verify the 
formats that were active (for the all the platforms supported by gnu) at 
the time it got built.  But given new formats, new platforms, etc 
there's no way to know what's in the code it's trying to compile now.

It seems like a lot of complexity, and an ugly co-mingling of platforms, 
all to prevent a few completely understandable warnings that only occur 
when building the cross compiler (ie not when actually running the cross 
compiler, doing bootstrap builds, etc).

That said, you have a better understanding of the design and vision of 
gcc than I do.  But if the warnings are your concern, perhaps I can 
propose other alternatives for dealing with them?

Or have I completely missed your point?

dw

On 7/28/2014 3:21 PM, Joseph S. Myers wrote:
> On Wed, 23 Jul 2014, David Wohlferd wrote:
>
>> 2014-07-23  David Wohlferd <dw@LimeGreenSocks.com>
>>
>>          * doc/c-family/c-format.c: Add support for target macro
>>            ASM_FPRINTF_TABLE, remove arm-specific formats.
>>          * gcc/config/arm/arm.h: Use ASM_FPRINTF_TABLE for %@ and %r.
>>          * gcc/doc/tm.texi: Document new target macro.
>>          * gcc/doc/tm.texi.in: Ditto.
>>          * gcc/testsuite/gcc.dg/format/asm_fprintf-1.c: Make tests
>>            for %@ and %r arm-specific.
> This patch would have the effect that only compilers for ARM target
> support the formats in asm_fprintf format checking.  That is, when
> building GCC, they would only be accepted when building GCC for ARM
> *host*.  But of course what you want is for them to be accepted when
> building GCC for ARM *target* but any host.  I.e., the support always
> needs to be compiled into c-format.c, and the decision whether to enable
> those formats needs to be taken by GCC when it is run, based on whether it
> is compiling a copy of GCC for ARM target (as opposed to GCC for some
> other target, or some other program altogether).
>
> Existing configuration based on the GCC being compiled is in
> init_dynamic_asm_fprintf_info and other such functions.  I suppose you
> could have a __gcc_asm_fprintf_arm_formats__ identifier declared in some
> GCC header when building a compiler for ARM target (I'm supposing these
> formats are unlikely to be used in files from which tm.h includes could be
> removed any time soon).
>

On Wed, 20 Aug 2014, David Wohlferd wrote:

> Or have I completely missed your point?

Suppose you build a copy of GCC, call it GCCA, and use it to compile a 
program P, with -Wformat enabled.  The following must hold:

* If P is not GCC, asm_fprintf formats are not accepted at all by GCCA 
when compiling P.

* If P is a copy of GCC (the same version as GCCA), say GCCB, configured 
for ARM target, then the formats that are used with asm_fprintf in such a 
copy of GCC must be accepted.  It is necessary that builds for all (host, 
target) combinations are clean, as long as the version of GCC used for the 
build is the same as the version of GCC being built, so that cross builds 
with --enable-werror-always can be used detect build problems that would 
show up in a native bootstrap.  If the build for some hosts is not clean, 
this breaks the use of --enable-werror-always for continuous integration.

* If P is a copy of GCC (the same version as GCCA), say GCCB, configured 
for some other target, then the formats that are used with asm_fprintf in 
such a copy of GCC must be accepted - but whether the ARM-GCC-specific 
formats are accepted doesn't matter *as long as that doesn't depend on how 
GCCA was configured*.  You can have GCC always accept them (as at 
present).  Or you can arrange things so that GCCA detects whether P is 
GCCB (ARM target) or GCCB (some other target) and so controls the set of 
supported asm_fprintf formats accordingly.  But having the accepted set 
depend on how GCCA was configured would adversely affect the usefulness of 
--enable-werror-always to detect build failures that would occur in a 
bootstrap without breaking in other cases.

What's not acceptable and represents a fundamental confusion of the 
different platforms inherently involved in a compiler is what the patch 
would introduce, which is the set of formats accepted depending on the 
target of GCCA (= host of GCCB); any such platform-dependence must be on 
the target of GCCB, not the host of GCCB.  That means this cannot be 
determined based on how GCCA is configured; instead, GCCA must obtain the 
information in some way at runtime from the source code of P, just as P 
can already communicate back to GCCA the argument types accepted by 
various of the GCC-specific formats.