[PATCH] Fix PR80776

[PATCH] Fix PR80776

[Richard Biener]

The following avoids -Wformat-overflow false positives by teaching
EVRP the trick about __builtin_unreachable () "other" edges and
attaching range info to SSA names.  EVRP does a better job in keeping
ranges for every SSA name from conditional info (VRP "optimizes" its
costly ASSERT_EXPR insertion process).

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

This will also fix the testcase from PR83072 but it doesn't really
fix all cases I want to fix with a fix for it.  OTOH it might be
this is enough for stage3.

Richard.

2017-11-27  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80776
	* gimple-ssa-evrp-analyze.h (evrp_range_analyzer::set_ssa_range_info):
	Declare.
	* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::set_ssa_range_info):
	New function.
	(evrp_range_analyzer::record_ranges_from_incoming_edges):
	If the incoming edge is an effective fallthru because the other
	edge only reaches a __builtin_unreachable () then record ranges
	derived from the controlling condition in SSA info.
	(evrp_range_analyzer::record_ranges_from_phis): Use set_ssa_range_info.
	(evrp_range_analyzer::record_ranges_from_stmt): Likewise.

	* gcc.dg/pr80776-1.c: New testcase.
	* gcc.dg/pr80776-2.c: Likewise.

Index: gcc/gimple-ssa-evrp-analyze.c
===================================================================
--- gcc/gimple-ssa-evrp-analyze.c	(revision 255163)
+++ gcc/gimple-ssa-evrp-analyze.c	(working copy)
@@ -91,6 +91,31 @@ evrp_range_analyzer::try_find_new_range
   return NULL;
 }
 
+/* For LHS record VR in the SSA info.  */
+void
+evrp_range_analyzer::set_ssa_range_info (tree lhs, value_range *vr)
+{
+  /* Set the SSA with the value range.  */
+  if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
+    {
+      if ((vr->type == VR_RANGE
+	   || vr->type == VR_ANTI_RANGE)
+	  && (TREE_CODE (vr->min) == INTEGER_CST)
+	  && (TREE_CODE (vr->max) == INTEGER_CST))
+	set_range_info (lhs, vr->type,
+			wi::to_wide (vr->min),
+			wi::to_wide (vr->max));
+    }
+  else if (POINTER_TYPE_P (TREE_TYPE (lhs))
+	   && ((vr->type == VR_RANGE
+		&& range_includes_zero_p (vr->min,
+					  vr->max) == 0)
+	       || (vr->type == VR_ANTI_RANGE
+		   && range_includes_zero_p (vr->min,
+					     vr->max) == 1)))
+    set_ptr_nonnull (lhs);
+}
+
 void
 evrp_range_analyzer::record_ranges_from_incoming_edge (basic_block bb)
 {
@@ -134,10 +159,19 @@ evrp_range_analyzer::record_ranges_from_
 	      if (vr)
 		vrs.safe_push (std::make_pair (asserts[i].name, vr));
 	    }
+
+	  /* If pred_e is really a fallthru we can record value ranges
+	     in SSA names as well.  */
+	  bool is_fallthru = assert_unreachable_fallthru_edge_p (pred_e);
+
 	  /* Push updated ranges only after finding all of them to avoid
 	     ordering issues that can lead to worse ranges.  */
 	  for (unsigned i = 0; i < vrs.length (); ++i)
-	    push_value_range (vrs[i].first, vrs[i].second);
+	    {
+	      push_value_range (vrs[i].first, vrs[i].second);
+	      if (is_fallthru)
+		set_ssa_range_info (vrs[i].first, vrs[i].second);
+	    }
 	}
     }
 }
@@ -185,24 +219,7 @@ evrp_range_analyzer::record_ranges_from_
       vr_values->update_value_range (lhs, &vr_result);
 
       /* Set the SSA with the value range.  */
