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

2019-04-09 Thread Richard Biener
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  
> 
>   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 (, 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 (, 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 (, 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 (, 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 (, 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 (, 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 (, repl);
> -  if (gimple_call_lhs (stmt))
> +  if (tree lhs = gimple_call_lhs (stmt))
>   {
> -   if (!useless_type_conversion_p 

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

2019-04-09 Thread Jakub Jelinek
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  

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.c2019-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.jj2019-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 (, 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 (, 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 (, 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 (, 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 (, 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 (, 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 (, 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