Patchwork Ensure gimplify_one_sizepos doesn't change something with INTEGER_TYPE into something with e.g. ENUMERAL_TYPE (PR middle-end/55851)

login
register
mail settings
Submitter Richard Guenther
Date Jan. 4, 2013, 1:21 p.m.
Message ID <alpine.LNX.2.00.1301041416420.6889@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/209456/
State New
Headers show

Comments

Richard Guenther - Jan. 4, 2013, 1:21 p.m.
On Fri, 4 Jan 2013, Jakub Jelinek wrote:

> Hi!
> 
> As discussed in the PR, as all INTEGRAL_TYPE_P types with the same
> sign/precision are usually considered compatible
> (useless_type_conversion_p), during gimplify_one_sizepos the gimplifier
> can change e.g. an integer expression into e.g. VAR_DECL with ENUMERAL_TYPE,
> and when the gimplifier later on passes that to size_binop etc., it can
> trigger asserts because ENUMERAL_TYPE isn't considered sizetype-ish enough.
> 
> The following patch (which I don't like too much admittedly) makes sure
> for constants we fold it back to INTEGER_TYPE constant, and for VAR_DECLs
> add an extra VAR_DECL in such a case.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> Or do you prefer some other way?

The other way would be

   switch (code)

at least the present check doesn't really verify we use sizetypes
for size_*op anymore.  I'd agree more with your patch if we'd
verify that we have proper [s]sizetype, [s]bitsizetype type
arguments to size_*op functions (which of course would be again
questionable to some degree).

That said - we seem to have moved into a direction that makes
the above patch not so questionable.

Richard.

> 2013-01-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/55851
> 	* gimplify.c (gimplify_one_sizepos): Ensure gimplify_expr
> 	doesn't turn *expr_p from INTEGER_TYPE expression into e.g.
> 	ENUMERAL_TYPE expression.
> 
> 	* gcc.c-torture/compile/pr55851.c: New test.
> 
> --- gcc/gimplify.c.jj	2012-12-20 19:13:00.000000000 +0100
> +++ gcc/gimplify.c	2013-01-03 16:16:07.288707387 +0100
> @@ -8180,6 +8180,26 @@ gimplify_one_sizepos (tree *expr_p, gimp
>    *expr_p = unshare_expr (expr);
>  
>    gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue);
> +
> +  /* Ensure we don't change an INTEGER_TYPE expression into e.g. ENUMERAL_TYPE
> +     expression.  */
> +  if (TREE_CODE (TREE_TYPE (*expr_p)) != TREE_CODE (TREE_TYPE (expr)))
> +    {
> +      gcc_checking_assert (useless_type_conversion_p (TREE_TYPE (expr),
> +						      TREE_TYPE (*expr_p)));
> +      if (TREE_CODE (*expr_p) == INTEGER_CST)
> +	*expr_p = fold_convert (TREE_TYPE (expr), *expr_p);
> +      else
> +	{
> +	  tree var = create_tmp_var (TREE_TYPE (expr), NULL);
> +	  tree mod = build2 (INIT_EXPR, TREE_TYPE (var),
> +			     var, unshare_expr (*expr_p));
> +	  SET_EXPR_LOCATION (mod, EXPR_LOC_OR_HERE (*expr_p));
> +	  gimplify_and_add (mod, stmt_p);
> +	  ggc_free (mod);
> +	  *expr_p = var;
> +	}
> +    }
>  }
>  
>  /* Gimplify the body of statements of FNDECL and return a GIMPLE_BIND node
> --- gcc/testsuite/gcc.c-torture/compile/pr55851.c.jj	2013-01-03 16:20:19.085284806 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr55851.c	2013-01-03 16:19:27.698571718 +0100
> @@ -0,0 +1,12 @@
> +/* PR middle-end/55851 */
> +
> +enum { A = 1UL, B = -1UL } var = A;
> +void foo (char *);
> +
> +void
> +test (void)
> +{
> +  char vla[1][var];
> +  vla[0][0] = 1;
> +  foo (&vla[0][0]);
> +}
> 
> 	Jakub
> 
>

Patch

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 194900)
+++ gcc/fold-const.c    (working copy)
@@ -900,9 +900,9 @@  associate_trees (location_t loc, tree t1
 static bool
 int_binop_types_match_p (enum tree_code code, const_tree type1, 
const_tree type2)
 {
-  if (TREE_CODE (type1) != INTEGER_TYPE && !POINTER_TYPE_P (type1))
+  if (!INTEGRAL_TYPE_P (type1) && !POINTER_TYPE_P (type1))
     return false;
-  if (TREE_CODE (type2) != INTEGER_TYPE && !POINTER_TYPE_P (type2))
+  if (!INTEGRAL_TYPE_P (type2) && !POINTER_TYPE_P (type2))
     return false;