-      if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
-	{
-	  if ((vr_result.type == VR_RANGE
-	       || vr_result.type == VR_ANTI_RANGE)
-	      && (TREE_CODE (vr_result.min) == INTEGER_CST)
-	      && (TREE_CODE (vr_result.max) == INTEGER_CST))
-	    set_range_info (lhs, vr_result.type,
-			    wi::to_wide (vr_result.min),
-			    wi::to_wide (vr_result.max));
-	}
-      else if (POINTER_TYPE_P (TREE_TYPE (lhs))
-	       && ((vr_result.type == VR_RANGE
-		    && range_includes_zero_p (vr_result.min,
-					      vr_result.max) == 0)
-		   || (vr_result.type == VR_ANTI_RANGE
-		       && range_includes_zero_p (vr_result.min,
-						 vr_result.max) == 1)))
-	set_ptr_nonnull (lhs);
+      set_ssa_range_info (lhs, &vr_result);
     }
 }
 
@@ -224,21 +241,7 @@ evrp_range_analyzer::record_ranges_from_
 	  vr_values->update_value_range (output, &vr);
 
 	  /* Set the SSA with the value range.  */
-	  if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
-	    {
-	      if ((vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE)
-		  && (TREE_CODE (vr.min) == INTEGER_CST)
-		  && (TREE_CODE (vr.max) == INTEGER_CST))
-		set_range_info (output, vr.type,
-				wi::to_wide (vr.min),
-				wi::to_wide (vr.max));
-	    }
-	  else if (POINTER_TYPE_P (TREE_TYPE (output))
-		   && ((vr.type == VR_RANGE
-			&& range_includes_zero_p (vr.min, vr.max) == 0)
-		       || (vr.type == VR_ANTI_RANGE
-			   && range_includes_zero_p (vr.min, vr.max) == 1)))
-	    set_ptr_nonnull (output);
+	  set_ssa_range_info (output, &vr);
 	}
       else
 	vr_values->set_defs_to_varying (stmt);
Index: gcc/gimple-ssa-evrp-analyze.h
===================================================================
--- gcc/gimple-ssa-evrp-analyze.h	(revision 255163)
+++ gcc/gimple-ssa-evrp-analyze.h	(working copy)
@@ -68,6 +68,7 @@ class evrp_range_analyzer
   value_range *try_find_new_range (tree, tree op, tree_code code, tree limit);
   void record_ranges_from_incoming_edge (basic_block);
   void record_ranges_from_phis (basic_block);
+  void set_ssa_range_info (tree, value_range *);
 
   /* STACK holds the old VR.  */
   auto_vec<std::pair <tree, value_range*> > stack;
