diff mbox series

c++: Handle COMPOUND_EXPRs in get_narrower and warnings_for_convert_and_check [PR91993]

Message ID 20200320083113.GU2156@tucnak
State New
Headers show
Series c++: Handle COMPOUND_EXPRs in get_narrower and warnings_for_convert_and_check [PR91993] | expand

Commit Message

Jeff Law via Gcc-patches March 20, 2020, 8:31 a.m. UTC
On Wed, Mar 18, 2020 at 05:37:26PM -0400, Jason Merrill via Gcc-patches wrote:
> How about improving get_narrower to handle COMPOUND_EXPR?  I'd think that
> would do the trick without affecting evaluation order.

Not completely, had to change also warnings_for_convert_and_check, but with
that it works.  The float-cast-overflow* changes are needed because now with
-O1+ we emit lots of -Woverflow warnings on the testcase, but we do emit
those warnings on the testcase even when compiling just with -O1 and without
-fsanitize=float-cast-overflow, so it seems to me a change in the right
direction, having -fsanitize= or explicit comma expressions smaller effect
on the warnings that are emitted.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-03-20  Jakub Jelinek  <jakub@redhat.com>

	PR c++/91993
	* tree.c (get_narrower): Handle COMPOUND_EXPR by recursing on
	ultimate rhs and if returned something different, reconstructing
	the COMPOUND_EXPRs.

	* c-warn.c (warnings_for_convert_and_check): For expr and/or
	result being COMPOUND_EXPRs, skip to ultimate rhs.

	* g++.dg/warn/Wconversion-pr91993.C: New test.
	* g++.dg/ubsan/pr91993.C: New test.
	* c-c++-common/ubsan/float-cast-overflow-1.c: Add -Wno-overflow
	to dg-options.
	* c-c++-common/ubsan/float-cast-overflow-2.c: Likewise.
	* c-c++-common/ubsan/float-cast-overflow-4.c: Likewise.



	Jakub

Comments

Jeff Law via Gcc-patches March 20, 2020, 5:36 p.m. UTC | #1
On 3/20/20 4:31 AM, Jakub Jelinek wrote:
> On Wed, Mar 18, 2020 at 05:37:26PM -0400, Jason Merrill via Gcc-patches wrote:
>> How about improving get_narrower to handle COMPOUND_EXPR?  I'd think that
>> would do the trick without affecting evaluation order.
> 
> Not completely, had to change also warnings_for_convert_and_check, but with
> that it works.  The float-cast-overflow* changes are needed because now with
> -O1+ we emit lots of -Woverflow warnings on the testcase, but we do emit
> those warnings on the testcase even when compiling just with -O1 and without
> -fsanitize=float-cast-overflow, so it seems to me a change in the right
> direction, having -fsanitize= or explicit comma expressions smaller effect
> on the warnings that are emitted.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

LGTM, unless any middle-end folks want to weigh in?

> 2020-03-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/91993
> 	* tree.c (get_narrower): Handle COMPOUND_EXPR by recursing on
> 	ultimate rhs and if returned something different, reconstructing
> 	the COMPOUND_EXPRs.

