Re: [PATCH] Fix ICE with va_arg (PR tree-optimization/69162)

2016-01-08 Thread Richard Biener
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  
> 
>   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.0 +0100
> +++ gcc/gimplify.c2016-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.0 +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.c2016-01-07 15:27:47.127148736 +0100
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/69162 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +#include 
> +
> +int
> +foo (void *a)
> +{
> +  va_list *b = a;
> +  return va_arg (*b, int);
> +}
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


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

2016-01-07 Thread Jakub Jelinek
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  

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.0 +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.jj2016-01-04 14:55:52.0 +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 
+
+int
+foo (void *a)
+{
+  va_list *b = a;
+  return va_arg (*b, int);
+}

Jakub