Index: gcc/testsuite/gcc.dg/pr80776-1.c
===================================================================
--- gcc/testsuite/gcc.dg/pr80776-1.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr80776-1.c	(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wformat-overflow" } */
+
+extern __inline __attribute__ ((__always_inline__)) __attribute__ ((__gnu_inline__)) __attribute__ ((__artificial__)) int
+__attribute__ ((__nothrow__ , __leaf__)) sprintf (char *__restrict __s, const char *__restrict __fmt, ...)
+{
+  return __builtin___sprintf_chk (__s, 2 - 1,
+				  __builtin_object_size (__s, 2 > 1), __fmt, __builtin_va_arg_pack ());
+}
+char number[sizeof "999999"];
+int somerandom (void);
+void
+Foo (void)
+{
+  int i = somerandom ();
+  if (! (0 <= i))
+    __builtin_unreachable ();
+  if (! (0 <= i && i <= 999999))
+    __builtin_unreachable ();
+  sprintf (number, "%d", i); /* { dg-bogus "writing" } */
+}
Index: gcc/testsuite/gcc.dg/pr80776-2.c
===================================================================
--- gcc/testsuite/gcc.dg/pr80776-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr80776-2.c	(working copy)
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wformat-overflow" } */
+
+extern int sprintf (char *restrict, const char *restrict, ...)
+     __attribute__ ((__nothrow__));
+     extern int foo (void);
+
+int
+Fgenerate_new_buffer_name (void)
+{
+  char number[2];
+  int i = foo ();
+  if (i < 0)
+    __builtin_unreachable ();
+  if (i >= 2)
+    __builtin_unreachable ();
+  return sprintf (number, "%d", i); /* { dg-bogus "writing" } */
+}

Re: [PATCH] Fix PR80776

[Jeff Law]

On 11/27/2017 06:39 AM, Richard Biener wrote:
> 
> The following avoids -Wformat-overflow false positives by teaching
> EVRP the trick about __builtin_unreachable () "other" edges and
> attaching range info to SSA names.  EVRP does a better job in keeping
> ranges for every SSA name from conditional info (VRP "optimizes" its
> costly ASSERT_EXPR insertion process).
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> This will also fix the testcase from PR83072 but it doesn't really
> fix all cases I want to fix with a fix for it.  OTOH it might be
> this is enough for stage3.
> 
> Richard.
> 
> 2017-11-27  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/80776
> 	* gimple-ssa-evrp-analyze.h (evrp_range_analyzer::set_ssa_range_info):
> 	Declare.
> 	* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::set_ssa_range_info):
> 	New function.
> 	(evrp_range_analyzer::record_ranges_from_incoming_edges):
> 	If the incoming edge is an effective fallthru because the other
> 	edge only reaches a __builtin_unreachable () then record ranges
> 	derived from the controlling condition in SSA info.
> 	(evrp_range_analyzer::record_ranges_from_phis): Use set_ssa_range_info.
> 	(evrp_range_analyzer::record_ranges_from_stmt): Likewise.
> 
> 	* gcc.dg/pr80776-1.c: New testcase.
> 	* gcc.dg/pr80776-2.c: Likewise.
So the thing to make sure of here is that the range information we
reflect back into the SSA_NAME actually applies everywhere the SSA_NAME
can appear.  ie, it's globally valid.

This means we can't reflect anything we derive from conditionals or
things like a *p making the range non-null back to the SSA_NAME.

I'd be concerned about the change to record_ranges_from_incoming_edge.

Jeff

Re: [PATCH] Fix PR80776

[Richard Biener]

On Mon, 27 Nov 2017, Jeff Law wrote:

> On 11/27/2017 06:39 AM, Richard Biener wrote:
> > 
> > The following avoids -Wformat-overflow false positives by teaching
> > EVRP the trick about __builtin_unreachable () "other" edges and
> > attaching range info to SSA names.  EVRP does a better job in keeping
> > ranges for every SSA name from conditional info (VRP "optimizes" its
> > costly ASSERT_EXPR insertion process).
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > 
> > This will also fix the testcase from PR83072 but it doesn't really
> > fix all cases I want to fix with a fix for it.  OTOH it might be
> > this is enough for stage3.
> > 
> > Richard.
> > 
> > 2017-11-27  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR tree-optimization/80776
> > 	* gimple-ssa-evrp-analyze.h (evrp_range_analyzer::set_ssa_range_info):
> > 	Declare.
> > 	* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::set_ssa_range_info):
> > 	New function.
> > 	(evrp_range_analyzer::record_ranges_from_incoming_edges):
> > 	If the incoming edge is an effective fallthru because the other
> > 	edge only reaches a __builtin_unreachable () then record ranges
> > 	derived from the controlling condition in SSA info.
> > 	(evrp_range_analyzer::record_ranges_from_phis): Use set_ssa_range_info.
> > 	(evrp_range_analyzer::record_ranges_from_stmt): Likewise.
> > 
> > 	* gcc.dg/pr80776-1.c: New testcase.
> > 	* gcc.dg/pr80776-2.c: Likewise.
> So the thing to make sure of here is that the range information we
> reflect back into the SSA_NAME actually applies everywhere the SSA_NAME
> can appear.  ie, it's globally valid.
> 
> This means we can't reflect anything we derive from conditionals or
> things like a *p making the range non-null back to the SSA_NAME.
> 
> I'd be concerned about the change to record_ranges_from_incoming_edge.

It's basically a copy of what VRP does when removing range assertions.
I've added the correctness check I missed and also the trick with
setting nonzero bits.

