Re: middle-end/6994: ICE in find_function_data (VLA types and SAVE_EXPRs and clones, oh my!)

Re: middle-end/6994: ICE in find_function_data (VLA types and SAVE_EXPRs and clones, oh my!)

[Zack Weinberg]

The following reply was made to PR middle-end/6994; it has been noted by GNATS.

From: Zack Weinberg <zack@codesourcery.com>
To: gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, adrian@suse.de,
	schwab@suse.de
Cc:  
Subject: Re: middle-end/6994: ICE in find_function_data (VLA types and SAVE_EXPRs and clones, oh my!)
Date: Wed, 9 Oct 2002 23:47:31 -0700

 On Wed, Oct 09, 2002 at 04:00:59PM -0700, Janis Johnson wrote:
 > At the time of the ICE in find_function_data, 'exp' in expand_expr is
 > a SAVE_EXPR whose function context, 'context', is cloned from
 > 'current_function_decl'.  The code in expand_expr falsely assumes that
 > since 'context' is different from 'current_function_decl', 'context' is
 > a containing function.  It calls find_function_data just to make sure
 > that 'context' is for a containing function, but it's not so that
 > function aborts.
 
 This is a shortened version of the tree structure causing the problem.
 
 <array_ref
     <array_ref
         type <array_type
             size <plus
                 <mult
                     <save 0x401a4040
                         <plus
                             <parm_decl 0x4019fee0 i>
                             <integer_cst -1>>
                         <function_decl A>>
                     <integer_cst 32>>
                  <integer_cst 32>>
             unit size <plus
                 <save 0x401a4320
                     <plus
                         <mult
 			    <parm_decl 0x4019fee0 i>
 			    <integer_cst 4>>
                         <integer_cst -4>>
                     <function_decl A>>
                 <integer_cst 4>>>
        
         <var_decl ar
             size <plus
                 <mult
                     <save 0x401a4900
 			<plus
 			    <parm_decl 0x4019fd90 i>
 			    <integer_cst -1>>
 			<function_decl __base_ctor>>
                     <integer_cst 32>>
                 <integer_cst 32>>
             unit size <plus
                 <save 0x401a49c0
                     <plus
                         <mult
 			    <parm_decl 0x4019fd90 i>
 			    <integer_cst 4>>
 			<integer_cst -4>>
 		    <function_decl __base_ctor>>
                 <integer_cst 4>>>
         <integer_cst 0>>
     <integer_cst 0>>
 
 Please note the presence of SAVE_EXPRs within the size and unit size
 fields of the ARRAY_TYPE at the top.  Note also how very similar those
 fields are to the analogous fields of the VAR_DECL just below.  In
 fact, this ARRAY_TYPE started out life as the type of that VAR_DECL:
 
   int ar[1][i];
 
 where 'i' is a parameter.  The differences are precisely that the
 SAVE_EXPRs and PARM_DECLs inside the type have not been updated to
 point to the right nodes for the clone.  Instead, they are left
 referring to the progenitor, which is what causes find_function_data
 to crash.
 
 The routine responsible for updating SAVE_EXPRs in clones is
 remap_save_expr, in tree-inline.c.  It didn't update those SAVE_EXPRs
 because it was never applied to them.  The reason for this is first
 that copy_trees_r explicitly assumes that TYPEs do not need copying:
 
     /* There's no need to copy types, or anything beneath them.  */
     *walk_subtrees = 0;
 
 This is clearly false for VLA types, such as the one we have here.
 Disabling that is not good enough, though; walk_tree processes neither
 the 'size' nor 'unit size' fields of struct tree_type.  Adding them
 causes this test case to be compiled successfully.  Hence the appended
 patch.
 
 I am not sure if this patch is either correct or complete.  It may be
 that we really do want to enforce the constraint that SAVE_EXPRs or
 other constructs needing fixups never appear inside TYPE nodes; it may
 be that, while some TYPE nodes need copying, others must not be; and
 it may be that we need to consider even more fields of struct
 tree_type in walk_tree.  Comments?
 
 zw
 
 	PR middle-end/6994
 	* tree-inline.c (walk_tree): For nodes of class 't', walk the
 	TYPE_SIZE and TYPE_SIZE_UNIT fields.
 	(copy_tree_r): Do copy types and things beneath them.
 
 	* g++.dg/ext/vla1.C: New test case.
 
 ===================================================================
 Index: tree-inline.c
 --- tree-inline.c	27 Sep 2002 12:48:04 -0000	1.31
 +++ tree-inline.c	10 Oct 2002 06:27:07 -0000
 @@ -1516,6 +1516,11 @@ walk_tree (tp, func, data, htab_)
      {
        WALK_SUBTREE_TAIL (TREE_TYPE (*tp));
      }
 +  else if (TREE_CODE_CLASS (code) == 't')
 +    {
 +      WALK_SUBTREE (TYPE_SIZE (*tp));
 +      WALK_SUBTREE (TYPE_SIZE_UNIT (*tp));
 +    }
  
    result = (*lang_hooks.tree_inlining.walk_subtrees) (tp, &walk_subtrees, func,
  						      data, htab);
 @@ -1683,9 +1688,11 @@ copy_tree_r (tp, walk_subtrees, data)
  	TREE_CHAIN (*tp) = chain;
  #endif /* INLINER_FOR_JAVA */
      }
 +#if 0  /* Not true in the presence of variably-sized types. */
    else if (TREE_CODE_CLASS (code) == 't')
      /* There's no need to copy types, or anything beneath them.  */
      *walk_subtrees = 0;
 +#endif
  
    return NULL_TREE;
  }
 ===================================================================
 Index: testsuite/g++.dg/ext/vla1.C
 --- testsuite/g++.dg/ext/vla1.C	1 Jan 1970 00:00:00 -0000
 +++ testsuite/g++.dg/ext/vla1.C	10 Oct 2002 06:40:29 -0000
 @@ -0,0 +1,16 @@
 +// { dg-do compile }
 +
 +// Check that (a) the VLA declaration provokes an error, and
 +// (b) the compiler does not crash.  See PR middle-end/6994.
 +
 +class A
 +{
 +  A (int);
 +};
 +
 +A::A (int i)
 +{
 +  int ar[1][i];  // { dg-error "variable-size array" }
 +
 +  ar[0][0] = 0;
 +}