On 09/11/2016 08:03 PM, Martin Sebor wrote:
Attached is revision 8 of the patch (hopefully) adjusted as
requested above, and with a simple test with of the multiline
diagnostic directives suggested by David. This revision also
enables the -fprintf-return-value option by default. The libgomp
test failures I was seeing in my earlier testing must have been
caused by an older version of GMP or MPFR that I had inadvertently
use (normally I use in-tree versions downloaded and expanded there
by the download_prerequisites script but that time I forgot that
step).
Ah, my question answered. I started looking at the multiline stuff and
didn't read the last sentence WRT libgomp. Sorry for the noise in my
prior message.
David, in the way of feedback, I spent hours debugging the simple
multiline test I added. It seems that DejaGnu is exquisitely
sensitive to whitespaces in the multiline output. I appreciate
that spacing is essential to lining up the caret and the tildes
with the text of the program but tests fail even when all the
expected output is lined up right in the multiline directive but
off by a space (or tab) with respect to the actual output. That
DejaGnu output doesn't make this very clear at all makes debugging
these types of issues a pain. I wonder if there's a better to do
this.
dejagnu as a whole is a pain and trying to get the right whitespace,
regexps, escaping is an exercise in pure torture. How you and David
have managed to do any multiline tests is amazing.
Thanks
Martin
gcc-49905.diff
PR middle-end/49905 - Better sanity checking on sprintf src & dest to
produce warning for dodgy code
gcc/ChangeLog:
PR middle-end/49905
* Makefile.in (OBJS): Add gimple-ssa-sprintf.o.
* config/linux.h (TARGET_PRINTF_POINTER_FORMAT): Redefine.
* config/linux.c (gnu_libc_printf_pointer_format): New function.
* config/sol2.h (TARGET_PRINTF_POINTER_FORMAT): Same.
* config/sol2.c (solaris_printf_pointer_format): New function.
* doc/invoke.texi (-Wformat-length, -fprintf-return-value): New
options.
* doc/tm.texi.in (TARGET_PRINTF_POINTER_FORMAT): Document.
* doc/tm.texi: Regenerate.
* gimple-fold.h (get_range_strlen): New function.
(get_maxval_strlen): Declare existing function.
* gimple-fold.c (get_range_strlen): Add arguments and compute both
maximum and minimum.
(get_range_strlen): Define overload.
(get_maxval_strlen): Adjust.
* gimple-ssa-sprintf.c: New file and pass.
* passes.def (pass_sprintf_length): Add new pass.
* targhooks.h (default_printf_pointer_format): Declare new function.
(gnu_libc_printf_pointer_format): Same.
(solaris_libc_printf_pointer_format): Same.
* targhooks.c (default_printf_pointer_format): Define new function.
* tree-pass.h (make_pass_sprintf_length): Declare new function.
* print-tree.c: Increase buffer size.
gcc/c-family/ChangeLog:
PR middle-end/49905
* c.opt: Add -Wformat-length and -fprintf-return-value.
gcc/testsuite/ChangeLog:
PR middle-end/49905
* gcc.dg/builtin-stringop-chk-1.c: Adjust.
* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-warn-2.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-warn-4.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-2.c: New test.
static bool
-get_maxval_strlen (tree arg, tree *length, bitmap *visited, int type)
+get_range_strlen (tree arg, tree length[2], bitmap *visited, int type,
+ bool fuzzy)
{
tree var, val;
gimple *def_stmt;
+ /* The minimum and maximum length. The MAXLEN pointer stays unchanged
+ but MINLEN may be cleared during the execution of the function. */
+ tree *minlen = length;
+ tree* const maxlen = length + 1;
Check formatting here. I'm not sure if the formatting standards
explicitly state how to handle the "*" when there's a qualifier between
the type and the object. But please check.
+
+ if (warned && fmtres.argmin)
+ {
+ if (fmtres.argmin == fmtres.argmax)
+ inform (info.fmtloc, "directive argument %qE", fmtres.argmin);
+ else if (fmtres.bounded)
+ inform (info.fmtloc, "directive argument in the range [%E, %E]",
+ fmtres.argmin, fmtres.argmax);
+ else
+ inform (info.fmtloc,
+ "using the range [%qE, %qE] for directive argument",
+ fmtres.argmin, fmtres.argmax);
Don't you need G_ for these messages?
Can you please check the calls to fmtwarn which pass in the string as an
argument and add G_ as needed? I think the cases where the string is
computed into a variable are all handled correctly, it's jut those where
the string appears as a call argument that need double checking. I
think there's ~3 of them.
+
+/* Account for the number of bytes between BEG and END (or between
+ BEG + strlen (BEG) when END is null) in the format string in a call
+ to a formatted output function described by INFO. Reflect the count
+ in RES and issue warnings as appropriate. */
+
+static void
+add_bytes (const pass_sprintf_length::call_info &info,
+ const char *beg, const char *end, format_result *res)
+{
+ size_t len = strlen (info.fmtstr + off);
+
+#if 0
+ int caret = off - !len;
+ int beg = len ? off : 0;
+ int end = off + len - !!len;
+ int chr = info.fmtstr[off];
+
+ if (const char *env = getenv ("LOC_CARET"))
+ caret = atoi (env);
+ if (const char *env = getenv ("LOC_BEGIN"))
+ beg = atoi (env);
+ if (const char *env = getenv ("LOC_END"))
+ end = atoi (env);
+ if (const char *env = getenv ("CHAR"))
+ chr = atoi (env);
+
+ substring_loc loc
+ (info.fmtloc, TREE_TYPE (info.format), caret, beg, end);
+#else
+ substring_loc loc
+ (info.fmtloc, TREE_TYPE (info.format), off - !len, len ? off : 0,
+ off + len - !!len);
+#endif
Please remove the #if 0'd code.
+
+ /* Avoid the return value optimization when the behavior of the call
+ is undefined either because any directive may have produced 4K or
+ more of output, or the return value exceeds INT_MAX, or because
+ the ourput overflows the destination object (but leave it enabled
s/ourput/output/
I think with the nits above fixed, this is OK for the trunk.
Thanks for your patience.
Jeff