This causes us to no longer handle the gcc.dg/pr80776-1.c case:

  <bb 2> :
  i_4 = somerandom ();
  if (i_4 < 0)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 3> :
  __builtin_unreachable ();

  <bb 4> :
  i.0_1 = (unsigned int) i_4;
  if (i.0_1 > 999999)
    goto <bb 5>; [INV]
  else
    goto <bb 6>; [INV]

  <bb 5> :
  __builtin_unreachable ();

  <bb 6> :
  _7 = __builtin___sprintf_chk (&number, 1, 7, "%d", i_4);

when trying to update the SSA range info for i_4 from the
if (i.0_1 > 999999) we see the use in the dominating condition
and thus conclude we cannot update the SSA range info like we want.

ifcombine also doesn't merge the tests because I think it gets
confused by the __builtin_unreachable ().  ifcombine also runs after
VRP1 which gets rid of the __builtin_unreachable ()s.

I think for GCC 9 we may want to experiment with moving ifcombine
before VRP1 and handling if-chains with __builtin_unreachable ()s.

Anyway, re-testing below.

Richard.

2017-11-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80776
	* gimple-ssa-evrp-analyze.h (evrp_range_analyzer::set_ssa_range_info):
	Declare.
	* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::set_ssa_range_info):
	New function.
	(evrp_range_analyzer::record_ranges_from_incoming_edges):
	If the incoming edge is an effective fallthru because the other
	edge only reaches a __builtin_unreachable () then record ranges
	derived from the controlling condition in SSA info.
	(evrp_range_analyzer::record_ranges_from_phis): Use set_ssa_range_info.
	(evrp_range_analyzer::record_ranges_from_stmt): Likewise.

	* gcc.dg/pr80776-1.c: New testcase.
	* gcc.dg/pr80776-2.c: Likewise.

Index: gcc/gimple-ssa-evrp-analyze.c
===================================================================
--- gcc/gimple-ssa-evrp-analyze.c	(revision 255172)
+++ gcc/gimple-ssa-evrp-analyze.c	(working copy)
@@ -91,6 +91,62 @@ evrp_range_analyzer::try_find_new_range
   return NULL;
 }
 
+/* For LHS record VR in the SSA info.  */
+void
+evrp_range_analyzer::set_ssa_range_info (tree lhs, value_range *vr)
+{
+  /* Set the SSA with the value range.  */
+  if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
+    {
+      if ((vr->type == VR_RANGE
+	   || vr->type == VR_ANTI_RANGE)
+	  && (TREE_CODE (vr->min) == INTEGER_CST)
+	  && (TREE_CODE (vr->max) == INTEGER_CST))
+	set_range_info (lhs, vr->type,
+			wi::to_wide (vr->min),
+			wi::to_wide (vr->max));
+    }
+  else if (POINTER_TYPE_P (TREE_TYPE (lhs))
+	   && ((vr->type == VR_RANGE
+		&& range_includes_zero_p (vr->min,
+					  vr->max) == 0)
+	       || (vr->type == VR_ANTI_RANGE
+		   && range_includes_zero_p (vr->min,
+					     vr->max) == 1)))
+    set_ptr_nonnull (lhs);
+}
+
+/* Return true if all uses of NAME are dominated by STMT or feed STMT
+   via a chain of single immediate uses.  */
+
+static bool
+all_uses_feed_or_dominated_by_stmt (tree name, gimple *stmt)
+{
+  use_operand_p use_p, use2_p;
+  imm_use_iterator iter;
+  basic_block stmt_bb = gimple_bb (stmt);
+
+  FOR_EACH_IMM_USE_FAST (use_p, iter, name)
+    {
+      gimple *use_stmt = USE_STMT (use_p), *use_stmt2;
+      if (use_stmt == stmt
+	  || is_gimple_debug (use_stmt)
+	  || (gimple_bb (use_stmt) != stmt_bb
+	      && dominated_by_p (CDI_DOMINATORS,
+				 gimple_bb (use_stmt), stmt_bb)))
+	continue;
+      while (use_stmt != stmt
+	     && is_gimple_assign (use_stmt)
+	     && TREE_CODE (gimple_assign_lhs (use_stmt)) == SSA_NAME
+	     && single_imm_use (gimple_assign_lhs (use_stmt),
+				&use2_p, &use_stmt2))
+	use_stmt = use_stmt2;
+      if (use_stmt != stmt)
+	return false;
+    }
+  return true;
+}
+
 void
 evrp_range_analyzer::record_ranges_from_incoming_edge (basic_block bb)
 {
@@ -134,10 +190,23 @@ evrp_range_analyzer::record_ranges_from_
 	      if (vr)
 		vrs.safe_push (std::make_pair (asserts[i].name, vr));
 	    }
