diff mbox

variably_modified_type_p tweak for cdtor cloning (PR debug/54828)

Message ID 20121023155053.GS1752@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Oct. 23, 2012, 3:50 p.m. UTC
Hi!

The following testcase ICEs, because the VLA ARRAY_TYPE in the ctor
isn't considered variably_modified_type_p during cloning of the ctor
when the types weren't gimplified yet.
The size is a non-constant expression that has SAVE_EXPR of a global
VAR_DECL in it, so when variably_modified_type_p is called with non-NULL
second argument, it returns NULL, we share the ARRAY_TYPE in between
the abstract ctor origin and and the base_ctor and comp_ctor clones,
later on when gimplifying the first of the clones gimplify_one_sizepos
actually replaces the expression with a local temporary VAR_DECL and from
that point it is variably_modified_type_p in the base ctor.  But
it is a var from different function in the other ctor and var with
a location in the abstract origin which causes dwarf2out.c ICE.

This patch fixes it by returning true also for non-gimplified types
where gimplify_one_sizepos is expected to turn that into a local temporary.
It seems frontends usually call variably_modified_type_p with
NULL as last argument, with non-NULL argument it is only used during
tree-nested.c/omp-low.c (which happens after gimplification), tree-inline.c
(which usually happens after gimplification, with the exception of cdtor
cloning, at least can't find anything else).

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

2012-10-23  Jakub Jelinek  <jakub@redhat.com>

	PR debug/54828
	* tree.c (RETURN_TRUE_IF_VAR): Return true also if
	!TYPE_SIZES_GIMPLIFIED (type) and _t is going to be gimplified
	into a local temporary.

	* g++.dg/debug/pr54828.C: New test.


	Jakub

Comments

Richard Biener Oct. 24, 2012, 8:35 a.m. UTC | #1
On Tue, Oct 23, 2012 at 5:50 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The following testcase ICEs, because the VLA ARRAY_TYPE in the ctor
> isn't considered variably_modified_type_p during cloning of the ctor
> when the types weren't gimplified yet.
> The size is a non-constant expression that has SAVE_EXPR of a global
> VAR_DECL in it, so when variably_modified_type_p is called with non-NULL
> second argument, it returns NULL, we share the ARRAY_TYPE in between
> the abstract ctor origin and and the base_ctor and comp_ctor clones,
> later on when gimplifying the first of the clones gimplify_one_sizepos
> actually replaces the expression with a local temporary VAR_DECL and from
> that point it is variably_modified_type_p in the base ctor.  But
> it is a var from different function in the other ctor and var with
> a location in the abstract origin which causes dwarf2out.c ICE.
>
> This patch fixes it by returning true also for non-gimplified types
> where gimplify_one_sizepos is expected to turn that into a local temporary.
> It seems frontends usually call variably_modified_type_p with
> NULL as last argument, with non-NULL argument it is only used during
> tree-nested.c/omp-low.c (which happens after gimplification), tree-inline.c
> (which usually happens after gimplification, with the exception of cdtor
> cloning, at least can't find anything else).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2012-10-23  Jakub Jelinek  <jakub@redhat.com>
>
>         PR debug/54828
>         * tree.c (RETURN_TRUE_IF_VAR): Return true also if
>         !TYPE_SIZES_GIMPLIFIED (type) and _t is going to be gimplified
>         into a local temporary.
>
>         * g++.dg/debug/pr54828.C: New test.
>
> --- gcc/tree.c.jj       2012-10-19 11:01:07.000000000 +0200
> +++ gcc/tree.c  2012-10-23 14:46:24.846195605 +0200
> @@ -8467,14 +8467,21 @@ variably_modified_type_p (tree type, tre
>    tree t;
>
>  /* Test if T is either variable (if FN is zero) or an expression containing
> -   a variable in FN.  */
> +   a variable in FN.  If TYPE isn't gimplified, return true also if
> +   gimplify_one_sizepos would gimplify the expression into a local
> +   variable.  */
>  #define RETURN_TRUE_IF_VAR(T)                                          \
>    do { tree _t = (T);                                                  \
>      if (_t != NULL_TREE                                                        \
>         && _t != error_mark_node                                        \
>         && TREE_CODE (_t) != INTEGER_CST                                \
>         && TREE_CODE (_t) != PLACEHOLDER_EXPR                           \
> -       && (!fn || walk_tree (&_t, find_var_from_fn, fn, NULL)))        \
> +       && (!fn                                                         \
> +           || (!TYPE_SIZES_GIMPLIFIED (type)                           \
> +               && !TREE_CONSTANT (_t)                                  \
> +               && TREE_CODE (_t) != VAR_DECL                           \
> +               && !CONTAINS_PLACEHOLDER_P (_t))                        \

Can you factor this and the variant in gimplify_one_sizepos out into
a predicate? Like is_gimple_sizepos ()?

Ok with that change.

Thanks,
Richard.

> +           || walk_tree (&_t, find_var_from_fn, fn, NULL)))            \
>        return true;  } while (0)
>
>    if (type == error_mark_node)
> --- gcc/testsuite/g++.dg/debug/pr54828.C.jj     2012-10-23 14:30:13.194012566 +0200
> +++ gcc/testsuite/g++.dg/debug/pr54828.C        2012-10-23 14:30:07.000000000 +0200
> @@ -0,0 +1,14 @@
> +// PR debug/54828
> +// { dg-do compile }
> +// { dg-options "-g" }
> +
> +struct T { T (); virtual ~T (); };
> +struct S : public virtual T { S (); virtual ~S (); };
> +int v;
> +void foo (char *);
> +
> +S::S ()
> +{
> +  char s[v];
> +  foo (s);
> +}
>
>         Jakub
diff mbox

Patch

--- gcc/tree.c.jj	2012-10-19 11:01:07.000000000 +0200
+++ gcc/tree.c	2012-10-23 14:46:24.846195605 +0200
@@ -8467,14 +8467,21 @@  variably_modified_type_p (tree type, tre
   tree t;
 
 /* Test if T is either variable (if FN is zero) or an expression containing
-   a variable in FN.  */
+   a variable in FN.  If TYPE isn't gimplified, return true also if
+   gimplify_one_sizepos would gimplify the expression into a local
+   variable.  */
 #define RETURN_TRUE_IF_VAR(T)						\
   do { tree _t = (T);							\
     if (_t != NULL_TREE							\
 	&& _t != error_mark_node					\
 	&& TREE_CODE (_t) != INTEGER_CST				\
 	&& TREE_CODE (_t) != PLACEHOLDER_EXPR				\
-	&& (!fn || walk_tree (&_t, find_var_from_fn, fn, NULL)))	\
+	&& (!fn								\
+	    || (!TYPE_SIZES_GIMPLIFIED (type)				\
+		&& !TREE_CONSTANT (_t)					\
+		&& TREE_CODE (_t) != VAR_DECL				\
+		&& !CONTAINS_PLACEHOLDER_P (_t))			\
+	    || walk_tree (&_t, find_var_from_fn, fn, NULL)))		\
       return true;  } while (0)
 
   if (type == error_mark_node)
--- gcc/testsuite/g++.dg/debug/pr54828.C.jj	2012-10-23 14:30:13.194012566 +0200
+++ gcc/testsuite/g++.dg/debug/pr54828.C	2012-10-23 14:30:07.000000000 +0200
@@ -0,0 +1,14 @@ 
+// PR debug/54828
+// { dg-do compile }
+// { dg-options "-g" }
+
+struct T { T (); virtual ~T (); };
+struct S : public virtual T { S (); virtual ~S (); };
+int v;
+void foo (char *);
+
+S::S ()
+{
+  char s[v];
+  foo (s);
+}