diff mbox series

Fix gimple-ssa-sprintf.c ICE (PR tree-optimization/83198, take 2)

Message ID 20171214090528.GP2353@tucnak
State New
Headers show
Series Fix gimple-ssa-sprintf.c ICE (PR tree-optimization/83198, take 2) | expand

Commit Message

Jakub Jelinek Dec. 14, 2017, 9:05 a.m. UTC
On Wed, Dec 13, 2017 at 03:55:13PM -0700, Martin Sebor wrote:
> For the second part, can you please also add a compile-time test
> to verify that the result isn't constrained to the same range as
> with a real argument?  Checking that the abort below isn't
> eliminated would do it for %f:
> 
>   void f (char *d)
>   {
>     int n = __builtin_sprintf (d, "%f", 1.0Q);
>     if (n < 8 || 13 < n)
>       __builtin_abort ();
>   }
> 
> A test that has a convenient setup for this is tree-ssa/builtin-
> sprintf-2.c in case you want to add to it.

Can't use that test, because I need __float128, add options for it etc.

Anyway, here is the same patch with added extra test, tested on x86_64-linux
and powerpc64-linux.  The tree-ssa test FAILs without the patch on both
targets.

2017-12-14  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/83198
	* gimple-ssa-sprintf.c (format_floating): Set type solely based on
	dir.modifier, regardless of TREE_TYPE (arg).  Assume non-REAL_CST
	value if arg is a REAL_CST with incompatible type.

	* gcc.dg/pr83198.c: New test.
	* gcc.dg/tree-ssa/pr83198.c: New test.



	Jakub

Comments

Richard Biener Dec. 14, 2017, 10:32 a.m. UTC | #1
On Thu, 14 Dec 2017, Jakub Jelinek wrote:

> On Wed, Dec 13, 2017 at 03:55:13PM -0700, Martin Sebor wrote:
> > For the second part, can you please also add a compile-time test
> > to verify that the result isn't constrained to the same range as
> > with a real argument?  Checking that the abort below isn't
> > eliminated would do it for %f:
> > 
> >   void f (char *d)
> >   {
> >     int n = __builtin_sprintf (d, "%f", 1.0Q);
> >     if (n < 8 || 13 < n)
> >       __builtin_abort ();
> >   }
> > 
> > A test that has a convenient setup for this is tree-ssa/builtin-
> > sprintf-2.c in case you want to add to it.
> 
> Can't use that test, because I need __float128, add options for it etc.
> 
> Anyway, here is the same patch with added extra test, tested on x86_64-linux
> and powerpc64-linux.  The tree-ssa test FAILs without the patch on both
> targets.

Ok