+
+	  /* If pred_e is really a fallthru we can record value ranges
+	     in SSA names as well.  */
+	  bool is_fallthru = assert_unreachable_fallthru_edge_p (pred_e);
+
 	  /* Push updated ranges only after finding all of them to avoid
 	     ordering issues that can lead to worse ranges.  */
 	  for (unsigned i = 0; i < vrs.length (); ++i)
-	    push_value_range (vrs[i].first, vrs[i].second);
+	    {
+	      push_value_range (vrs[i].first, vrs[i].second);
+	      if (is_fallthru
+		  && all_uses_feed_or_dominated_by_stmt (vrs[i].first, stmt))
+		{
+		  set_ssa_range_info (vrs[i].first, vrs[i].second);
+		  maybe_set_nonzero_bits (pred_e, vrs[i].first);
+		}
+	    }
 	}
     }
 }
@@ -185,24 +254,7 @@ evrp_range_analyzer::record_ranges_from_
       vr_values->update_value_range (lhs, &vr_result);
 
       /* Set the SSA with the value range.  */
-      if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
-	{
-	  if ((vr_result.type == VR_RANGE
-	       || vr_result.type == VR_ANTI_RANGE)
-	      && (TREE_CODE (vr_result.min) == INTEGER_CST)
-	      && (TREE_CODE (vr_result.max) == INTEGER_CST))
-	    set_range_info (lhs, vr_result.type,
-			    wi::to_wide (vr_result.min),
-			    wi::to_wide (vr_result.max));
-	}
-      else if (POINTER_TYPE_P (TREE_TYPE (lhs))
-	       && ((vr_result.type == VR_RANGE
-		    && range_includes_zero_p (vr_result.min,
-					      vr_result.max) == 0)
-		   || (vr_result.type == VR_ANTI_RANGE
-		       && range_includes_zero_p (vr_result.min,
-						 vr_result.max) == 1)))
-	set_ptr_nonnull (lhs);
+      set_ssa_range_info (lhs, &vr_result);
     }
 }
 
@@ -224,21 +276,7 @@ evrp_range_analyzer::record_ranges_from_
 	  vr_values->update_value_range (output, &vr);
 
 	  /* Set the SSA with the value range.  */
-	  if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
-	    {
-	      if ((vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE)
-		  && (TREE_CODE (vr.min) == INTEGER_CST)
-		  && (TREE_CODE (vr.max) == INTEGER_CST))
-		set_range_info (output, vr.type,
-				wi::to_wide (vr.min),
-				wi::to_wide (vr.max));
-	    }
-	  else if (POINTER_TYPE_P (TREE_TYPE (output))
-		   && ((vr.type == VR_RANGE
-			&& range_includes_zero_p (vr.min, vr.max) == 0)
-		       || (vr.type == VR_ANTI_RANGE
-			   && range_includes_zero_p (vr.min, vr.max) == 1)))
-	    set_ptr_nonnull (output);
+	  set_ssa_range_info (output, &vr);
 	}
       else
 	vr_values->set_defs_to_varying (stmt);
Index: gcc/gimple-ssa-evrp-analyze.h
===================================================================
--- gcc/gimple-ssa-evrp-analyze.h	(revision 255172)
+++ gcc/gimple-ssa-evrp-analyze.h	(working copy)
@@ -68,6 +68,7 @@ class evrp_range_analyzer
   value_range *try_find_new_range (tree, tree op, tree_code code, tree limit);
   void record_ranges_from_incoming_edge (basic_block);
   void record_ranges_from_phis (basic_block);
