ubsan: Move INT_MIN / -1 instrumentation from -fsanitize=integer-divide-by-zero to...

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.

diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c
index 12a7bca5c32eae9f9921ab653dabf9f234042758..a4509c68c16d4e22259a8a20d2b267082c732421 100644 (file)
--- 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;
 }

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index b472e448011ef11878763b0294cafd1d7f6e6da8..c74f876e6674f5bb225b2a136affa433ff2421a7 100644 (file)
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -12733,7 +12733,9 @@ build_binary_op (location_t location, enum tree_code code,
     }
 
   if (sanitize_flags_p ((SANITIZE_SHIFT
-                        | SANITIZE_DIVIDE | SANITIZE_FLOAT_DIVIDE))
+                        | SANITIZE_DIVIDE
+                        | SANITIZE_FLOAT_DIVIDE
+                        | SANITIZE_SI_OVERFLOW))
       && current_function_decl != NULL_TREE
       && (doing_div_or_mod || doing_shift)
       && !require_constant_value)
@@ -12744,7 +12746,8 @@ build_binary_op (location_t location, enum tree_code code,
       op0 = c_fully_fold (op0, false, NULL);
       op1 = c_fully_fold (op1, false, NULL);
       if (doing_div_or_mod && (sanitize_flags_p ((SANITIZE_DIVIDE
-                                                 | SANITIZE_FLOAT_DIVIDE))))
+                                                 | SANITIZE_FLOAT_DIVIDE
+                                                 | SANITIZE_SI_OVERFLOW))))
        instrument_expr = ubsan_instrument_division (location, op0, op1);
       else if (doing_shift && sanitize_flags_p (SANITIZE_SHIFT))
        instrument_expr = ubsan_instrument_shift (location, code, op0, op1);

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index a2398dbe660c004cdd635cac3f61f9d88c3ed6e5..cd130f16a66b001fe4dd339b0f3da7a1219e9d9f 100644 (file)
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6038,7 +6038,9 @@ cp_build_binary_op (const op_location_t &location,
     }
 
   if (sanitize_flags_p ((SANITIZE_SHIFT
-                        | SANITIZE_DIVIDE | SANITIZE_FLOAT_DIVIDE))
+                        | SANITIZE_DIVIDE
+                        | SANITIZE_FLOAT_DIVIDE
+                        | SANITIZE_SI_OVERFLOW))
       && current_function_decl != NULL_TREE
       && !processing_template_decl
       && (doing_div_or_mod || doing_shift))
