diff mbox

Fix ICE with va_arg (PR tree-optimization/69162)

Message ID 20160107214625.GQ18720@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Jan. 7, 2016, 9:46 p.m. UTC
Hi!

The addition of IFN_VA_ARG which is only during stdarg pass lowered
introduced ICE on the following testcase.  The problem is that
we expected that the type the first argument of the internal call points
to will remain the one that used to be there during gimplification, but
as pointer conversions are useless, that is not guaranteed, it can become
void * or any other pointer type.

Fixed by remembering the type on another argument.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-01-07  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/69162
	* gimplify.c (gimplify_va_arg_expr): Encode original type of
	valist argument in another argument.
	(gimplify_modify_expr): Adjust for the above change.  Cleanup.
	* tree-stdarg.c (expand_ifn_va_arg_1): Use new 3rd argument
	to determine the va_list type, build a MEM_REF instead of
	build_fold_indirect_ref.

	* gcc.dg/pr69162.c: New test.


	Jakub

Comments

Richard Biener Jan. 8, 2016, 9:13 a.m. UTC | #1
On Thu, 7 Jan 2016, Jakub Jelinek wrote:

> Hi!
> 
> The addition of IFN_VA_ARG which is only during stdarg pass lowered
> introduced ICE on the following testcase.  The problem is that
> we expected that the type the first argument of the internal call points
> to will remain the one that used to be there during gimplification, but
> as pointer conversions are useless, that is not guaranteed, it can become
> void * or any other pointer type.
> 
> Fixed by remembering the type on another argument.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2016-01-07  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/69162
> 	* gimplify.c (gimplify_va_arg_expr): Encode original type of
> 	valist argument in another argument.
> 	(gimplify_modify_expr): Adjust for the above change.  Cleanup.
> 	* tree-stdarg.c (expand_ifn_va_arg_1): Use new 3rd argument
> 	to determine the va_list type, build a MEM_REF instead of
> 	build_fold_indirect_ref.
> 
> 	* gcc.dg/pr69162.c: New test.
> 
> --- gcc/gimplify.c.jj	2016-01-04 14:55:53.000000000 +0100
> +++ gcc/gimplify.c	2016-01-07 15:19:17.283215609 +0100
> @@ -4724,12 +4724,12 @@ gimplify_modify_expr (tree *expr_p, gimp
>  	  tree type = TREE_TYPE (call);
>  	  tree ap = CALL_EXPR_ARG (call, 0);
>  	  tree tag = CALL_EXPR_ARG (call, 1);
> +	  tree aptag = CALL_EXPR_ARG (call, 2);
>  	  tree newcall = build_call_expr_internal_loc (EXPR_LOCATION (call),
>  						       IFN_VA_ARG, type,
>  						       nargs + 1, ap, tag,
> -						       vlasize);
> -	  tree *call_p = &(TREE_OPERAND (*from_p, 0));
> -	  *call_p = newcall;
> +						       aptag, vlasize);
> +	  TREE_OPERAND (*from_p, 0) = newcall;
>  	}
>      }
>  
> @@ -11501,7 +11501,7 @@ gimplify_va_arg_expr (tree *expr_p, gimp
>    tree promoted_type, have_va_type;
>    tree valist = TREE_OPERAND (*expr_p, 0);
>    tree type = TREE_TYPE (*expr_p);
> -  tree t, tag;
> +  tree t, tag, aptag;
>    location_t loc = EXPR_LOCATION (*expr_p);
>  
>    /* Verify that valist is of the proper type.  */
> @@ -11555,7 +11555,10 @@ gimplify_va_arg_expr (tree *expr_p, gimp
>      }
>  
>    tag = build_int_cst (build_pointer_type (type), 0);
> -  *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, valist, tag);
> +  aptag = build_int_cst (TREE_TYPE (valist), 0);
> +
> +  *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 3,
> +					  valist, tag, aptag);
>  
>    /* Clear the tentatively set PROP_gimple_lva, to indicate that IFN_VA_ARG
>       needs to be expanded.  */
> --- gcc/tree-stdarg.c.jj	2016-01-04 14:55:52.000000000 +0100
> +++ gcc/tree-stdarg.c	2016-01-07 15:20:14.340424740 +0100
> @@ -1018,7 +1018,7 @@ expand_ifn_va_arg_1 (function *fun)
>      for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
>        {
>  	gimple *stmt = gsi_stmt (i);
> -	tree ap, expr, lhs, type;
> +	tree ap, aptype, expr, lhs, type;
>  	gimple_seq pre = NULL, post = NULL;
>  
>  	if (!gimple_call_ifn_va_arg_p (stmt))
> @@ -1028,9 +1028,12 @@ expand_ifn_va_arg_1 (function *fun)
>  
>  	type = TREE_TYPE (TREE_TYPE (gimple_call_arg (stmt, 1)));
>  	ap = gimple_call_arg (stmt, 0);
> +	aptype = TREE_TYPE (gimple_call_arg (stmt, 2));
> +	gcc_assert (POINTER_TYPE_P (aptype));
>  
>  	/* Balanced out the &ap, usually added by build_va_arg.  */
> -	ap = build_fold_indirect_ref (ap);
> +	ap = build2 (MEM_REF, TREE_TYPE (aptype), ap,
> +		     build_int_cst (aptype, 0));
>  
>  	push_gimplify_context (false);
>  	saved_location = input_location;
> @@ -1053,7 +1056,7 @@ expand_ifn_va_arg_1 (function *fun)
>  	    if (chkp_function_instrumented_p (fun->decl))
>  	      chkp_fixup_inlined_call (lhs, expr);
>  
> -	    if (nargs == 3)
> +	    if (nargs == 4)
>  	      {
>  		/* We've transported the size of with WITH_SIZE_EXPR here as
>  		   the last argument of the internal fn call.  Now reinstate
> --- gcc/testsuite/gcc.dg/pr69162.c.jj	2016-01-07 15:27:10.215660347 +0100
> +++ gcc/testsuite/gcc.dg/pr69162.c	2016-01-07 15:27:47.127148736 +0100
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/69162 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +#include <stdarg.h>
> +
> +int
> +foo (void *a)
> +{
> +  va_list *b = a;
> +  return va_arg (*b, int);
> +}
> 
> 	Jakub
> 
>
diff mbox

Patch

--- gcc/gimplify.c.jj	2016-01-04 14:55:53.000000000 +0100
+++ gcc/gimplify.c	2016-01-07 15:19:17.283215609 +0100
@@ -4724,12 +4724,12 @@  gimplify_modify_expr (tree *expr_p, gimp
 	  tree type = TREE_TYPE (call);
 	  tree ap = CALL_EXPR_ARG (call, 0);
 	  tree tag = CALL_EXPR_ARG (call, 1);
+	  tree aptag = CALL_EXPR_ARG (call, 2);
 	  tree newcall = build_call_expr_internal_loc (EXPR_LOCATION (call),
 						       IFN_VA_ARG, type,
 						       nargs + 1, ap, tag,
-						       vlasize);
-	  tree *call_p = &(TREE_OPERAND (*from_p, 0));
-	  *call_p = newcall;
+						       aptag, vlasize);
+	  TREE_OPERAND (*from_p, 0) = newcall;
 	}
     }
 
@@ -11501,7 +11501,7 @@  gimplify_va_arg_expr (tree *expr_p, gimp
   tree promoted_type, have_va_type;
   tree valist = TREE_OPERAND (*expr_p, 0);
   tree type = TREE_TYPE (*expr_p);
-  tree t, tag;
+  tree t, tag, aptag;
   location_t loc = EXPR_LOCATION (*expr_p);
 
   /* Verify that valist is of the proper type.  */
@@ -11555,7 +11555,10 @@  gimplify_va_arg_expr (tree *expr_p, gimp
     }
 
   tag = build_int_cst (build_pointer_type (type), 0);
-  *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, valist, tag);
+  aptag = build_int_cst (TREE_TYPE (valist), 0);
+
+  *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 3,
+					  valist, tag, aptag);
 
   /* Clear the tentatively set PROP_gimple_lva, to indicate that IFN_VA_ARG
      needs to be expanded.  */
