[PATCH] Improve -fsanitize=undefined a little bit

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

This patch improves a little bit the generic expansion of
UBSAN_CHECK_{ADD,SUB} by either making sure that if one of the
operands is CONST_INT, it is the second one (so that the first compare
+ conditional jump is folded) or by looking at value ranges if we can prove
one of the arguments is either always negative or always non-negative.

Furthermore, the gimple_fold_stmt_to_constant_1 change earlier only
folds UBSAN_CHECK_* if both arguments are determined to be constant,
but we can actually do better than that, if value ranges prove that
there won't be an overflow, we can replace it by normal
{PLUS,MINUS,MULT}_EXPR.

Testcase I had in mind is e.g., here (in generic expansion) in fn1
we have to do 2 runtime (3 in the code) comparisons, in fn[23456]
just one, in fn7 the new tree-vrp.c code now figures out the
operation will never overflow and thus let's the optimizers generate
better code, and in fn8 we have handled it before already.

Marek, do you think you could turn this eventually into a testcases,
perhaps for different types (macroize?) and also operations (+/-/*/unary -),
with some test that just checks if things that don't overflow get the
right values and perhaps a few tests for each of the different expansions
where they do in fact overflow (those need to be one overflow per
test I'm afraid, even when you can have e.g. most of the test stuff
in some header or .c file and just tweak some macros)?

For ubsan_expand_si_overflow_neg_check, I think tree-vrp.c change can be
improved to handle also anti ranges, if the first argument of
UBSAN_CHECK_SUB has value range [0, 0] and second argument anti-range
~[x, y] where x is minimum, then it will also never overflow.  Though,
I wonder if VRP doesn't canonicalize say ~[INT_MIN, INT_MIN] into
[INT_MIN+1, INT_MAX].

And, ubsan_expand_si_overflow_mul_check needs more work, first of all
to actually handle cases where we now punt (aka pretend overflow never
happens).

int
fn1 (int x, int y)
{
  return x + y;
}

int
fn2 (int x)
{
  return x + 7;
}

int
fn3 (unsigned char x, int y)
{
  return x + y;
}

int
fn4 (int x, unsigned char y)
{
  return x + y;
}

int
fn5 (unsigned char x, int y)
{
  return ~x + y;
}

int
fn6 (int x, unsigned char y)
{
  return x + ~y;
}

int
fn7 (unsigned char x, unsigned char y)
{
  return x + y;
}

int
fn8 (void)
{
  int x = 5;
  int y = 6;
  return x + y;
}


2013-12-05  Jakub Jelinek  <jakub@redhat.com>

	* internal-fn.c: Include stringpool.h and tree-ssanames.h.
	(ubsan_expand_si_overflow_addsub_check): In the generic expansion,
	try to improve generated code if one of the arguments is constant
	or get_range_info says that one of the argument is always
	negative or always non-negative.
	* tree-vrp.c (simplify_internal_call_using_ranges): New function.
	(simplify_stmt_using_ranges): Call it.

--- gcc/internal-fn.c.jj	2013-12-05 09:39:36.000000000 +0100
+++ gcc/internal-fn.c	2013-12-05 12:24:38.925292695 +0100
@@ -34,6 +34,8 @@ along with GCC; see the file COPYING3.
 #include "ubsan.h"
 #include "target.h"
 #include "predict.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 
 /* The names of each internal function, indexed by function number.  */
 const char *const internal_fn_name_array[] = {
@@ -213,28 +215,81 @@ ubsan_expand_si_overflow_addsub_check (t
   if (icode == CODE_FOR_nothing)
     {
       rtx sub_check = gen_label_rtx ();
+      int pos_neg = 3;
 
       /* Compute the operation.  On RTL level, the addition is always
 	 unsigned.  */
       res = expand_binop (mode, add_optab, op0, op1,
 			  NULL_RTX, false, OPTAB_LIB_WIDEN);
 
+      /* If we can prove one of the arguments is always non-negative
+	 or always negative, we can do just one comparison and
+	 conditional jump instead of 2 at runtime, 3 present in the
+	 emitted code.  If one of the arguments is CONST_INT, all we
+	 need is to make sure it is op1, then the first
+	 emit_cmp_and_jump_insns will be just folded.  Otherwise try
+	 to use range info if available.  */
+      if (CONST_INT_P (op0))
+	{
+	  rtx tem = op0;
+	  op0 = op1;
+	  op1 = tem;
+	}
+      else if (CONST_INT_P (op1))
+	;
+      else if (TREE_CODE (arg0) == SSA_NAME)
+	{
+	  double_int arg0_min, arg0_max;
+	  if (get_range_info (arg0, &arg0_min, &arg0_max) == VR_RANGE)
+	    {
+	      if (!arg0_min.is_negative ())
+		pos_neg = 1;
+	      else if (arg0_max.is_negative ())
+		pos_neg = 2;
+	    }
+	  if (pos_neg != 3)
+	    {
+	      rtx tem = op0;
+	      op0 = op1;
+	      op1 = tem;
+	    }
+	}
+      if (pos_neg == 3 && !CONST_INT_P (op1) && TREE_CODE (arg1) == SSA_NAME)
+	{
+	  double_int arg1_min, arg1_max;
+	  if (get_range_info (arg1, &arg1_min, &arg1_max) == VR_RANGE)
+	    {
+	      if (!arg1_min.is_negative ())
+		pos_neg = 1;
+	      else if (arg1_max.is_negative ())
+		pos_neg = 2;
+	    }
+	}
+
       /* If the op1 is negative, we have to use a different check.  */
-      emit_cmp_and_jump_insns (op1, const0_rtx, LT, NULL_RTX, mode,
-			       false, sub_check, PROB_EVEN);
+      if (pos_neg == 3)
+	emit_cmp_and_jump_insns (op1, const0_rtx, LT, NULL_RTX, mode,
+				 false, sub_check, PROB_EVEN);
 
       /* Compare the result of the addition with one of the operands.  */
-      emit_cmp_and_jump_insns (res, op0, code == PLUS_EXPR ? GE : LE,
-			       NULL_RTX, mode, false, done_label,
-			       PROB_VERY_LIKELY);
+      if (pos_neg & 1)
+	emit_cmp_and_jump_insns (res, op0, code == PLUS_EXPR ? GE : LE,
+				 NULL_RTX, mode, false, done_label,
+				 PROB_VERY_LIKELY);
+
       /* If we get here, we have to print the error.  */
-      emit_jump (do_error);
+      if (pos_neg == 3)
+	{
+	  emit_jump (do_error);
+
+	  emit_label (sub_check);
+	}
 
-      emit_label (sub_check);
       /* We have k = a + b for b < 0 here.  k <= a must hold.  */
-      emit_cmp_and_jump_insns (res, op0, code == PLUS_EXPR ? LE : GE,
-			       NULL_RTX, mode, false, done_label,
-			       PROB_VERY_LIKELY);
+      if (pos_neg & 2)
+	emit_cmp_and_jump_insns (res, op0, code == PLUS_EXPR ? LE : GE,
+				 NULL_RTX, mode, false, done_label,
+				 PROB_VERY_LIKELY);
     }
 
    emit_label (do_error);
--- gcc/tree-vrp.c.jj	2013-12-05 09:39:34.000000000 +0100
+++ gcc/tree-vrp.c	2013-12-05 11:57:42.096767153 +0100
@@ -9299,6 +9299,68 @@ simplify_float_conversion_using_ranges (
   return true;
 }
 
+/* Simplify an internal fn call using ranges if possible.  */
+
+static bool
+simplify_internal_call_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
+{
+  enum tree_code subcode;
+  switch (gimple_call_internal_fn (stmt))
+    {
+    case IFN_UBSAN_CHECK_ADD:
+      subcode = PLUS_EXPR;
+      break;
+    case IFN_UBSAN_CHECK_SUB:
+      subcode = MINUS_EXPR;
+      break;
+    case IFN_UBSAN_CHECK_MUL:
+      subcode = MULT_EXPR;
+      break;
+    default:
+      return false;
+    }
+
+  value_range_t vr0 = VR_INITIALIZER;
+  value_range_t vr1 = VR_INITIALIZER;
+  tree op0 = gimple_call_arg (stmt, 0);
+  tree op1 = gimple_call_arg (stmt, 1);
+
+  if (TREE_CODE (op0) == SSA_NAME)
+    vr0 = *get_value_range (op0);
+  else if (TREE_CODE (op0) == INTEGER_CST)
+    set_value_range_to_value (&vr0, op0, NULL);
+  else
+    return false;
+
+  if (TREE_CODE (op1) == SSA_NAME)
+    vr1 = *get_value_range (op1);
+  else if (TREE_CODE (op1) == INTEGER_CST)
+    set_value_range_to_value (&vr1, op1, NULL);
+  else
+    return false;
+
+  if (!range_int_cst_p (&vr0) || !range_int_cst_p (&vr1))
+    return false;
+
+  tree r1 = int_const_binop (subcode, vr0.min, vr1.min);
+  tree r2 = int_const_binop (subcode, vr0.max, vr1.max);
+  if (r1 == NULL_TREE || TREE_OVERFLOW (r1)
+      || r2 == NULL_TREE || TREE_OVERFLOW (r2))
+    return false;
+  if (subcode == MULT_EXPR)
+    {
+      tree r3 = int_const_binop (subcode, vr0.min, vr1.max);
+      tree r4 = int_const_binop (subcode, vr0.max, vr1.min);
+      if (r3 == NULL_TREE || TREE_OVERFLOW (r3)
+	  || r4 == NULL_TREE || TREE_OVERFLOW (r4))
+	return false;
+    }
+  gimple g = gimple_build_assign_with_ops (subcode, gimple_call_lhs (stmt),
+					   op0, op1);
+  gsi_replace (gsi, g, false);
+  return true;
+}
+
 /* Simplify STMT using ranges if possible.  */
 
 static bool
@@ -9367,6 +9429,9 @@ simplify_stmt_using_ranges (gimple_stmt_
     return simplify_cond_using_ranges (stmt);
   else if (gimple_code (stmt) == GIMPLE_SWITCH)
     return simplify_switch_using_ranges (stmt);
+  else if (is_gimple_call (stmt)
+	   && gimple_call_internal_p (stmt))
+    return simplify_internal_call_using_ranges (gsi, stmt);
 
   return false;
 }

	Jakub