convert: Fix test for out of bounds shift count [PR113574]

Message ID ZbIYp9p5+xUM8lb8@tucnak
State Unresolved
Headers
Series convert: Fix test for out of bounds shift count [PR113574] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Jan. 25, 2024, 8:15 a.m. UTC
  Hi!

The following patch is miscompiled, because convert_to_integer_1 for
LSHIFT_EXPR tests if the INTEGER_CST shift count is too high, but
incorrectly compares it against TYPE_SIZE rather than TYPE_PRECISION.
The type in question is unsigned _BitInt(1), which has TYPE_PRECISION 1,
TYPE_SIZE 8, and the shift count is 2 in that case.

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

2024-01-25  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/113574
	* convert.cc (convert_to_integer_1) <case LSHIFT_EXPR>: Compare shift
	count against TYPE_PRECISION rather than TYPE_SIZE.

	* gcc.dg/torture/bitint-52.c: New test.


	Jakub
  

Comments

Richard Biener Jan. 25, 2024, 9:15 a.m. UTC | #1
On Thu, 25 Jan 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following patch is miscompiled, because convert_to_integer_1 for
> LSHIFT_EXPR tests if the INTEGER_CST shift count is too high, but
> incorrectly compares it against TYPE_SIZE rather than TYPE_PRECISION.
> The type in question is unsigned _BitInt(1), which has TYPE_PRECISION 1,
> TYPE_SIZE 8, and the shift count is 2 in that case.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2024-01-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/113574
> 	* convert.cc (convert_to_integer_1) <case LSHIFT_EXPR>: Compare shift
> 	count against TYPE_PRECISION rather than TYPE_SIZE.
> 
> 	* gcc.dg/torture/bitint-52.c: New test.
> 
> --- gcc/convert.cc.jj	2024-01-03 11:51:24.000000000 +0100
> +++ gcc/convert.cc	2024-01-24 17:29:56.328124611 +0100
> @@ -762,7 +762,8 @@ convert_to_integer_1 (tree type, tree ex
>  	      {
>  		/* If shift count is less than the width of the truncated type,
>  		   really shift.  */
> -		if (tree_int_cst_lt (TREE_OPERAND (expr, 1), TYPE_SIZE (type)))
> +		if (wi::to_widest (TREE_OPERAND (expr, 1))
> +		    < TYPE_PRECISION (type))
>  		  /* In this case, shifting is like multiplication.  */
>  		  goto trunc1;
>  		else
> --- gcc/testsuite/gcc.dg/torture/bitint-52.c.jj	2024-01-24 17:33:42.174986825 +0100
> +++ gcc/testsuite/gcc.dg/torture/bitint-52.c	2024-01-24 17:33:33.395108814 +0100
> @@ -0,0 +1,23 @@
> +/* PR middle-end/113574 */
> +/* { dg-do run { target bitint } } */
> +/* { dg-options "-std=c23 -pedantic-errors" } */
> +/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
> +/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
> +
> +unsigned _BitInt(1) a;
> +unsigned _BitInt(8) b;
> +
> +void
> +foo (unsigned _BitInt(16) x)
> +{
> +  a += (x << 2) | b;
> +}
> +
> +int
> +main ()
> +{
> +  foo (0xfef1uwb);
> +  if (a)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>
  

Patch

--- gcc/convert.cc.jj	2024-01-03 11:51:24.000000000 +0100
+++ gcc/convert.cc	2024-01-24 17:29:56.328124611 +0100
@@ -762,7 +762,8 @@  convert_to_integer_1 (tree type, tree ex
 	      {
 		/* If shift count is less than the width of the truncated type,
 		   really shift.  */
-		if (tree_int_cst_lt (TREE_OPERAND (expr, 1), TYPE_SIZE (type)))
+		if (wi::to_widest (TREE_OPERAND (expr, 1))
+		    < TYPE_PRECISION (type))
 		  /* In this case, shifting is like multiplication.  */
 		  goto trunc1;
 		else
--- gcc/testsuite/gcc.dg/torture/bitint-52.c.jj	2024-01-24 17:33:42.174986825 +0100
+++ gcc/testsuite/gcc.dg/torture/bitint-52.c	2024-01-24 17:33:33.395108814 +0100
@@ -0,0 +1,23 @@ 
+/* PR middle-end/113574 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
+/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+unsigned _BitInt(1) a;
+unsigned _BitInt(8) b;
+
+void
+foo (unsigned _BitInt(16) x)
+{
+  a += (x << 2) | b;
+}
+
+int
+main ()
+{
+  foo (0xfef1uwb);
+  if (a)
+    __builtin_abort ();
+  return 0;
+}