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


Reply via email to