--- gcc/tree-stdarg.c.jj	2016-01-04 14:55:52.000000000 +0100
+++ gcc/tree-stdarg.c	2016-01-07 15:20:14.340424740 +0100
@@ -1018,7 +1018,7 @@  expand_ifn_va_arg_1 (function *fun)
     for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
       {
 	gimple *stmt = gsi_stmt (i);
-	tree ap, expr, lhs, type;
+	tree ap, aptype, expr, lhs, type;
 	gimple_seq pre = NULL, post = NULL;
 
 	if (!gimple_call_ifn_va_arg_p (stmt))
@@ -1028,9 +1028,12 @@  expand_ifn_va_arg_1 (function *fun)
 
 	type = TREE_TYPE (TREE_TYPE (gimple_call_arg (stmt, 1)));
 	ap = gimple_call_arg (stmt, 0);
+	aptype = TREE_TYPE (gimple_call_arg (stmt, 2));
+	gcc_assert (POINTER_TYPE_P (aptype));
 
 	/* Balanced out the &ap, usually added by build_va_arg.  */
-	ap = build_fold_indirect_ref (ap);
+	ap = build2 (MEM_REF, TREE_TYPE (aptype), ap,
+		     build_int_cst (aptype, 0));
 
 	push_gimplify_context (false);
 	saved_location = input_location;
@@ -1053,7 +1056,7 @@  expand_ifn_va_arg_1 (function *fun)
 	    if (chkp_function_instrumented_p (fun->decl))
 	      chkp_fixup_inlined_call (lhs, expr);
 
-	    if (nargs == 3)
+	    if (nargs == 4)
 	      {
 		/* We've transported the size of with WITH_SIZE_EXPR here as
 		   the last argument of the internal fn call.  Now reinstate
--- gcc/testsuite/gcc.dg/pr69162.c.jj	2016-01-07 15:27:10.215660347 +0100
+++ gcc/testsuite/gcc.dg/pr69162.c	2016-01-07 15:27:47.127148736 +0100
@@ -0,0 +1,12 @@ 
+/* PR tree-optimization/69162 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#include <stdarg.h>
+
+int
+foo (void *a)
+{
+  va_list *b = a;
+  return va_arg (*b, int);
+}