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
