On 04/28/2017 12:05 PM, Martin Sebor wrote:
So the initialization could be done once per translation unit rather
than once per function -- assuming the target character set doesn't
change within a translation unit.
That seems like it ought to be easy.
It is easy. I was going to respond by saying "It already is done
that way" because I implemented it in the first patch (by checking
the mapping for '%'. But now I see I accidentally removed the code
in the update while exploring ways to optimize it some more. Sigh.
Let me put it back.
:-)
+ set corresponding to the string TARGSTR consisting of characters in
+ the execution character set. */
+
+static const char*
+target_to_host (const char *targstr)
+{
+ /* The interesting subset of source and execution characters are
+ the same so no conversion is necessary. */
+ if (target_to_host_charmap['\0'] == 1)
+ return targstr;
+
+ /* Convert the initial substring of TARGSTR to the corresponding
+ characters in the host set, appending "..." if TARGSTR is too
+ long to fit. Using the static buffer assumes the function is
+ not called in between sequence points (which it isn't). */
+ static char hostr[32];
+ for (char *ph = hostr; ; ++targstr)
+ {
+ *ph++ = target_to_host (*targstr);
+ if (!*targstr)
+ break;
+
+ if (ph - hostr == sizeof hostr - 4)
+ {
+ *ph = '\0';
+ strcat (ph, "...");
+ break;
+ }
+ }
+
+ return hostr;
Ewww. I guess the alternative would be something like:
Expand the return value to include a indicator of whether or not the
original string was returned (common case) or if a new string was
returned and thus needs to be deallocated by the caller.
That's probably pretty unpleasant given we don't have a central place
where we call target_to_host, so the caller side would be much uglier.
Are there any downstream impacts when the string is too long to covert
other than not warning for things which were unconverted?
The function is only used when printing the text of the directive
in the warning. It doesn't prevent the warnings, it just truncates
them.
Noted. Thanks.
I don't like returning a pointer to a static buffer but given that
the scope of the function is limited to the pass it seems fairly
safe. Another alternative would be to pass in a buffer and its
size. That shouldn't complicate the caller too much. The easiest,
cleanest, and safest solution by far is to return a std::string,
but I have the impression that would be against GCC convention of
avoiding the STL.
That's what caught my eye. I don't generally like that either.
Putting the buffer into the caller is better. Thanks for doing that.
Attached is an updated patch with these two tweaks.
Martin
gcc-80523.diff
PR tree-optimization/80523 - -Wformat-overflow doesn't consider -fexec-charset
gcc/ChangeLog:
PR tree-optimization/80523
* gimple-ssa-sprintf.c (target_to_host_charmap): New global variable.
(init_target_to_host_charmap, target_to_host, target_strtol10): New
functions.
(maybe_warn, format_directive, parse_directive): Use new functions.
(pass_sprintf_length::execute): Call init_target_to_host_charmap.
gcc/testsuite/ChangeLog:
PR tree-optimization/80523
* gcc.dg/tree-ssa/builtin-sprintf-warn-18.c: New test.
OK.
jeff