> 	* c-warn.c (warnings_for_convert_and_check): For expr and/or
> 	result being COMPOUND_EXPRs, skip to ultimate rhs.
> 
> 	* g++.dg/warn/Wconversion-pr91993.C: New test.
> 	* g++.dg/ubsan/pr91993.C: New test.
> 	* c-c++-common/ubsan/float-cast-overflow-1.c: Add -Wno-overflow
> 	to dg-options.
> 	* c-c++-common/ubsan/float-cast-overflow-2.c: Likewise.
> 	* c-c++-common/ubsan/float-cast-overflow-4.c: Likewise.
> 
> --- gcc/tree.c.jj	2020-03-17 13:50:52.382941623 +0100
> +++ gcc/tree.c	2020-03-19 18:43:18.405499531 +0100
> @@ -8862,6 +8862,21 @@ get_narrower (tree op, int *unsignedp_pt
>     tree win = op;
>     bool integral_p = INTEGRAL_TYPE_P (TREE_TYPE (op));
>   
> +  if (TREE_CODE (op) == COMPOUND_EXPR)
> +    {
> +      while (TREE_CODE (op) == COMPOUND_EXPR)
> +	op = TREE_OPERAND (op, 1);
> +      tree ret = get_narrower (op, unsignedp_ptr);
> +      if (ret == op)
> +	return win;
> +      op = win;
> +      for (tree *p = &win; TREE_CODE (op) == COMPOUND_EXPR;
> +	   op = TREE_OPERAND (op, 1), p = &TREE_OPERAND (*p, 1))
> +	*p = build2_loc (EXPR_LOCATION (op), COMPOUND_EXPR,
> +			 TREE_TYPE (ret), TREE_OPERAND (op, 0),
> +			 ret);
> +      return win;
> +    }
>     while (TREE_CODE (op) == NOP_EXPR)
>       {
>         int bitschange
> --- gcc/c-family/c-warn.c.jj	2020-01-23 19:17:24.072286895 +0100
> +++ gcc/c-family/c-warn.c	2020-03-19 19:21:43.389730189 +0100
> @@ -1359,6 +1359,11 @@ warnings_for_convert_and_check (location
>   {
>     loc = expansion_point_location_if_in_system_header (loc);
>   
> +  while (TREE_CODE (expr) == COMPOUND_EXPR)
> +    expr = TREE_OPERAND (expr, 1);
> +  while (TREE_CODE (result) == COMPOUND_EXPR)
> +    result = TREE_OPERAND (result, 1);
> +
>     bool cst = TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant;
>   
>     tree exprtype = TREE_TYPE (expr);
> --- gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C.jj	2020-03-19 18:21:02.635049452 +0100
> +++ gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C	2020-03-19 18:21:02.635049452 +0100
> @@ -0,0 +1,17 @@
> +// PR c++/91993
> +// { dg-do compile }
> +// { dg-options "-Wconversion" }
> +
> +typedef unsigned char uc;
> +
> +int
> +foo (const uc &a, const uc &b, const uc &c)
> +{
> +  return static_cast<uc>(static_cast<uc>(a << 1U) | b) | c;		// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
> +}
> +
> +int
> +bar (const uc &a, const uc &b, const uc &c, int &d)
> +{
> +  return static_cast<uc>(static_cast<uc>((d++, a) << 1U) | b) | c;	// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
> +}
> --- gcc/testsuite/g++.dg/ubsan/pr91993.C.jj	2020-03-19 18:21:02.635049452 +0100
> +++ gcc/testsuite/g++.dg/ubsan/pr91993.C	2020-03-19 18:21:02.635049452 +0100
> @@ -0,0 +1,17 @@
> +// PR c++/91993
> +// { dg-do compile }
> +// { dg-options "-Wconversion -fsanitize=undefined" }
> +
> +typedef unsigned char uc;
> +
> +int
> +foo (const uc &a, const uc &b, const uc &c)
> +{
> +  return static_cast<uc>(static_cast<uc>(a << 1U) | b) | c;		// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
> +}
> +
> +int
> +bar (const uc &a, const uc &b, const uc &c, int &d)
> +{
> +  return static_cast<uc>(static_cast<uc>((d++, a) << 1U) | b) | c;	// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
> +}
> --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c.jj	2020-01-12 11:54:37.029404115 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c	2020-03-20 00:40:57.403648603 +0100
> @@ -1,5 +1,5 @@
>   /* { dg-do run { target { lp64 || ilp32 } } } */
> -/* { dg-options "-fsanitize=float-cast-overflow" } */
> +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */
>   /* { dg-additional-options "-ffloat-store" { target { ia32 } } } */
>   /* { dg-additional-options "-mieee" { target { { alpha*-*-* } || { sh*-*-* } } } } */
>   
> --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c.jj	2020-01-12 11:54:37.029404115 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c	2020-03-20 00:41:21.757289152 +0100
> @@ -1,6 +1,6 @@
>   /* { dg-do run } */
>   /* { dg-require-effective-target int128 } */
> -/* { dg-options "-fsanitize=float-cast-overflow" } */
> +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */
>   
>   #include "float-cast.h"
>   
> --- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c.jj	2020-01-12 11:54:37.029404115 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c	2020-03-20 00:41:32.053137185 +0100
> @@ -1,5 +1,5 @@
>   /* { dg-do run { target { lp64 } } } */
> -/* { dg-options "-fsanitize=float-cast-overflow" } */
> +/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */
>   
>   #include <limits.h>
>   #include "float-cast.h"
> 
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/tree.c.jj	2020-03-17 13:50:52.382941623 +0100
+++ gcc/tree.c	2020-03-19 18:43:18.405499531 +0100
@@ -8862,6 +8862,21 @@  get_narrower (tree op, int *unsignedp_pt
   tree win = op;
   bool integral_p = INTEGRAL_TYPE_P (TREE_TYPE (op));
 
+  if (TREE_CODE (op) == COMPOUND_EXPR)
+    {
+      while (TREE_CODE (op) == COMPOUND_EXPR)
+	op = TREE_OPERAND (op, 1);
+      tree ret = get_narrower (op, unsignedp_ptr);
+      if (ret == op)
+	return win;
+      op = win;
+      for (tree *p = &win; TREE_CODE (op) == COMPOUND_EXPR;
+	   op = TREE_OPERAND (op, 1), p = &TREE_OPERAND (*p, 1))
+	*p = build2_loc (EXPR_LOCATION (op), COMPOUND_EXPR,
+			 TREE_TYPE (ret), TREE_OPERAND (op, 0),
+			 ret);
+      return win;
+    }
   while (TREE_CODE (op) == NOP_EXPR)
     {
       int bitschange
--- gcc/c-family/c-warn.c.jj	2020-01-23 19:17:24.072286895 +0100
+++ gcc/c-family/c-warn.c	2020-03-19 19:21:43.389730189 +0100
@@ -1359,6 +1359,11 @@  warnings_for_convert_and_check (location
 {
   loc = expansion_point_location_if_in_system_header (loc);
 
+  while (TREE_CODE (expr) == COMPOUND_EXPR)
+    expr = TREE_OPERAND (expr, 1);
+  while (TREE_CODE (result) == COMPOUND_EXPR)
+    result = TREE_OPERAND (result, 1);
+
   bool cst = TREE_CODE_CLASS (TREE_CODE (result)) == tcc_constant;
 
   tree exprtype = TREE_TYPE (expr);
--- gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C.jj	2020-03-19 18:21:02.635049452 +0100
+++ gcc/testsuite/g++.dg/warn/Wconversion-pr91993.C	2020-03-19 18:21:02.635049452 +0100
@@ -0,0 +1,17 @@ 
+// PR c++/91993
+// { dg-do compile }
+// { dg-options "-Wconversion" }
+
+typedef unsigned char uc;
+
+int
+foo (const uc &a, const uc &b, const uc &c)
+{
+  return static_cast<uc>(static_cast<uc>(a << 1U) | b) | c;		// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
+}
+
+int
+bar (const uc &a, const uc &b, const uc &c, int &d)
+{
+  return static_cast<uc>(static_cast<uc>((d++, a) << 1U) | b) | c;	// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
+}
--- gcc/testsuite/g++.dg/ubsan/pr91993.C.jj	2020-03-19 18:21:02.635049452 +0100
+++ gcc/testsuite/g++.dg/ubsan/pr91993.C	2020-03-19 18:21:02.635049452 +0100
@@ -0,0 +1,17 @@ 
+// PR c++/91993
+// { dg-do compile }
+// { dg-options "-Wconversion -fsanitize=undefined" }
+
+typedef unsigned char uc;
+
+int
+foo (const uc &a, const uc &b, const uc &c)
+{
+  return static_cast<uc>(static_cast<uc>(a << 1U) | b) | c;		// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
+}
+
+int
+bar (const uc &a, const uc &b, const uc &c, int &d)
+{
+  return static_cast<uc>(static_cast<uc>((d++, a) << 1U) | b) | c;	// { dg-bogus "conversion from 'int' to 'unsigned char' may change value" }
+}
--- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c.jj	2020-01-12 11:54:37.029404115 +0100
+++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-1.c	2020-03-20 00:40:57.403648603 +0100
@@ -1,5 +1,5 @@ 
 /* { dg-do run { target { lp64 || ilp32 } } } */
-/* { dg-options "-fsanitize=float-cast-overflow" } */
+/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */
 /* { dg-additional-options "-ffloat-store" { target { ia32 } } } */
 /* { dg-additional-options "-mieee" { target { { alpha*-*-* } || { sh*-*-* } } } } */
 
--- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c.jj	2020-01-12 11:54:37.029404115 +0100
+++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-2.c	2020-03-20 00:41:21.757289152 +0100
@@ -1,6 +1,6 @@ 
 /* { dg-do run } */
 /* { dg-require-effective-target int128 } */
-/* { dg-options "-fsanitize=float-cast-overflow" } */
+/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */
 
 #include "float-cast.h"
 
--- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c.jj	2020-01-12 11:54:37.029404115 +0100
+++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-4.c	2020-03-20 00:41:32.053137185 +0100
@@ -1,5 +1,5 @@ 
 /* { dg-do run { target { lp64 } } } */
-/* { dg-options "-fsanitize=float-cast-overflow" } */
+/* { dg-options "-fsanitize=float-cast-overflow -Wno-overflow" } */
 
 #include <limits.h>
 #include "float-cast.h"