+  void set_ssa_range_info (tree, value_range *);
 
   /* STACK holds the old VR.  */
   auto_vec<std::pair <tree, value_range*> > stack;
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 255172)
+++ gcc/tree-vrp.c	(working copy)
@@ -5082,10 +5082,9 @@ all_imm_uses_in_stmt_or_feed_cond (tree
    var is the x_3 var from ASSERT_EXPR, we can clear low 5 bits
    from the non-zero bitmask.  */
 
-static void
-maybe_set_nonzero_bits (basic_block bb, tree var)
+void
+maybe_set_nonzero_bits (edge e, tree var)
 {
-  edge e = single_pred_edge (bb);
   basic_block cond_bb = e->src;
   gimple *stmt = last_stmt (cond_bb);
   tree cst;
@@ -5200,7 +5199,7 @@ remove_range_assertions (void)
 		    set_range_info (var, SSA_NAME_RANGE_TYPE (lhs),
 				    SSA_NAME_RANGE_INFO (lhs)->get_min (),
 				    SSA_NAME_RANGE_INFO (lhs)->get_max ());
-		    maybe_set_nonzero_bits (bb, var);
+		    maybe_set_nonzero_bits (single_pred_edge (bb), var);
 		  }
 	      }
 
Index: gcc/tree-vrp.h
===================================================================
--- gcc/tree-vrp.h	(revision 255172)
+++ gcc/tree-vrp.h	(working copy)
@@ -116,6 +116,7 @@ extern bool overflow_comparison_p (tree_
 extern bool range_int_cst_singleton_p (value_range *);
 extern int value_inside_range (tree, tree, tree);
 extern tree get_single_symbol (tree, bool *, tree *);
+extern void maybe_set_nonzero_bits (edge, tree);
 
 
 struct switch_update {
Index: gcc/testsuite/gcc.dg/pr80776-1.c
===================================================================
--- gcc/testsuite/gcc.dg/pr80776-1.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr80776-1.c	(working copy)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wformat-overflow" } */
+
+extern __inline __attribute__ ((__always_inline__)) __attribute__ ((__gnu_inline__)) __attribute__ ((__artificial__)) int
+__attribute__ ((__nothrow__ , __leaf__)) sprintf (char *__restrict __s, const char *__restrict __fmt, ...)
+{
+  return __builtin___sprintf_chk (__s, 2 - 1,
+				  __builtin_object_size (__s, 2 > 1), __fmt, __builtin_va_arg_pack ());
+}
+char number[sizeof "999999"];
+int somerandom (void);
+void
+Foo (void)
+{
+  int i = somerandom ();
+  if (! (0 <= i))
+    __builtin_unreachable ();
+  if (! (0 <= i && i <= 999999))
+    __builtin_unreachable ();
+  /* The correctness bits for [E]VRP cannot handle chained conditionals
+     when deciding to ignore a unreachable branch for setting SSA range info. */
+  sprintf (number, "%d", i); /* { dg-bogus "writing" "" { xfail *-*-* } } */
+}
Index: gcc/testsuite/gcc.dg/pr80776-2.c
===================================================================
--- gcc/testsuite/gcc.dg/pr80776-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr80776-2.c	(working copy)
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wformat-overflow" } */
+
+extern int sprintf (char *restrict, const char *restrict, ...)
+     __attribute__ ((__nothrow__));
+     extern int foo (void);
+
+int
+Fgenerate_new_buffer_name (void)
+{
+  char number[2];
+  int i = foo ();
+  if (i < 0)
+    __builtin_unreachable ();
+  if (i >= 2)
+    __builtin_unreachable ();
+  return sprintf (number, "%d", i); /* { dg-bogus "writing" } */
+}

Re: [PATCH] Fix PR80776

[Jeff Law]

On 11/28/2017 02:14 AM, Richard Biener wrote:
> On Mon, 27 Nov 2017, Jeff Law wrote:
> 
>> On 11/27/2017 06:39 AM, Richard Biener wrote:
>>>
>>> The following avoids -Wformat-overflow false positives by teaching
>>> EVRP the trick about __builtin_unreachable () "other" edges and
>>> attaching range info to SSA names.  EVRP does a better job in keeping
>>> ranges for every SSA name from conditional info (VRP "optimizes" its
>>> costly ASSERT_EXPR insertion process).
>>>
>>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>>>
>>> This will also fix the testcase from PR83072 but it doesn't really
>>> fix all cases I want to fix with a fix for it.  OTOH it might be
>>> this is enough for stage3.
>>>
>>> Richard.
>>>
>>> 2017-11-27  Richard Biener  <rguenther@suse.de>
>>>
>>> 	PR tree-optimization/80776
>>> 	* gimple-ssa-evrp-analyze.h (evrp_range_analyzer::set_ssa_range_info):
>>> 	Declare.
>>> 	* gimple-ssa-evrp-analyze.c (evrp_range_analyzer::set_ssa_range_info):
>>> 	New function.
>>> 	(evrp_range_analyzer::record_ranges_from_incoming_edges):
>>> 	If the incoming edge is an effective fallthru because the other
>>> 	edge only reaches a __builtin_unreachable () then record ranges
>>> 	derived from the controlling condition in SSA info.
>>> 	(evrp_range_analyzer::record_ranges_from_phis): Use set_ssa_range_info.
>>> 	(evrp_range_analyzer::record_ranges_from_stmt): Likewise.
>>>
>>> 	* gcc.dg/pr80776-1.c: New testcase.
>>> 	* gcc.dg/pr80776-2.c: Likewise.
>> So the thing to make sure of here is that the range information we
>> reflect back into the SSA_NAME actually applies everywhere the SSA_NAME
>> can appear.  ie, it's globally valid.
>>
>> This means we can't reflect anything we derive from conditionals or
>> things like a *p making the range non-null back to the SSA_NAME.
>>
>> I'd be concerned about the change to record_ranges_from_incoming_edge.
> 
> It's basically a copy of what VRP does when removing range assertions.
> I've added the correctness check I missed and also the trick with
> setting nonzero bits.
> 
> This causes us to no longer handle the gcc.dg/pr80776-1.c case:
> 
>   <bb 2> :
>   i_4 = somerandom ();
>   if (i_4 < 0)
>     goto <bb 3>; [INV]
>   else
>     goto <bb 4>; [INV]
> 
>   <bb 3> :
>   __builtin_unreachable ();
> 
>   <bb 4> :
>   i.0_1 = (unsigned int) i_4;
>   if (i.0_1 > 999999)
>     goto <bb 5>; [INV]
>   else
>     goto <bb 6>; [INV]
> 
>   <bb 5> :
>   __builtin_unreachable ();
> 
>   <bb 6> :
>   _7 = __builtin___sprintf_chk (&number, 1, 7, "%d", i_4);
> 
> when trying to update the SSA range info for i_4 from the
> if (i.0_1 > 999999) we see the use in the dominating condition
> and thus conclude we cannot update the SSA range info like we want.
So the unreachables add a twist.  If we assume the unreachables are in
fact unreachable, then ISTM we can use the conditionals to derive
refined ranges on the other edge.  The implications of the unreachable
didn't really sink in until just now.

> 
> ifcombine also doesn't merge the tests because I think it gets
> confused by the __builtin_unreachable ().  ifcombine also runs after
> VRP1 which gets rid of the __builtin_unreachable ()s.
Yea.  As you know I was just poking at a case yesterday where ifcombine
worked sub-optimally and its placement was inconvenient...  But given
the particular case I was looking at it made more sense to simplify the
IL prior to ifcombine.

> 
> I think for GCC 9 we may want to experiment with moving ifcombine
> before VRP1 and handling if-chains with __builtin_unreachable ()s.
Worth investigation.

Jeff