When the following program is compiled, the compiler aborts. int main (int argc, char **argv) { int size = 10; int i; typedef struct { char val[size]; } block; block retframe_block () { return *(block *) 0; } retframe_block (); return 0; } The message is ------------------------------------------------------- BUG.c: In function `main': BUG.c:15: internal compiler error: in assign_stack_temp_for_type, at function.c:646 Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://www.gnu.org/software/gcc/bugs.html> for instructions. -------------------------------------------------------- Release: 3.3 3.3 20020919 (experimental) Environment: System: Linux eagle 2.4.18-3 #1 Thu Apr 18 07:37:53 EDT 2002 i686 unknown Architecture: i686 host: i686-pc-linux-gnu build: i686-pc-linux-gnu target: sh-unknown-elf configured with: ../gcc/configure --target=sh-elf --prefix=/home/naveens/debug --with-ld=/home/gnu/local/bin/sh-elf-ld --with-as=/home/gnu/local/bin/sh-elf-as --with-newlib --with-libs=/home/naveens/newlib/sh-elf/lib --with-headers=/home/naveens/newlib/sh-elf/include --enable-languages=c,c++ --enable-checking=rtl How-To-Repeat: The problem occurs with i686 native, and problem is present in earlier versions too (2.95, 2.96 (Redhat)). Please repeat with the test case attached. My guess is, it will occur with any gcc build.
Fix: Still Investigating. The problem seem to occur only while allocating structure of variable size in the nested function.
State-Changed-From-To: open->analyzed State-Changed-Why: Confirmed. ICEs with 2.95, 3.0, 3.2 and 3.3
From: Dara Hazeghi <dhazeghi@yahoo.com> To: gcc-gnats@gcc.gnu.org Cc: Subject: Re: c/8081: ICE with variably sized types and nested functions Date: Fri, 9 May 2003 21:21:31 -0700 http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit- trail&database=gcc&pr=8081 Hello, this is just confirming that this bug still occurs in gcc 3.3 branch and mainline (20030509). Dara
Still exists on the mainline (20030618): pr8081.c: In function `main': pr8081.c:17: internal compiler error: in assign_stack_temp_for_type, at function.c:677 Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://gcc.gnu.org/bugs.html> for instructions.
Even 2.91.66 does not ICE.
Even 2.91.66 ICEs.
Created attachment 4681 [details] A proposed patch for the problem When a nested function,returning a structure of variable size is called it is unable to create a temporary space on the stack frame for storing the returning value. This was observed when with slight modification of the program was successfully compiled. modifications: declaration of a block b; and assigning the return value of the function to b. int main (int argc, char **argv) { int size = 10; int i; typedef struct { char val[size]; } block; block b; block retframe_block () { return *(block *) 0; } b=retframe_block (); return 0; } The obvious reason of its successful compilation is that ,there was no need of assigning a temporary space to the stack frame as the return block was returned to address of block b. This gave the inspiration of dynamically allocating the temporary space to the frame when a nested function is called without assignment. Bootstrapped and Regtested for i386 platform.ok to apply? 2003-09-02 Sitikant Sahu <sitikants@noida.hcltech.com> * calls.c (expand_call): Allocate dynamically on stack for variable size structure return (PR 8081).
Here is link to discussion thread for the proposed patch. http://gcc.gnu.org/ml/gcc/2003-09/threads.html#00388
I couldn't tell if Zack okayed the patch or not from the thread. If the patch was okayed, please install it. Also, did you get to file a bug for the other problem you found?
I still had some concerns with the latest iteration of the patch. Also I do not know if Mr. Sahu has a copyright assignment on file with the FSF. (Perhaps HCLtech/Noida has made arrangements for all of its employees, which would be nice.)
Also the patch will not fix the tree-ssa failure which is different and might be fixed by a different patch later on.
Actually I think I was wrong with respect with the tree-ssa/4.0.0.
I also filed PR 21374 for a tree dumping ICE.
*** Bug 21663 has been marked as a duplicate of this bug. ***
*** Bug 22581 has been marked as a duplicate of this bug. ***
*** Bug 23456 has been marked as a duplicate of this bug. ***
Mainline no longer ICEs on the testcase. 3.4.5 shows pr8081.c:15: internal compiler error: in assign_stack_temp_for_type, at function.c:658 while 4.0.2 now aborts in pr8081.c:15: internal compiler error: in declare_return_variable, at tree-inline.c:913 which is sort of a regression for 4.0.2 ;)
Still fails on the mainline: t.c: In function ‘main’: t.c:15: internal compiler error: in assign_stack_temp_for_type, at function.c:595 Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://gcc.gnu.org/bugs.html> for instructions.
$ gcc-4.3 --version gcc-4.3 (Debian 4.3-20080116-1) 4.3.0 20080116 (experimental) [trunk revision 131577] Copyright (C) 2007 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. $ gcc-4.3 -O2 -Wall -g foo.c foo.c: In function 'main': foo.c:5: warning: unused variable 'i' foo.c:15: internal compiler error: in declare_return_variable, at tree-inline.c:1791 Please submit a full bug report, with preprocessed source if appropriate.
reconfirmed with 4.5 20091228. please could somebody update the "Known to fail" field? Matthias $ /usr/lib/gcc-snapshot/bin/gcc foo.m foo.m: In function 'main': foo.m:15:18: internal compiler error: in assign_stack_temp_for_type, at function.c:703 Please submit a full bug report, with preprocessed source if appropriate.
Done (I'm trusting you blindly)
*** Bug 21374 has been marked as a duplicate of this bug. ***
In PR21374 it was decided to reject the testcase which makes this a C frontend issue. Testcase that still ICEs on the trunk even when optimizing: int main (int argc, char **argv) { int size = 10; typedef struct { char val[size]; } block; block b; block __attribute__((noinline)) retframe_block () { return *(block *) &b; } b=retframe_block (); return 0; } as alternative to rejecting this case we can lower returning variable-size types during un-nesting to explicit passing of a return slot and using memcpy, making the nested function return nothing.
Created attachment 26306 [details] A patch for using by-reference passing (In reply to comment #23) > as alternative to rejecting this case we can lower returning variable-size > types during un-nesting to explicit passing of a return slot and using > memcpy, making the nested function return nothing. It's of course not that easy as we gimplify before un-nesting. The frontend would be responsible to arrange things that way, similar to how we pass a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT variable). Like for the attached patch. Passes extern void abort (void); int main (int argc, char **argv) { int size = 10; typedef struct { char val[size]; } block; block a, b; block __attribute__((noinline)) retframe_block () { return *(block *) &b; } b.val[0] = -1; b.val[1] = -2; a=retframe_block (); if (a.val[0] != -1 || a.val[1] != -2) abort (); return 0; } I'm not sure if one can construct a testcase where using return-slot optimization causes wrong-code generation. Alternatively checking DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to all VLA types could work (though not for indirect calls).
> It's of course not that easy as we gimplify before un-nesting. The frontend > would be responsible to arrange things that way, similar to how we pass > a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT > variable). Like for the attached patch. Passes > > extern void abort (void); > int > main (int argc, char **argv) > { > int size = 10; > typedef struct > { > char val[size]; > } > block; > block a, b; > block __attribute__((noinline)) > retframe_block () > { > return *(block *) &b; > } > b.val[0] = -1; > b.val[1] = -2; > a=retframe_block (); > if (a.val[0] != -1 > || a.val[1] != -2) > abort (); > return 0; > } > > I'm not sure if one can construct a testcase where using return-slot > optimization causes wrong-code generation. Alternatively checking > DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to > all VLA types could work (though not for indirect calls). You should ask specialists. :-) In Ada, we do this routinely and the strategy used is that of the "forced RSO": we generate INIT_EXPR instead of MODIFY_EXPR and we create an explicit temporary if we detect potential overlap. Btw, I don't understand why you're mixing DECL_BY_REFERENCE and RSO here, just Index: gimplify.c =================================================================== --- gimplify.c (revision 183104) +++ gimplify.c (working copy) @@ -4417,6 +4417,9 @@ gimplify_modify_expr_rhs (tree *expr_p, /* It's OK to use the target directly if it's being initialized. */ use_target = true; + else if (variably_modified_type_p (TREE_TYPE (*to_p), NULL_TREE)) + /* Always use the target for variable-sized types. */ + use_target = true; else if (TREE_CODE (*to_p) != SSA_NAME && (!is_gimple_variable (*to_p) || needs_to_live_in_memory (*to_p))) works for me on the testcase.
On Fri, 13 Jan 2012, ebotcazou at gcc dot gnu.org wrote: > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081 > > Eric Botcazou <ebotcazou at gcc dot gnu.org> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > CC| |ebotcazou at gcc dot > | |gnu.org > > --- Comment #25 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-13 08:36:56 UTC --- > > It's of course not that easy as we gimplify before un-nesting. The frontend > > would be responsible to arrange things that way, similar to how we pass > > a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT > > variable). Like for the attached patch. Passes > > > > extern void abort (void); > > int > > main (int argc, char **argv) > > { > > int size = 10; > > typedef struct > > { > > char val[size]; > > } > > block; > > block a, b; > > block __attribute__((noinline)) > > retframe_block () > > { > > return *(block *) &b; > > } > > b.val[0] = -1; > > b.val[1] = -2; > > a=retframe_block (); > > if (a.val[0] != -1 > > || a.val[1] != -2) > > abort (); > > return 0; > > } > > > > I'm not sure if one can construct a testcase where using return-slot > > optimization causes wrong-code generation. Alternatively checking > > DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to > > all VLA types could work (though not for indirect calls). > > You should ask specialists. :-) In Ada, we do this routinely and the strategy > used is that of the "forced RSO": we generate INIT_EXPR instead of MODIFY_EXPR > and we create an explicit temporary if we detect potential overlap. > > Btw, I don't understand why you're mixing DECL_BY_REFERENCE and RSO here, just > > Index: gimplify.c > =================================================================== > --- gimplify.c (revision 183104) > +++ gimplify.c (working copy) > @@ -4417,6 +4417,9 @@ gimplify_modify_expr_rhs (tree *expr_p, > /* It's OK to use the target directly if it's being > initialized. */ > use_target = true; > + else if (variably_modified_type_p (TREE_TYPE (*to_p), NULL_TREE)) > + /* Always use the target for variable-sized types. */ > + use_target = true; > else if (TREE_CODE (*to_p) != SSA_NAME > && (!is_gimple_variable (*to_p) > || needs_to_live_in_memory (*to_p))) > > works for me on the testcase. Ah, ok. So I suppose the Frontend could force RSO here as well by just setting CALL_EXPR_RETURN_SLOT_OPT on the CALL_EXPR. Not sure which approach is better.
(In reply to comment #26) > On Fri, 13 Jan 2012, ebotcazou at gcc dot gnu.org wrote: > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081 > > > > Eric Botcazou <ebotcazou at gcc dot gnu.org> changed: > > > > What |Removed |Added > > ---------------------------------------------------------------------------- > > CC| |ebotcazou at gcc dot > > | |gnu.org > > > > --- Comment #25 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-13 08:36:56 UTC --- > > > It's of course not that easy as we gimplify before un-nesting. The frontend > > > would be responsible to arrange things that way, similar to how we pass > > > a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT > > > variable). Like for the attached patch. Passes > > > > > > extern void abort (void); > > > int > > > main (int argc, char **argv) > > > { > > > int size = 10; > > > typedef struct > > > { > > > char val[size]; > > > } > > > block; > > > block a, b; > > > block __attribute__((noinline)) > > > retframe_block () > > > { > > > return *(block *) &b; > > > } > > > b.val[0] = -1; > > > b.val[1] = -2; > > > a=retframe_block (); > > > if (a.val[0] != -1 > > > || a.val[1] != -2) > > > abort (); > > > return 0; > > > } > > > > > > I'm not sure if one can construct a testcase where using return-slot > > > optimization causes wrong-code generation. Alternatively checking > > > DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to > > > all VLA types could work (though not for indirect calls). > > > > You should ask specialists. :-) In Ada, we do this routinely and the strategy > > used is that of the "forced RSO": we generate INIT_EXPR instead of MODIFY_EXPR > > and we create an explicit temporary if we detect potential overlap. > > > > Btw, I don't understand why you're mixing DECL_BY_REFERENCE and RSO here, just > > > > Index: gimplify.c > > =================================================================== > > --- gimplify.c (revision 183104) > > +++ gimplify.c (working copy) > > @@ -4417,6 +4417,9 @@ gimplify_modify_expr_rhs (tree *expr_p, > > /* It's OK to use the target directly if it's being > > initialized. */ > > use_target = true; > > + else if (variably_modified_type_p (TREE_TYPE (*to_p), NULL_TREE)) > > + /* Always use the target for variable-sized types. */ > > + use_target = true; > > else if (TREE_CODE (*to_p) != SSA_NAME > > && (!is_gimple_variable (*to_p) > > || needs_to_live_in_memory (*to_p))) > > > > works for me on the testcase. > > Ah, ok. So I suppose the Frontend could force RSO here as well by > just setting CALL_EXPR_RETURN_SLOT_OPT on the CALL_EXPR. Not sure > which approach is better. OTOH as GIMPLE cannot deal with VLAs on the LHS of a CALL when not applying RSO the above is correct anyway.
> OTOH as GIMPLE cannot deal with VLAs on the LHS of a CALL when not applying RSO > the above is correct anyway. Right, gimplify_return_expr already has a similar provision: else if (aggregate_value_p (result_decl, TREE_TYPE (current_function_decl))) { if (TREE_CODE (DECL_SIZE (result_decl)) != INTEGER_CST) { if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (result_decl))) gimplify_type_sizes (TREE_TYPE (result_decl), pre_p); /* Note that we don't use gimplify_vla_decl because the RESULT_DECL should be effectively allocated by the caller, i.e. all calls to this function must be subject to the Return Slot Optimization. */ gimplify_one_sizepos (&DECL_SIZE (result_decl), pre_p); gimplify_one_sizepos (&DECL_SIZE_UNIT (result_decl), pre_p); } result = result_decl; }
Mine.
Author: rguenth Date: Fri Jan 13 12:05:27 2012 New Revision: 183153 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183153 Log: 2012-01-13 Richard Guenther <rguenther@suse.de> PR middle-end/8081 * gimplify.c (gimplify_modify_expr_rhs): For calls with a variable-sized result always use RSO. * gcc.dg/torture/pr8081.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/torture/pr8081.c Modified: trunk/gcc/ChangeLog trunk/gcc/gimplify.c trunk/gcc/testsuite/ChangeLog
Fixed for GCC 4.7.