| Summary: | [3.4 regression] ICE in find_function_data | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Andreas Schwab <schwab> |
| Component: | c++ | Assignee: | Richard Henderson <rth> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gcc-bugs, janis187, reichelt, rth |
| Priority: | P1 | Keywords: | ice-on-valid-code |
| Version: | 3.4.0 | ||
| Target Milestone: | 3.4.0 | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | ||
| Attachments: | cube.ii | ||
|
Description
Andreas Schwab
2002-06-11 08:26:02 UTC
From: Reichelt <reichelt@igpm.rwth-aachen.de> To: adrian@suse.de, gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org, schwab@suse.de Cc: Subject: Re: middle-end/6994: ICE in find_function_data Date: Wed, 12 Jun 2002 16:18:03 +0200 Hi, the bug can be reproduced with the following code snippet (just compile with g++ -c): ----------------------------snip here------------------------ class A { A( int ); }; A::A ( int i ) { int ar[1][i]; ar[0][0] = 0; } ----------------------------snip here------------------------ The bug is a regression from gcc 2.95.3 (it first appears in gcc 3.0 and is still present in the main trunk - checked on mips-sgi-irix6.5). Greetings, Volker Reichelt http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=6994 From: Reichelt <reichelt@igpm.rwth-aachen.de> To: adrian@suse.de, gcc-gnats@gcc.gnu.org, gcc-bugs@gcc.gnu.org, nobody@gcc.gnu.org, schwab@suse.de Cc: Subject: Re: middle-end/6994: ICE in find_function_data Date: Wed, 12 Jun 2002 16:18:03 +0200 Hi, the bug can be reproduced with the following code snippet (just compile with g++ -c): ----------------------------snip here------------------------ class A { A( int ); }; A::A ( int i ) { int ar[1][i]; ar[0][0] = 0; } ----------------------------snip here------------------------ The bug is a regression from gcc 2.95.3 (it first appears in gcc 3.0 and is still present in the main trunk - checked on mips-sgi-irix6.5). Greetings, Volker Reichelt http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=6994 From: Janis Johnson <janis187@us.ibm.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 Date: Wed, 9 Oct 2002 16:00:59 -0700 This problem still exists in 3.3 20021008 and 3.2.1 20021009 on i686-pc-linux-gnu. As mentioned earlier, it's a regression from GCC 2.95.3. The PR name ought to be "c++" rather than "middle-end". 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. I don't where to go from here, but hope that this information will be of use to someone familiar with the C++ front end. Janis 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; +} Responsible-Changed-From-To: unassigned->zack Responsible-Changed-Why: Working on this. State-Changed-From-To: open->analyzed State-Changed-Why: Fix in hand; only needs a bit more testing. State-Changed-From-To: analyzed->closed State-Changed-Why: Fixed in 3.2.1 and 3.3. Richard's patch http://gcc.gnu.org/ml/gcc-cvs/2003-09/msg00666.html makes the testcase testsuite/g++.dg/ext/vla1.C fail again, but the underlying problem is a deeper one, according to him (excerpt from a private mail from Richard): | My patch merely exposed a previous problem via segv. | | It *should* have been failing before. I don't know why it wasn't. | Note that by reverting my change, you get no error on line 12. | | The trees being produced by the c++ front end in this case are | completely bogus. It is mere accident that the bogus trees were | being remapped to error_mark_node, and thus silently ignored by | the middle-end. | | The front end should be generating an error message and forcing | error_mark_node there. Richard, you were wondering why vla1.C wasn't failing before: | It *should* have been failing before. I don't know why it wasn't. | Note that by reverting my change, you get no error on line 12. You probably didn't see the error because you only get it with "-pedantic" (the variable sized arrays are a GCC extension). The testsuite seems to have "-pedantic" enabled by default. So the testsuite was correct then. I think this may be a second copy_body problem that Jason has identified, but has not yet fixed. I'll make sure to get this done. fixed by:
* tree-inline.c (remap_type): New.
(remap_decl): Use it. Remap DECL_SIZE*.
(copy_body_r): Use it.
(walk_tree): Walk TREE_TYPE too.
(copy_tree_r): Don't walk subtrees of types.
* tree.c (variably_modified_type_p): Restructure. Consider integer
types with non-const bounds variably modified.
|