ubsan: Move INT_MIN / -1 instrumentation from -fsanitize=integer-divide-by-zero to -fsanitize=signed-integer-overflow [PR102515]

As noted by Richi, in clang INT_MIN / -1 is instrumented under
-fsanitize=signed-integer-overflow rather than
-fsanitize=integer-divide-by-zero as we did and doing it in the former
makes more sense, as it is overflow during division rather than division
by zero.
I've verified on godbolt that clang behaved that way since 3.2-ish times or
so when sanitizers were added.
Furthermore, we've been using
-f{,no-}sanitize-recover=integer-divide-by-zero to decide on the float
-fsanitize=float-divide-by-zero instrumentation _abort suffix.
The case where INT_MIN / -1 is instrumented by one sanitizer and
x / 0 by another one when both are enabled is slightly harder if
the -f{,no-}sanitize-recover={integer-divide-by-zero,signed-integer-overflow}
flags differ, then we need to emit both __ubsan_handle_divrem_overflow
and __ubsan_handle_divrem_overflow_abort calls guarded by their respective
checks rather than one guarded by check1 || check2.

2021-10-01  Jakub Jelinek  <jakub@redhat.com>
	    Richard Biener  <rguenther@suse.de>

	PR sanitizer/102515
gcc/
	* doc/invoke.texi (-fsanitize=integer-divide-by-zero): Remove
	INT_MIN / -1 division detection from here ...
	(-fsanitize=signed-integer-overflow): ... and add it here.
gcc/c-family/
	* c-ubsan.c (ubsan_instrument_division): Check the right
	flag_sanitize_recover bit, depending on which sanitization
	is done.  Sanitize INT_MIN / -1 under SANITIZE_SI_OVERFLOW
	rather than SANITIZE_DIVIDE.  If both SANITIZE_SI_OVERFLOW
	and SANITIZE_DIVIDE is enabled, neither check is known
	to be false and flag_sanitize_recover bits for those two
	aren't the same, emit both __ubsan_handle_divrem_overflow
	and __ubsan_handle_divrem_overflow_abort calls.
gcc/c/
	* c-typeck.c (build_binary_op): Call ubsan_instrument_division
	for division even for SANITIZE_SI_OVERFLOW.
gcc/cp/
	* typeck.c (cp_build_binary_op): Call ubsan_instrument_division
	for division even for SANITIZE_SI_OVERFLOW.
gcc/testsuite/
	* c-c++-common/ubsan/div-by-zero-3.c: Use
	-fsanitize=signed-integer-overflow instead of
	-fsanitize=integer-divide-by-zero.
	* c-c++-common/ubsan/div-by-zero-5.c: Likewise.
	* c-c++-common/ubsan/div-by-zero-4.c: Likewise.  Add
	-fsanitize-undefined-trap-on-error.
	* c-c++-common/ubsan/float-div-by-zero-2.c: New test.
	* c-c++-common/ubsan/overflow-div-1.c: New test.
	* c-c++-common/ubsan/overflow-div-2.c: New test.
	* c-c++-common/ubsan/overflow-div-3.c: New test.

1 files changed, 40 insertions, 9 deletions

diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c
index 12a7bca..a4509c6 100644
--- a/gcc/c-family/c-ubsan.c
+++ b/gcc/c-family/c-ubsan.c
@@ -39,8 +39,9 @@ along with GCC; see the file COPYING3.  If not see
 tree
 ubsan_instrument_division (location_t loc, tree op0, tree op1)
 {
-  tree t, tt;
+  tree t, tt, x = NULL_TREE;
   tree type = TREE_TYPE (op0);
+  enum sanitize_code flag = SANITIZE_DIVIDE;
 
   /* At this point both operands should have the same type,
      because they are already converted to RESULT_TYPE.
@@ -58,24 +59,42 @@ ubsan_instrument_division (location_t loc, tree op0, tree op1)
 		     op1, build_int_cst (type, 0));
   else if (TREE_CODE (type) == REAL_TYPE
 	   && sanitize_flags_p (SANITIZE_FLOAT_DIVIDE))
-    t = fold_build2 (EQ_EXPR, boolean_type_node,
-		     op1, build_real (type, dconst0));
+    {
+      t = fold_build2 (EQ_EXPR, boolean_type_node,
+		       op1, build_real (type, dconst0));
+      flag = SANITIZE_FLOAT_DIVIDE;
+    }
   else
-    return NULL_TREE;
+    t = NULL_TREE;
 
   /* We check INT_MIN / -1 only for signed types.  */
   if (TREE_CODE (type) == INTEGER_TYPE
-      && sanitize_flags_p (SANITIZE_DIVIDE)
+      && sanitize_flags_p (SANITIZE_SI_OVERFLOW)
       && !TYPE_UNSIGNED (type))
     {
-      tree x;
       tt = fold_build2 (EQ_EXPR, boolean_type_node, unshare_expr (op1),
 			build_int_cst (type, -1));
       x = fold_build2 (EQ_EXPR, boolean_type_node, op0,
 		       TYPE_MIN_VALUE (type));
       x = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, x, tt);
-      t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, x);
+      if (t == NULL_TREE || integer_zerop (t))
+	{
+	  t = x;
+	  x = NULL_TREE;
+	  flag = SANITIZE_SI_OVERFLOW;
+	}
+      else if (flag_sanitize_undefined_trap_on_error
+	       || (((flag_sanitize_recover & SANITIZE_DIVIDE) == 0)
+		   == ((flag_sanitize_recover & SANITIZE_SI_OVERFLOW) == 0)))
+	{
+	  t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, x);
+	  x = NULL_TREE;
+	}
+      else if (integer_zerop (x))
+	x = NULL_TREE;
     }
+  else if (t == NULL_TREE)
+    return NULL_TREE;
 
   /* If the condition was folded to 0, no need to instrument
      this expression.  */
@@ -95,7 +114,7 @@ ubsan_instrument_division (location_t loc, tree op0, tree op1)
 				     NULL_TREE);
       data = build_fold_addr_expr_loc (loc, data);
       enum built_in_function bcode
-	= (flag_sanitize_recover & SANITIZE_DIVIDE)
+	= (flag_sanitize_recover & flag)
 	  ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW
 	  : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT;
       tt = builtin_decl_explicit (bcode);
@@ -103,8 +122,20 @@ ubsan_instrument_division (location_t loc, tree op0, tree op1)
       op1 = unshare_expr (op1);
       tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0),
 				ubsan_encode_value (op1));
+      if (x)
+	{
+	  bcode = (flag_sanitize_recover & SANITIZE_SI_OVERFLOW)
+		  ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW
+		  : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT;
+	  tree xt = builtin_decl_explicit (bcode);
+	  op0 = unshare_expr (op0);
+	  op1 = unshare_expr (op1);
+	  xt = build_call_expr_loc (loc, xt, 3, data, ubsan_encode_value (op0),
+				    ubsan_encode_value (op1));
+	  x = fold_build3 (COND_EXPR, void_type_node, x, xt, void_node);
+	}
     }
-  t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_node);
+  t = fold_build3 (COND_EXPR, void_type_node, t, tt, x ? x : void_node);
 
   return t;
 }