> 2017-12-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/83198
> 	* gimple-ssa-sprintf.c (format_floating): Set type solely based on
> 	dir.modifier, regardless of TREE_TYPE (arg).  Assume non-REAL_CST
> 	value if arg is a REAL_CST with incompatible type.
> 
> 	* gcc.dg/pr83198.c: New test.
> 	* gcc.dg/tree-ssa/pr83198.c: New test.
> 
> --- gcc/gimple-ssa-sprintf.c.jj	2017-11-03 15:37:04.000000000 +0100
> +++ gcc/gimple-ssa-sprintf.c	2017-12-13 13:37:59.289435623 +0100
> @@ -1885,6 +1885,8 @@ static fmtresult
>  format_floating (const directive &dir, tree arg)
>  {
>    HOST_WIDE_INT prec[] = { dir.prec[0], dir.prec[1] };
> +  tree type = (dir.modifier == FMT_LEN_L || dir.modifier == FMT_LEN_ll
> +	       ? long_double_type_node : double_type_node);
>  
>    /* For an indeterminate precision the lower bound must be assumed
>       to be zero.  */
> @@ -1892,10 +1894,6 @@ format_floating (const directive &dir, t
>      {
>        /* Get the number of fractional decimal digits needed to represent
>  	 the argument without a loss of accuracy.  */
> -      tree type = arg ? TREE_TYPE (arg) :
> -	(dir.modifier == FMT_LEN_L || dir.modifier == FMT_LEN_ll
> -	 ? long_double_type_node : double_type_node);
> -
>        unsigned fmtprec
>  	= REAL_MODE_FORMAT (TYPE_MODE (type))->p;
>  
> @@ -1946,7 +1944,9 @@ format_floating (const directive &dir, t
>  	}
>      }
>  
> -  if (!arg || TREE_CODE (arg) != REAL_CST)
> +  if (!arg
> +      || TREE_CODE (arg) != REAL_CST
> +      || !useless_type_conversion_p (type, TREE_TYPE (arg)))
>      return format_floating (dir, prec);
>  
>    /* The minimum and maximum number of bytes produced by the directive.  */
> --- gcc/testsuite/gcc.dg/pr83198.c.jj	2017-12-13 13:43:36.056192309 +0100
> +++ gcc/testsuite/gcc.dg/pr83198.c	2017-12-13 13:47:11.716474956 +0100
> @@ -0,0 +1,18 @@
> +/* PR tree-optimization/83198 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wall -Wno-format" } */
> +
> +int
> +foo (char *d[6], int x)
> +{
> +  int r = 0;
> +  r += __builtin_sprintf (d[0], "%f", x);
> +  r += __builtin_sprintf (d[1], "%a", x);
> +  r += __builtin_sprintf (d[2], "%f", "foo");
> +  r += __builtin_sprintf (d[3], "%a", "bar");
> +#ifdef __SIZEOF_FLOAT128__
> +  r += __builtin_sprintf (d[4], "%a", 1.0Q);
> +  r += __builtin_sprintf (d[5], "%Lf", 1.0Q);
> +#endif
> +  return r;
> +}
> --- gcc/testsuite/gcc.dg/tree-ssa/pr83198.c.jj	2017-12-14 09:43:07.534102549 +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr83198.c	2017-12-14 09:52:39.772964559 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/83198 */
> +/* { dg-do compile { target __float128 } } */
> +/* { dg-options "-O2 -fprintf-return-value -Wno-format -fdump-tree-optimized" } */
> +/* { dg-add-options __float128 } */
> +
> +void bar (void);
> +void link_error (void);
> +
> +void
> +foo (char *x)
> +{
> +  int a = __builtin_sprintf (x, "%f", 1.0Q);
> +  if (a < 8)
> +    link_error ();
> +  if (a > 13)
> +    bar ();
> +  if (a > 322)
> +    link_error ();
> +}
> +
> +/* Verify we don't optimize return value to [8, 13].  */
> +/* { dg-final { scan-tree-dump-not "link_error \\(\\);" "optimized" } } */
> +/* { dg-final { scan-tree-dump "bar \\(\\);" "optimized" } } */
> 
> 
> 	Jakub
> 
>
diff mbox series

Patch

--- gcc/gimple-ssa-sprintf.c.jj	2017-11-03 15:37:04.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c	2017-12-13 13:37:59.289435623 +0100
@@ -1885,6 +1885,8 @@  static fmtresult
 format_floating (const directive &dir, tree arg)
 {
   HOST_WIDE_INT prec[] = { dir.prec[0], dir.prec[1] };
+  tree type = (dir.modifier == FMT_LEN_L || dir.modifier == FMT_LEN_ll
+	       ? long_double_type_node : double_type_node);
 
   /* For an indeterminate precision the lower bound must be assumed
      to be zero.  */
@@ -1892,10 +1894,6 @@  format_floating (const directive &dir, t
     {
       /* Get the number of fractional decimal digits needed to represent
 	 the argument without a loss of accuracy.  */
-      tree type = arg ? TREE_TYPE (arg) :
-	(dir.modifier == FMT_LEN_L || dir.modifier == FMT_LEN_ll
-	 ? long_double_type_node : double_type_node);
-
       unsigned fmtprec
 	= REAL_MODE_FORMAT (TYPE_MODE (type))->p;
 
@@ -1946,7 +1944,9 @@  format_floating (const directive &dir, t
 	}
     }
 
-  if (!arg || TREE_CODE (arg) != REAL_CST)
+  if (!arg
+      || TREE_CODE (arg) != REAL_CST
+      || !useless_type_conversion_p (type, TREE_TYPE (arg)))
     return format_floating (dir, prec);
 
   /* The minimum and maximum number of bytes produced by the directive.  */
--- gcc/testsuite/gcc.dg/pr83198.c.jj	2017-12-13 13:43:36.056192309 +0100
+++ gcc/testsuite/gcc.dg/pr83198.c	2017-12-13 13:47:11.716474956 +0100
@@ -0,0 +1,18 @@ 
+/* PR tree-optimization/83198 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wno-format" } */
+
+int
+foo (char *d[6], int x)
+{
+  int r = 0;
+  r += __builtin_sprintf (d[0], "%f", x);
+  r += __builtin_sprintf (d[1], "%a", x);
+  r += __builtin_sprintf (d[2], "%f", "foo");
+  r += __builtin_sprintf (d[3], "%a", "bar");
+#ifdef __SIZEOF_FLOAT128__
+  r += __builtin_sprintf (d[4], "%a", 1.0Q);
+  r += __builtin_sprintf (d[5], "%Lf", 1.0Q);
+#endif
+  return r;
+}
--- gcc/testsuite/gcc.dg/tree-ssa/pr83198.c.jj	2017-12-14 09:43:07.534102549 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr83198.c	2017-12-14 09:52:39.772964559 +0100
@@ -0,0 +1,23 @@ 
+/* PR tree-optimization/83198 */
+/* { dg-do compile { target __float128 } } */
+/* { dg-options "-O2 -fprintf-return-value -Wno-format -fdump-tree-optimized" } */
+/* { dg-add-options __float128 } */
+
+void bar (void);
+void link_error (void);
+
+void
+foo (char *x)
+{
+  int a = __builtin_sprintf (x, "%f", 1.0Q);
+  if (a < 8)
+    link_error ();
+  if (a > 13)
+    bar ();
+  if (a > 322)
+    link_error ();
+}
+
+/* Verify we don't optimize return value to [8, 13].  */
+/* { dg-final { scan-tree-dump-not "link_error \\(\\);" "optimized" } } */
+/* { dg-final { scan-tree-dump "bar \\(\\);" "optimized" } } */