Patchwork Fix ubsan expansion (PR sanitizer/60613)

login
register
mail settings
Submitter Jakub Jelinek
Date March 21, 2014, 8:32 p.m.
Message ID <20140321203254.GW1817@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/332742/
State New
Headers show

Comments

Jakub Jelinek - March 21, 2014, 8:32 p.m.
Hi!

As MINUS_EXPR is not commutative, we really can't swap op0 with op1
for testing whether subtraction overflowed, that is only possible for
PLUS_EXPR, for MINUS_EXPR we really have to know if op1 is constant
or negative or non-negative and have to compare result with op0 depending on
that.

Bootstrapped/regtested on x86_64-linux and i686-linux, i686-linux
extra --with-build-config=bootstrap-ubsan bootstrap ongoing.

Ok for trunk?

2014-03-21  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/60613
	* interna-fn.c (ubsan_expand_si_overflow_addsub_check): For
	code == MINUS_EXPR, never swap op0 with op1.

	* c-c++-common/ubsan/pr60613-1.c: New test.
	* c-c++-common/ubsan/pr60613-2.c: New test.


	Jakub
Richard Guenther - March 22, 2014, 12:52 p.m.
On March 21, 2014 9:32:54 PM CET, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>As MINUS_EXPR is not commutative, we really can't swap op0 with op1
>for testing whether subtraction overflowed, that is only possible for
>PLUS_EXPR, for MINUS_EXPR we really have to know if op1 is constant
>or negative or non-negative and have to compare result with op0
>depending on
>that.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, i686-linux
>extra --with-build-config=bootstrap-ubsan bootstrap ongoing.
>
>Ok for trunk?

Ok,
Thanks,
Richard.

