Hi David,

On 22/07/14 02:46, David Wohlferd wrote:
I have been looking at asm_fprintf in final.c, and I think there's a
design flaw.  But since the change affects ARM and since I have no
access to an ARM system, I need a second opinion.

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.  Pretty straight-forward.

However, it isn't enough to add these two items to the case statement in
asm_fprintf.  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 on non-ARM platforms since there
are no case statements in asm_fprintf to support them.

Compiling the code in asm_fprintf-1.c (see the patch) with this patch
correctly reports "unknown conversion type character" for both 'r' and
'@' in  x86_64-pc-cygwin.  It would be helpful if someone could confirm
that it still compiles without error under ARM after applying this
patch.  I'm reluctant to post this to gcc-patches when it has never been
run.

I've tested the asm_fprintf-1.c test on an arm-none-eabi cross compiler with this patch which I think should be enough for this patch's purposes, I've confirmed that the assembly generated contains the:
.ascii  "%@%r\000"

string so I'd suggest you go ahead and post it to gcc-patches.
This is, of course, not a full regression suite run and I'm not sure if there's anything else in the testsuite that would exercise this bit of code, but I can kick off a run for that later if you'd like.

Kyrill
dw


Reply via email to