diff mbox series

Fix s{,n}printf folding ICEs with slightly bogus prototypes (PR tree-optimization/89998)

Message ID 20190409064357.GH21066@tucnak
State New
Headers show
Series Fix s{,n}printf folding ICEs with slightly bogus prototypes (PR tree-optimization/89998) | expand

Commit Message

Jakub Jelinek April 9, 2019, 6:43 a.m. UTC
Hi!

If there are major differences in argument or return types of builtin
prototypes, we already don't mark them as builtins, but if there are smaller
differences like signedness changes of integral types with the same
precision, we still accept them (with warning).

The following patch makes sure we don't ICE in those cases by using the lhs
type where possible instead of hardcoding that s{,n}printf return always
int.

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

2019-04-09  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/89998
	* gimple-ssa-sprintf.c (try_substitute_return_value): Use lhs type
	instead of integer_type_node if possible, don't add ranges if return
	type is not compatible with int.
	* gimple-fold.c (gimple_fold_builtin_sprintf,
	gimple_fold_builtin_snprintf): Use lhs type instead of hardcoded
	integer_type_node.

	* gcc.c-torture/compile/pr89998-1.c: New test.
	* gcc.c-torture/compile/pr89998-2.c: New test.


	Jakub

Comments

Richard Biener April 9, 2019, 10:21 a.m. UTC | #1
On Tue, 9 Apr 2019, Jakub Jelinek wrote:

> Hi!
> 
> If there are major differences in argument or return types of builtin
> prototypes, we already don't mark them as builtins, but if there are smaller
> differences like signedness changes of integral types with the same
> precision, we still accept them (with warning).
> 
> The following patch makes sure we don't ICE in those cases by using the lhs
> type where possible instead of hardcoding that s{,n}printf return always
> int.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-04-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/89998
> 	* gimple-ssa-sprintf.c (try_substitute_return_value): Use lhs type
> 	instead of integer_type_node if possible, don't add ranges if return
> 	type is not compatible with int.
> 	* gimple-fold.c (gimple_fold_builtin_sprintf,
> 	gimple_fold_builtin_snprintf): Use lhs type instead of hardcoded
> 	integer_type_node.
> 
> 	* gcc.c-torture/compile/pr89998-1.c: New test.
> 	* gcc.c-torture/compile/pr89998-2.c: New test.
> 
> --- gcc/gimple-ssa-sprintf.c.jj	2019-03-05 09:42:23.867092470 +0100
> +++ gcc/gimple-ssa-sprintf.c	2019-04-08 11:23:43.568301945 +0200
> @@ -3692,10 +3692,10 @@ try_substitute_return_value (gimple_stmt
>  	 are badly declared.  */
>        && !stmt_ends_bb_p (info.callstmt))
>      {
> -      tree cst = build_int_cst (integer_type_node, retval[0]);
> +      tree cst = build_int_cst (lhs ? TREE_TYPE (lhs) : integer_type_node,
> +				retval[0]);
>  
> -      if (lhs == NULL_TREE
> -	  && info.nowrite)
> +      if (lhs == NULL_TREE && info.nowrite)
>  	{
>  	  /* Remove the call to the bounded function with a zero size
>  	     (e.g., snprintf(0, 0, "%i", 123)) if there is no lhs.  */
> @@ -3736,7 +3736,7 @@ try_substitute_return_value (gimple_stmt
>  	    }
>  	}
>      }
> -  else if (lhs)
> +  else if (lhs && types_compatible_p (TREE_TYPE (lhs), integer_type_node))
>      {
>        bool setrange = false;
>  
> --- gcc/gimple-fold.c.jj	2019-03-07 20:07:20.292011398 +0100
> +++ gcc/gimple-fold.c	2019-04-08 11:10:45.380940700 +0200
> @@ -3231,11 +3231,10 @@ gimple_fold_builtin_sprintf (gimple_stmt
>  	gimple_set_no_warning (repl, true);
>  
>        gimple_seq_add_stmt_without_update (&stmts, repl);
> -      if (gimple_call_lhs (stmt))
> +      if (tree lhs = gimple_call_lhs (stmt))
>  	{
> -	  repl = gimple_build_assign (gimple_call_lhs (stmt),
> -				      build_int_cst (integer_type_node,
> -						     strlen (fmt_str)));
> +	  repl = gimple_build_assign (lhs, build_int_cst (TREE_TYPE (lhs),
> +							  strlen (fmt_str)));
>  	  gimple_seq_add_stmt_without_update (&stmts, repl);
>  	  gsi_replace_with_seq_vops (gsi, stmts);
>  	  /* gsi now points at the assignment to the lhs, get a
> @@ -3285,12 +3284,12 @@ gimple_fold_builtin_sprintf (gimple_stmt
>  	gimple_set_no_warning (repl, true);
>  
>        gimple_seq_add_stmt_without_update (&stmts, repl);
> -      if (gimple_call_lhs (stmt))
> +      if (tree lhs = gimple_call_lhs (stmt))
>  	{
> -	  if (!useless_type_conversion_p (integer_type_node,
> +	  if (!useless_type_conversion_p (TREE_TYPE (lhs),
>  					  TREE_TYPE (orig_len)))
> -	    orig_len = fold_convert (integer_type_node, orig_len);
> -	  repl = gimple_build_assign (gimple_call_lhs (stmt), orig_len);
> +	    orig_len = fold_convert (TREE_TYPE (lhs), orig_len);
> +	  repl = gimple_build_assign (lhs, orig_len);
>  	  gimple_seq_add_stmt_without_update (&stmts, repl);
>  	  gsi_replace_with_seq_vops (gsi, stmts);
>  	  /* gsi now points at the assignment to the lhs, get a
> @@ -3370,10 +3369,10 @@ gimple_fold_builtin_snprintf (gimple_stm
>        gimple_seq stmts = NULL;
>        gimple *repl = gimple_build_call (fn, 2, dest, fmt);
>        gimple_seq_add_stmt_without_update (&stmts, repl);
> -      if (gimple_call_lhs (stmt))
> +      if (tree lhs = gimple_call_lhs (stmt))
>  	{
> -	  repl = gimple_build_assign (gimple_call_lhs (stmt),
> -				      build_int_cst (integer_type_node, len));
> +	  repl = gimple_build_assign (lhs,
> +				      build_int_cst (TREE_TYPE (lhs), len));
>  	  gimple_seq_add_stmt_without_update (&stmts, repl);
>  	  gsi_replace_with_seq_vops (gsi, stmts);
>  	  /* gsi now points at the assignment to the lhs, get a
> @@ -3422,12 +3421,12 @@ gimple_fold_builtin_snprintf (gimple_stm
>        gimple_seq stmts = NULL;
>        gimple *repl = gimple_build_call (fn, 2, dest, orig);
>        gimple_seq_add_stmt_without_update (&stmts, repl);
> -      if (gimple_call_lhs (stmt))
> +      if (tree lhs = gimple_call_lhs (stmt))
>  	{
> -	  if (!useless_type_conversion_p (integer_type_node,
> +	  if (!useless_type_conversion_p (TREE_TYPE (lhs),
>  					  TREE_TYPE (orig_len)))
> -	    orig_len = fold_convert (integer_type_node, orig_len);
> -	  repl = gimple_build_assign (gimple_call_lhs (stmt), orig_len);
> +	    orig_len = fold_convert (TREE_TYPE (lhs), orig_len);
> +	  repl = gimple_build_assign (lhs, orig_len);
>  	  gimple_seq_add_stmt_without_update (&stmts, repl);
>  	  gsi_replace_with_seq_vops (gsi, stmts);
>  	  /* gsi now points at the assignment to the lhs, get a
> --- gcc/testsuite/gcc.c-torture/compile/pr89998-1.c.jj	2019-04-08 12:52:04.946948129 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr89998-1.c	2019-04-08 12:33:33.315156426 +0200
> @@ -0,0 +1,42 @@
> +/* PR tree-optimization/89998 */
> +
> +unsigned int sprintf (char *str, const char *fmt, ...);
> +unsigned int snprintf (char *str, __SIZE_TYPE__ len, const char *fmt, ...);
> +
> +int
> +f1 (char *s)
> +{
> +  return sprintf (s, "foo");
> +}
> +
> +int
> +f2 (char *s)
> +{
> +  return sprintf (s, "%d", 123);
> +}
> +
> +int
> +f3 (int *p, char *s)
> +{
> +  const char *t = "bar";
> +  return sprintf (s, "%s", t);
> +}
> +
> +int
> +f4 (char *s)
> +{
> +  return snprintf (s, 8, "foo");
> +}
> +
> +int
> +f5 (char *s)
> +{
> +  return snprintf (s, 8, "%d", 123);
> +}
> +
> +int
> +f6 (int *p, char *s)
> +{
> +  const char *t = "bar";
> +  return snprintf (s, 8, "%s", t);
> +}
> --- gcc/testsuite/gcc.c-torture/compile/pr89998-2.c.jj	2019-04-08 12:52:08.852883591 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr89998-2.c	2019-04-08 12:52:00.846015892 +0200
> @@ -0,0 +1,4 @@
> +/* PR tree-optimization/89998 */
> +/* { dg-additional-options "-fno-printf-return-value" } */
> +
> +#include "pr89998-1.c"
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/gimple-ssa-sprintf.c.jj	2019-03-05 09:42:23.867092470 +0100
+++ gcc/gimple-ssa-sprintf.c	2019-04-08 11:23:43.568301945 +0200
@@ -3692,10 +3692,10 @@  try_substitute_return_value (gimple_stmt
 	 are badly declared.  */
       && !stmt_ends_bb_p (info.callstmt))
     {
-      tree cst = build_int_cst (integer_type_node, retval[0]);
+      tree cst = build_int_cst (lhs ? TREE_TYPE (lhs) : integer_type_node,
+				retval[0]);
 
-      if (lhs == NULL_TREE
-	  && info.nowrite)
+      if (lhs == NULL_TREE && info.nowrite)
 	{
 	  /* Remove the call to the bounded function with a zero size
 	     (e.g., snprintf(0, 0, "%i", 123)) if there is no lhs.  */
@@ -3736,7 +3736,7 @@  try_substitute_return_value (gimple_stmt
 	    }
 	}
     }
-  else if (lhs)
+  else if (lhs && types_compatible_p (TREE_TYPE (lhs), integer_type_node))
     {
       bool setrange = false;
 
--- gcc/gimple-fold.c.jj	2019-03-07 20:07:20.292011398 +0100
+++ gcc/gimple-fold.c	2019-04-08 11:10:45.380940700 +0200
@@ -3231,11 +3231,10 @@  gimple_fold_builtin_sprintf (gimple_stmt
 	gimple_set_no_warning (repl, true);
 
       gimple_seq_add_stmt_without_update (&stmts, repl);
-      if (gimple_call_lhs (stmt))
+      if (tree lhs = gimple_call_lhs (stmt))
 	{
-	  repl = gimple_build_assign (gimple_call_lhs (stmt),
-				      build_int_cst (integer_type_node,
-						     strlen (fmt_str)));
+	  repl = gimple_build_assign (lhs, build_int_cst (TREE_TYPE (lhs),
+							  strlen (fmt_str)));
 	  gimple_seq_add_stmt_without_update (&stmts, repl);
 	  gsi_replace_with_seq_vops (gsi, stmts);
 	  /* gsi now points at the assignment to the lhs, get a
@@ -3285,12 +3284,12 @@  gimple_fold_builtin_sprintf (gimple_stmt
 	gimple_set_no_warning (repl, true);
 
       gimple_seq_add_stmt_without_update (&stmts, repl);
-      if (gimple_call_lhs (stmt))
+      if (tree lhs = gimple_call_lhs (stmt))
 	{
-	  if (!useless_type_conversion_p (integer_type_node,
+	  if (!useless_type_conversion_p (TREE_TYPE (lhs),
 					  TREE_TYPE (orig_len)))
-	    orig_len = fold_convert (integer_type_node, orig_len);
-	  repl = gimple_build_assign (gimple_call_lhs (stmt), orig_len);
+	    orig_len = fold_convert (TREE_TYPE (lhs), orig_len);
+	  repl = gimple_build_assign (lhs, orig_len);
 	  gimple_seq_add_stmt_without_update (&stmts, repl);
 	  gsi_replace_with_seq_vops (gsi, stmts);
 	  /* gsi now points at the assignment to the lhs, get a
@@ -3370,10 +3369,10 @@  gimple_fold_builtin_snprintf (gimple_stm
       gimple_seq stmts = NULL;
       gimple *repl = gimple_build_call (fn, 2, dest, fmt);
       gimple_seq_add_stmt_without_update (&stmts, repl);
-      if (gimple_call_lhs (stmt))
+      if (tree lhs = gimple_call_lhs (stmt))
 	{
-	  repl = gimple_build_assign (gimple_call_lhs (stmt),
-				      build_int_cst (integer_type_node, len));
+	  repl = gimple_build_assign (lhs,
+				      build_int_cst (TREE_TYPE (lhs), len));
 	  gimple_seq_add_stmt_without_update (&stmts, repl);
 	  gsi_replace_with_seq_vops (gsi, stmts);
 	  /* gsi now points at the assignment to the lhs, get a
@@ -3422,12 +3421,12 @@  gimple_fold_builtin_snprintf (gimple_stm
       gimple_seq stmts = NULL;
       gimple *repl = gimple_build_call (fn, 2, dest, orig);
       gimple_seq_add_stmt_without_update (&stmts, repl);
-      if (gimple_call_lhs (stmt))
+      if (tree lhs = gimple_call_lhs (stmt))
 	{
-	  if (!useless_type_conversion_p (integer_type_node,
+	  if (!useless_type_conversion_p (TREE_TYPE (lhs),
 					  TREE_TYPE (orig_len)))
-	    orig_len = fold_convert (integer_type_node, orig_len);
-	  repl = gimple_build_assign (gimple_call_lhs (stmt), orig_len);
+	    orig_len = fold_convert (TREE_TYPE (lhs), orig_len);
+	  repl = gimple_build_assign (lhs, orig_len);
 	  gimple_seq_add_stmt_without_update (&stmts, repl);
 	  gsi_replace_with_seq_vops (gsi, stmts);
 	  /* gsi now points at the assignment to the lhs, get a
--- gcc/testsuite/gcc.c-torture/compile/pr89998-1.c.jj	2019-04-08 12:52:04.946948129 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr89998-1.c	2019-04-08 12:33:33.315156426 +0200
@@ -0,0 +1,42 @@ 
+/* PR tree-optimization/89998 */
+
+unsigned int sprintf (char *str, const char *fmt, ...);
+unsigned int snprintf (char *str, __SIZE_TYPE__ len, const char *fmt, ...);
+
+int
+f1 (char *s)
+{
+  return sprintf (s, "foo");
+}
+
+int
+f2 (char *s)
+{
+  return sprintf (s, "%d", 123);
+}
+
+int
+f3 (int *p, char *s)
+{
+  const char *t = "bar";
+  return sprintf (s, "%s", t);
+}
+
+int
+f4 (char *s)
+{
+  return snprintf (s, 8, "foo");
+}
+
+int
+f5 (char *s)
+{
+  return snprintf (s, 8, "%d", 123);
+}
+
+int
+f6 (int *p, char *s)
+{
+  const char *t = "bar";
+  return snprintf (s, 8, "%s", t);
+}
--- gcc/testsuite/gcc.c-torture/compile/pr89998-2.c.jj	2019-04-08 12:52:08.852883591 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr89998-2.c	2019-04-08 12:52:00.846015892 +0200
@@ -0,0 +1,4 @@ 
+/* PR tree-optimization/89998 */
+/* { dg-additional-options "-fno-printf-return-value" } */
+
+#include "pr89998-1.c"