diff mbox series

Fix gimple-ssa-sprintf ICE (PR tree-optimization/89566)

Message ID 20190304222601.GQ7611@tucnak
State New
Headers show
Series Fix gimple-ssa-sprintf ICE (PR tree-optimization/89566) | expand

Commit Message

Jakub Jelinek March 4, 2019, 10:26 p.m. UTC
Hi!

Before PR87041 changes sprintf_dom_walker::handle_gimple_call
would punt if gimple_call_builtin_p (which did all the needed call argument
checking) failed, but it doesn't fail anymore because it wants to handle
format attribute.  That is fine, but if gimple_call_builtin_p failed, we
shouldn't handle them as builtins, just possibly as format string argument
functions.  Note, info.func might be even backend or FE builtin and
DECL_FUNCTION_CODE in those cases means something completely different
anyway.

So, the following patch makes sure to only set info.fncode if
gimple_call_builtin_p succeeds, and for format attribute does at least
minimal verification, that the call actually has an argument at idx_format
position which also has a pointer-ish type, and that idx_args isn't out of
bounds either (that one can be equal to number of arguments, that represents
zero varargs arguments).

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

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

	PR tree-optimization/89566
	* gimple-ssa-sprintf.c (sprintf_dom_walker::handle_gimple_call):
	Set info.fncode to BUILT_IN_NONE if gimple_call_builtin_p failed.
	Punt if get_user_idx_format succeeds, but idx_format argument is
	not provided or doesn't have pointer type, or if idx_args is above
	number of provided arguments.

	* c-c++-common/pr89566.c: New test.


	Jakub

Comments

Richard Biener March 5, 2019, 8:38 a.m. UTC | #1
On Mon, 4 Mar 2019, Jakub Jelinek wrote:

> Hi!
> 
> Before PR87041 changes sprintf_dom_walker::handle_gimple_call
> would punt if gimple_call_builtin_p (which did all the needed call argument
> checking) failed, but it doesn't fail anymore because it wants to handle
> format attribute.  That is fine, but if gimple_call_builtin_p failed, we
> shouldn't handle them as builtins, just possibly as format string argument
> functions.  Note, info.func might be even backend or FE builtin and
> DECL_FUNCTION_CODE in those cases means something completely different
> anyway.
> 
> So, the following patch makes sure to only set info.fncode if
> gimple_call_builtin_p succeeds, and for format attribute does at least
> minimal verification, that the call actually has an argument at idx_format
> position which also has a pointer-ish type, and that idx_args isn't out of
> bounds either (that one can be equal to number of arguments, that represents
> zero varargs arguments).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-03-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/89566
> 	* gimple-ssa-sprintf.c (sprintf_dom_walker::handle_gimple_call):
> 	Set info.fncode to BUILT_IN_NONE if gimple_call_builtin_p failed.
> 	Punt if get_user_idx_format succeeds, but idx_format argument is
> 	not provided or doesn't have pointer type, or if idx_args is above
> 	number of provided arguments.
> 
> 	* c-c++-common/pr89566.c: New test.
> 
> --- gcc/gimple-ssa-sprintf.c.jj	2019-02-24 20:18:01.348754080 +0100
> +++ gcc/gimple-ssa-sprintf.c	2019-03-04 10:52:32.867295908 +0100
> @@ -3858,16 +3858,21 @@ sprintf_dom_walker::handle_gimple_call (
>    if (!info.func)
>      return false;
>  
> -  info.fncode = DECL_FUNCTION_CODE (info.func);
> -
>    /* Format string argument number (valid for all functions).  */
>    unsigned idx_format = UINT_MAX;
> -  if (!gimple_call_builtin_p (info.callstmt, BUILT_IN_NORMAL))
> +  if (gimple_call_builtin_p (info.callstmt, BUILT_IN_NORMAL))
> +    info.fncode = DECL_FUNCTION_CODE (info.func);
> +  else
>      {
>        unsigned idx_args;
>        idx_format = get_user_idx_format (info.func, &idx_args);
> -      if (idx_format == UINT_MAX)
> +      if (idx_format == UINT_MAX
> +	  || idx_format >= gimple_call_num_args (info.callstmt)
> +	  || idx_args > gimple_call_num_args (info.callstmt)
> +	  || !POINTER_TYPE_P (TREE_TYPE (gimple_call_arg (info.callstmt,
> +							  idx_format))))
>  	return false;
> +      info.fncode = BUILT_IN_NONE;
>        info.argidx = idx_args;
>      }
>  
> --- gcc/testsuite/c-c++-common/pr89566.c.jj	2019-03-04 10:56:10.060730886 +0100
> +++ gcc/testsuite/c-c++-common/pr89566.c	2019-03-04 10:55:54.334989014 +0100
> @@ -0,0 +1,15 @@
> +/* PR tree-optimization/89566 */
> +/* { dg-do compile } */
> +
> +typedef struct FILE { int i; } FILE;
> +#ifdef __cplusplus
> +extern "C"
> +#endif
> +int fprintf (FILE *, const char *, ...);
> +
> +int
> +main ()
> +{
> +  ((void (*)()) fprintf) ();	// { dg-warning "function called through a non-compatible type" "" { target c } }
> +  return 0;
> +}
> 
> 	Jakub
> 
>
diff mbox series

Patch

--- gcc/gimple-ssa-sprintf.c.jj	2019-02-24 20:18:01.348754080 +0100
+++ gcc/gimple-ssa-sprintf.c	2019-03-04 10:52:32.867295908 +0100
@@ -3858,16 +3858,21 @@  sprintf_dom_walker::handle_gimple_call (
   if (!info.func)
     return false;
 
-  info.fncode = DECL_FUNCTION_CODE (info.func);
-
   /* Format string argument number (valid for all functions).  */
   unsigned idx_format = UINT_MAX;
-  if (!gimple_call_builtin_p (info.callstmt, BUILT_IN_NORMAL))
+  if (gimple_call_builtin_p (info.callstmt, BUILT_IN_NORMAL))
+    info.fncode = DECL_FUNCTION_CODE (info.func);
+  else
     {
       unsigned idx_args;
       idx_format = get_user_idx_format (info.func, &idx_args);
-      if (idx_format == UINT_MAX)
+      if (idx_format == UINT_MAX
+	  || idx_format >= gimple_call_num_args (info.callstmt)
+	  || idx_args > gimple_call_num_args (info.callstmt)
+	  || !POINTER_TYPE_P (TREE_TYPE (gimple_call_arg (info.callstmt,
+							  idx_format))))
 	return false;
+      info.fncode = BUILT_IN_NONE;
       info.argidx = idx_args;
     }
 
--- gcc/testsuite/c-c++-common/pr89566.c.jj	2019-03-04 10:56:10.060730886 +0100
+++ gcc/testsuite/c-c++-common/pr89566.c	2019-03-04 10:55:54.334989014 +0100
@@ -0,0 +1,15 @@ 
+/* PR tree-optimization/89566 */
+/* { dg-do compile } */
+
+typedef struct FILE { int i; } FILE;
+#ifdef __cplusplus
+extern "C"
+#endif
+int fprintf (FILE *, const char *, ...);
+
+int
+main ()
+{
+  ((void (*)()) fprintf) ();	// { dg-warning "function called through a non-compatible type" "" { target c } }
+  return 0;
+}