[PR middle-end/85598] make -Wprintf* pass use loop info for PHI's

[Aldy Hernandez]

Hi folks.

The problem here is that Wprintf* uses the evrp_range_analyzer engine, 
and doesn't understand that the x_5 != 256 conditional below should make 
the RANGE [0, 255], not [0, 256].

  <bb 3> [local count: 1063004407]:
  # RANGE [0, 256] NONZERO 511
  # x_10 = PHI <0(2), x_5(3)>
  snprintf (&temp, 4, "%%%02X", x_10);
  # RANGE [1, 256] NONZERO 511
  x_5 = x_10 + 1;
  if (x_5 != 256)
    goto <bb 3>; [98.99%]
  else
    goto <bb 4>; [1.01%]

This PR is handled quite easily in the ranger work, but alas there is 
nothing for this release.  Therefore, I'd like to dedicate as little 
time as possible to this PR this time around.

Various possible solutions are discussed in the PR.  I think an easy way 
is to lean on loop analysis to refine the PHI result.  It turns out the 
evrp engine already does this if SCEV data is available.

As Richard mentioned in the PR, we should avoid code differences 
dependent on -W* flags.  I have put the loop init code in the pass 
itself, but am open to moving it outside, perhaps early in the gate 
function ??.

I also took the liberty of calling loop_optimizer_init() with the 
smallest subset of LOOPS_NORMAL which could still fix the problem 
(LOOPS_HAVE_PREHEADERS).  This avoids unnecessary changes to the CFG. 
But really, I'm just guessing here.  LOOPS_NORMAL would also work, 
albeit creating forwarder blocks.

Tested on x86-64 Linux.

What do you think?

Aldy
commit 189e32856dc4656931a66d4da0be81abb2eceb46
Author: Aldy Hernandez <al...@redhat.com>
Date:   Wed Jan 30 12:25:25 2019 +0100

            PR middle-end/85598
            * gimple-ssa-sprintf.c (pass_sprintf_length::execute): Build
            optimizer info when running late and optimizing.

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 8e6016fc42f..973452fd209 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -65,6 +65,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-propagate.h"
 #include "calls.h"
 #include "cfgloop.h"
+#include "tree-scalar-evolution.h"
+#include "tree-ssa-loop.h"
 #include "intl.h"
 #include "langhooks.h"
 
@@ -4200,10 +4202,34 @@ pass_sprintf_length::execute (function *fun)
   init_target_to_host_charmap ();
 
   calculate_dominance_info (CDI_DOMINATORS);
+  bool optimizing_late = optimize > 0 && fold_return_value;
+  if (optimizing_late)
+    {
+      /* ?? We should avoid changing the CFG as much as possible.
+	 This is a warning pass, after all.  Thus, use
+	 LOOPS_HAVE_PREHEADERS instead of LOOPS_NORMAL.  This avoids
+	 extensive changes to the CFG, without the full hammer of
+	 AVOID_CFG_MANIPULATIONS which would cripple SCEV.
+
+	 ?? Should we run this outside of the pass (early in the gate
+	 function).
+
+	 FIXME: This should go away, when we have finer grained or
+	 on-demand range information.  */
+      loop_optimizer_init (LOOPS_HAVE_PREHEADERS);
+      scev_initialize ();
+    }
+
 
   sprintf_dom_walker sprintf_dom_walker;
   sprintf_dom_walker.walk (ENTRY_BLOCK_PTR_FOR_FN (fun));
 
+  if (optimizing_late)
+    {
+      scev_finalize ();
+      loop_optimizer_finalize ();
+    }
+
   /* Clean up object size info.  */
   fini_object_sizes ();
   return 0;
diff --git a/gcc/testsuite/gcc.dg/pr85598.c b/gcc/testsuite/gcc.dg/pr85598.c
new file mode 100644
index 00000000000..c84b63f8084
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr85598.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+extern void __chk_fail (void);
+extern int snprintf (char *, size_t, const char *, ...);
+
+int main()
+{
+    char temp[100];
+    unsigned int x;
+    char *str = temp;
+    for(x=0; x<256; ++x) {
+      snprintf(str, 4, "%%%02X", x);
+    }
+}