>2014-03-21  Jakub Jelinek  <jakub@redhat.com>
>
>	PR sanitizer/60613
>	* interna-fn.c (ubsan_expand_si_overflow_addsub_check): For
>	code == MINUS_EXPR, never swap op0 with op1.
>
>	* c-c++-common/ubsan/pr60613-1.c: New test.
>	* c-c++-common/ubsan/pr60613-2.c: New test.
>
>--- gcc/internal-fn.c.jj	2014-03-18 12:27:10.000000000 +0100
>+++ gcc/internal-fn.c	2014-03-21 15:41:39.116303973 +0100
>@@ -221,14 +221,15 @@ ubsan_expand_si_overflow_addsub_check (t
>   res = expand_binop (mode, code == PLUS_EXPR ? add_optab : sub_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
>+      /* If we can prove one of the arguments (for MINUS_EXPR only
>+	 the second operand, as subtraction is not commutative) 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))
>+      if (code == PLUS_EXPR && CONST_INT_P (op0))
> 	{
> 	  rtx tem = op0;
> 	  op0 = op1;
>@@ -236,7 +237,7 @@ ubsan_expand_si_overflow_addsub_check (t
> 	}
>       else if (CONST_INT_P (op1))
> 	;
>-      else if (TREE_CODE (arg0) == SSA_NAME)
>+      else if (code == PLUS_EXPR && TREE_CODE (arg0) == SSA_NAME)
> 	{
> 	  double_int arg0_min, arg0_max;
> 	  if (get_range_info (arg0, &arg0_min, &arg0_max) == VR_RANGE)
>--- gcc/testsuite/c-c++-common/ubsan/pr60613-1.c.jj	2014-03-21
>16:00:47.930272534 +0100
>+++ gcc/testsuite/c-c++-common/ubsan/pr60613-1.c	2014-03-21
>15:47:50.000000000 +0100
>@@ -0,0 +1,33 @@
>+/* PR sanitizer/60613 */
>+/* { dg-do run } */
>+/* { dg-options "-fsanitize=undefined" } */
>+
>+long long y;
>+
>+__attribute__((noinline, noclone)) long long
>+foo (long long x)
>+{
>+  asm ("");
>+  if (x >= 0 || x < -2040)
>+    return 23;
>+  x += 2040;
>+  return x - y;
>+}
>+
>+__attribute__((noinline, noclone)) long long
>+bar (long long x)
>+{
>+  asm ("");
>+  return 8LL - x;
>+}
>+
>+int
>+main ()
>+{
>+  y = 1;
>+  if (foo (8 - 2040) != 8 - 1)
>+    __builtin_abort ();
>+  if (bar (1) != 8 - 1)
>+    __builtin_abort ();
>+  return 0;
>+}
>--- gcc/testsuite/c-c++-common/ubsan/pr60613-2.c.jj	2014-03-21
>16:00:50.795259403 +0100
>+++ gcc/testsuite/c-c++-common/ubsan/pr60613-2.c	2014-03-21
>16:08:56.915733544 +0100
>@@ -0,0 +1,36 @@
>+/* PR sanitizer/60613 */
>+/* { dg-do run } */
>+/* { dg-options "-fsanitize=undefined" } */
>+
>+long long y;
>+
>+__attribute__((noinline, noclone)) long long
>+foo (long long x)
>+{
>+  asm ("");
>+  if (x >= 0 || x < -2040)
>+    return 23;
>+  x += 2040;
>+  return x - y;
>+}
>+
>+__attribute__((noinline, noclone)) long long
>+bar (long long x)
>+{
>+  asm ("");
>+  return 8LL - x;
>+}
>+
>+int
>+main ()
>+{
>+  y = -__LONG_LONG_MAX__ + 6;
>+  if (foo (8 - 2040) != -__LONG_LONG_MAX__)
>+    __builtin_abort ();
>+  if (bar (-__LONG_LONG_MAX__ + 5) != -__LONG_LONG_MAX__ + 1)
>+    __builtin_abort ();
>+  return 0;
>+}
>+
>+/* { dg-output "signed integer overflow: 8 \\- -9223372036854775801
>cannot be represented in type 'long long int'(\n|\r\n|\r)" } */
>+/* { dg-output "\[^\n\r]*signed integer overflow: 8 \\-
>-9223372036854775802 cannot be represented in type 'long long int'" }
>*/
>
>	Jakub

Patch

--- gcc/internal-fn.c.jj	2014-03-18 12:27:10.000000000 +0100
+++ gcc/internal-fn.c	2014-03-21 15:41:39.116303973 +0100
@@ -221,14 +221,15 @@  ubsan_expand_si_overflow_addsub_check (t
       res = expand_binop (mode, code == PLUS_EXPR ? add_optab : sub_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
+      /* If we can prove one of the arguments (for MINUS_EXPR only
+	 the second operand, as subtraction is not commutative) 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))
+      if (code == PLUS_EXPR && CONST_INT_P (op0))
 	{
 	  rtx tem = op0;
 	  op0 = op1;
@@ -236,7 +237,7 @@  ubsan_expand_si_overflow_addsub_check (t
 	}
       else if (CONST_INT_P (op1))
 	;
-      else if (TREE_CODE (arg0) == SSA_NAME)
+      else if (code == PLUS_EXPR && TREE_CODE (arg0) == SSA_NAME)
 	{
 	  double_int arg0_min, arg0_max;
 	  if (get_range_info (arg0, &arg0_min, &arg0_max) == VR_RANGE)
--- gcc/testsuite/c-c++-common/ubsan/pr60613-1.c.jj	2014-03-21 16:00:47.930272534 +0100
+++ gcc/testsuite/c-c++-common/ubsan/pr60613-1.c	2014-03-21 15:47:50.000000000 +0100
@@ -0,0 +1,33 @@ 
+/* PR sanitizer/60613 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined" } */
+
+long long y;
+
+__attribute__((noinline, noclone)) long long
+foo (long long x)
+{
+  asm ("");
+  if (x >= 0 || x < -2040)
+    return 23;
+  x += 2040;
+  return x - y;
+}
+
+__attribute__((noinline, noclone)) long long
+bar (long long x)
+{
+  asm ("");
+  return 8LL - x;
+}
+
+int
+main ()
+{
+  y = 1;
+  if (foo (8 - 2040) != 8 - 1)
+    __builtin_abort ();
+  if (bar (1) != 8 - 1)
+    __builtin_abort ();
+  return 0;
+}
--- gcc/testsuite/c-c++-common/ubsan/pr60613-2.c.jj	2014-03-21 16:00:50.795259403 +0100
+++ gcc/testsuite/c-c++-common/ubsan/pr60613-2.c	2014-03-21 16:08:56.915733544 +0100
@@ -0,0 +1,36 @@ 
+/* PR sanitizer/60613 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined" } */
+
+long long y;
+
+__attribute__((noinline, noclone)) long long
+foo (long long x)
+{
+  asm ("");
+  if (x >= 0 || x < -2040)
+    return 23;
+  x += 2040;
+  return x - y;
+}
+
+__attribute__((noinline, noclone)) long long
+bar (long long x)
+{
+  asm ("");
+  return 8LL - x;
+}
+
+int
+main ()
+{
+  y = -__LONG_LONG_MAX__ + 6;
+  if (foo (8 - 2040) != -__LONG_LONG_MAX__)
+    __builtin_abort ();
+  if (bar (-__LONG_LONG_MAX__ + 5) != -__LONG_LONG_MAX__ + 1)
+    __builtin_abort ();
+  return 0;
+}
+
+/* { dg-output "signed integer overflow: 8 \\- -9223372036854775801 cannot be represented in type 'long long int'(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*signed integer overflow: 8 \\- -9223372036854775802 cannot be represented in type 'long long int'" } */