@@ -6050,7 +6052,9 @@ cp_build_binary_op (const op_location_t &location,
       op1 = fold_non_dependent_expr (op1, complain);
       tree instrument_expr1 = NULL_TREE;
       if (doing_div_or_mod
-         && sanitize_flags_p (SANITIZE_DIVIDE | SANITIZE_FLOAT_DIVIDE))
+         && sanitize_flags_p (SANITIZE_DIVIDE
+                              | SANITIZE_FLOAT_DIVIDE
+                              | SANITIZE_SI_OVERFLOW))
        {
          /* For diagnostics we want to use the promoted types without
             shorten_binary_op.  So convert the arguments to the

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d0198d77434d1041042b5bf6fdda9ad84dfee29b..5f39b208049bbfdcc06e75d291a8dcfe2155f624 100644 (file)
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15229,7 +15229,7 @@ ISO C90 and C99, etc.
 
 @item -fsanitize=integer-divide-by-zero
 @opindex fsanitize=integer-divide-by-zero
-Detect integer division by zero as well as @code{INT_MIN / -1} division.
+Detect integer division by zero.
 
 @item -fsanitize=unreachable
 @opindex fsanitize=unreachable
@@ -15261,7 +15261,8 @@ returning a value.  This option works in C++ only.
 @opindex fsanitize=signed-integer-overflow
 This option enables signed integer overflow checking.  We check that
 the result of @code{+}, @code{*}, and both unary and binary @code{-}
-does not overflow in the signed arithmetics.  Note, integer promotion
+does not overflow in the signed arithmetics.  This also detects
+@code{INT_MIN / -1} signed division.  Note, integer promotion
 rules must be taken into account.  That is, the following is not an
 overflow:
 @smallexample

diff --git a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-3.c b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-3.c
index 266423aa49ffbe170a58d016037188d93f911498..1d51e3573e76505b8d45d73a53ca5bed5c664fdd 100644 (file)
--- a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-3.c
+++ b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-3.c
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-options "-fsanitize=integer-divide-by-zero -Wno-overflow" } */
+/* { dg-options "-fsanitize=signed-integer-overflow -Wno-overflow" } */
 
 #include <limits.h>
 

diff --git a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c
index 02162e139e30518d71bcd9b282eca3afdeab2733..ef431c93e4e2f4c842f7ca6e4a7e53d81573730a 100644 (file)
--- a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c
+++ b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c
@@ -1,5 +1,5 @@
 /* { dg-do run } */
-/* { dg-options "-fsanitize=integer-divide-by-zero -Wno-overflow" } */
+/* { dg-options "-fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error -Wno-overflow" } */
 
 #define INT_MIN (-__INT_MAX__ - 1)
 

diff --git a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c
index bb391c5b36df0f2bdc863ff149295e462ae633e9..853a3dc50035e9947005f8683cd08e1f39677065 100644 (file)
--- a/gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c
+++ b/gcc/testsuite/c-c++-common/ubsan/div-by-zero-5.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-fsanitize=integer-divide-by-zero" } */
+/* { dg-options "-fsanitize=signed-integer-overflow" } */
 
 void
 foo (void)

diff --git a/gcc/testsuite/c-c++-common/ubsan/float-div-by-zero-2.c b/gcc/testsuite/c-c++-common/ubsan/float-div-by-zero-2.c
new file mode 100644 (file)
index 0000000..61d050a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/float-div-by-zero-2.c
@@ -0,0 +1,18 @@
+/* { dg-do run } */
+/* { dg-shouldfail "ubsan" } */
+/* { dg-options "-fsanitize=float-divide-by-zero -fno-sanitize-recover=float-divide-by-zero -fsanitize-recover=integer-divide-by-zero" } */
+
+int
+main (void)
+{
+  volatile float a = 1.3f;
+  volatile double b = 0.0;
+  volatile int c = 4;
+  volatile float res;
+
+  res = a / b;
+
+  return 0;
+}
+
+/* { dg-output "division by zero" } */

diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-div-1.c b/gcc/testsuite/c-c++-common/ubsan/overflow-div-1.c
new file mode 100644 (file)
index 0000000..d781660
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/overflow-div-1.c
@@ -0,0 +1,17 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=integer-divide-by-zero -fno-sanitize-recover=undefined,float-divide-by-zero -Wno-overflow" } */
+
+#include <limits.h>
+
+int
+main (void)
+{
+  volatile int min = INT_MIN;
+  volatile int zero = 0;
+
+  INT_MIN / -1;
+  min / -1;
+  min / (10 * zero - (2 - 1));
+
+  return 0;
+}

diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-div-2.c b/gcc/testsuite/c-c++-common/ubsan/overflow-div-2.c
new file mode 100644 (file)
index 0000000..dfef1b0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/overflow-div-2.c
@@ -0,0 +1,41 @@
+/* { dg-do run { target *-*-linux* *-*-gnu* } } */
+/* { dg-shouldfail "ubsan" } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=integer-divide-by-zero" } */
+
+#include <limits.h>
+#include <signal.h>
+#include <stdlib.h>
+
+int cnt;
+
+__attribute__((noipa)) int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+void
+handler (int i)
+{
+  if (cnt++ != 0)
+    exit (0);
+  volatile int b = foo (5, 0);
+  exit (0);
+}
+
+int
+main (void)
+{
+  struct sigaction s;
+  sigemptyset (&s.sa_mask);
+  s.sa_handler = handler;
+  s.sa_flags = 0;
+  sigaction (SIGFPE, &s, NULL);
+  volatile int a = foo (INT_MIN, -1);
+  cnt++;
+  volatile int b = foo (5, 0);
+  return 0;
+}
+
+/* { dg-output "division of -2147483648 by -1 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*division by zero\[^\n\r]*" } */

diff --git a/gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c b/gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c
new file mode 100644 (file)
index 0000000..479dffb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/overflow-div-3.c
@@ -0,0 +1,41 @@
+/* { dg-do run { target *-*-linux* *-*-gnu* } } */
+/* { dg-shouldfail "ubsan" } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=signed-integer-overflow" } */
+
+#include <limits.h>
+#include <signal.h>
+#include <stdlib.h>
+
+int cnt;
+
+__attribute__((noipa)) int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+void
+handler (int i)
+{
+  if (cnt++ != 0)
+    exit (0);
+  volatile int b = foo (INT_MIN, -1);
+  exit (0);
+}
+
+int
+main (void)
+{
+  struct sigaction s;
+  sigemptyset (&s.sa_mask);
+  s.sa_handler = handler;
+  s.sa_flags = 0;
+  sigaction (SIGFPE, &s, NULL);
+  volatile int a = foo (42, 0);
+  cnt++;
+  volatile int b = foo (INT_MIN, -1);
+  return 0;
+}
+
+/* { dg-output "division by zero\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*division of -2147483648 by -1 cannot be represented in type 'int'" } */