Bug 42051 - [OOP] ICE on array-valued function with CLASS formal argument
Summary: [OOP] ICE on array-valued function with CLASS formal argument
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Mikael Morin
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2009-11-15 16:24 UTC by Damian Rouson
Modified: 2011-08-16 21:22 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.6.0
Known to fail: 4.5.0 fortran-dev
Last reconfirmed: 2009-12-03 21:28:35


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Damian Rouson 2009-11-15 16:24:48 UTC
Gfortran 4.5.0 20091106 (with Janus's patch for PR42048) produces the following on the code below:

gfortran -c field_grid.f03 
field_grid.f03: In function ‘output’:
field_grid.f03:27:0: internal compiler error: in gfc_conv_variable, at fortran/trans-expr.c:550
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
 

module grid_module
  implicit none 

  type grid
    real ,dimension(1) :: x=1.
  contains
    procedure :: return_x
  end type 

contains

  function return_x(this) result(this_x)
    class(grid) ,intent(in) :: this
    real  ,dimension(1) :: this_x
    this_x = this%x
  end function

end module 

module field_module
  use grid_module, only : grid 
  implicit none 

  type field
    type(grid) :: mesh
  contains
    procedure :: output
  end type 

contains

  subroutine output(this)
    class(field) ,intent(in) :: this
    real ,dimension(:) ,allocatable :: x
    x=this%mesh%return_x()
  end subroutine

end module
Comment 1 janus 2009-11-15 18:30:31 UTC
(In reply to comment #0)
> field_grid.f03:27:0: internal compiler error: in gfc_conv_variable, at
> fortran/trans-expr.c:550

Here is a reduced test case which gives the same ICE:


  type grid
  end type 

contains

  function return_x(this) result(this_x)
    class(grid) :: this
    real  ,dimension(1) :: this_x
  end function

  subroutine output()
    type(grid) :: mesh
    real ,dimension(1) :: x
    x = return_x(mesh)
  end subroutine

end


So it seems this is neither connected to TBPs, nor to ALLOCATABLE.

The ICE goes away however, if I remove the DIMENSION attributes, or if I make the CLASS argument a TYPE.
Comment 2 janus 2009-11-15 18:49:37 UTC
Backtrace:

#0  gfc_conv_variable (se=0x7fffffffd860, expr=0x1788a00) at /home/jweil/gcc45/trunk/gcc/fortran/trans-expr.c:550
#1  0x000000000059ab0a in gfc_conv_expr (se=0x7fffffffd860, expr=0x1788a00) at /home/jweil/gcc45/trunk/gcc/fortran/trans-expr.c:4322
#2  0x000000000059adde in gfc_conv_expr_reference (se=0x7fffffffd860, expr=0x1788a00) at /home/jweil/gcc45/trunk/gcc/fortran/trans-expr.c:4413
#3  0x0000000000595a88 in gfc_conv_procedure_call (se=0x7fffffffdc00, sym=0x17823b0, arg=0x17337b0, expr=0x1788ad0, append_args=0x0)
    at /home/jweil/gcc45/trunk/gcc/fortran/trans-expr.c:2813
Comment 3 Paul Thomas 2009-12-03 21:28:35 UTC
The smaller testcase of comment #1 is fixed with

Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c	(revision 154935)
+++ gcc/fortran/trans-expr.c	(working copy)
@@ -2446,12 +2446,14 @@
   ss = gfc_walk_expr (e);
   if (ss == gfc_ss_terminator)
     {
+      parmse->ss = NULL;
       gfc_conv_expr_reference (parmse, e);
       tmp = fold_convert (TREE_TYPE (ctree), parmse->expr);
       gfc_add_modify (&parmse->pre, ctree, tmp);
     }
   else
     {
+      parmse->ss = ss;
       gfc_conv_expr (parmse, e);
       gfc_add_modify (&parmse->pre, ctree, parmse->expr);
     }

The original fails because the vtable cannot be found.  THis is due to:
use grid_module, only : grid 

Removing the ",only : grid" restores correct linkage.

I knew that we would hit this before long, so now is as good a time as any to fix it:-)

Cheers

Paul 
Comment 4 Paul Thomas 2009-12-03 21:46:42 UTC
(In reply to comment #3)

> The original fails because the vtable cannot be found.  This is due to:
> use grid_module, only : grid 

This is true of trunk:
/usr/lib/../lib64/crt1.o: In function `_start':
(.text+0x20): undefined reference to `main'
/tmp/ccwercw1.o: In function `__field_module_MOD_output':
pr42051.f03:(.text+0x84): undefined reference to `vtab$grid.1611'
collect2: ld returned 1 exit status

fortran-dev ICEs at line 372 in trans-array.c.  *sigh*

The reduced testcase is fine with both.

Paul 
Comment 5 Dominique d'Humieres 2009-12-04 13:17:53 UTC
> The smaller testcase of comment #1 is fixed with

Confirmed

> The original fails because the vtable cannot be found.  THis is due to:
> use grid_module, only : grid 
>
> Removing the ",only : grid" restores correct linkage.

I don't see any difference with/without  ",only : grid". In both cases I get:

[trunk revision 154654]
pr42051*.f90:27:0: internal compiler error: in gfc_conv_variable, at fortran/trans-expr.c:550

[branch fortran-dev revision 154936] and [trunk revision 154973p4v]
f951: internal compiler error: in gfc_build_null_descriptor, at fortran/trans-array.c:372

Comment 6 janus 2010-03-02 23:31:38 UTC
Here is another very similar example:

  implicit none
  type :: lorenz
  end type lorenz
  type(lorenz) :: attractor
  print *,output(attractor)

contains

  function output(this) result(coordinates)
    class(lorenz), intent(in) :: this
    real, dimension(:), allocatable :: coordinates
  end function output

end


main.f90: In function ‘MAIN__’:
main.f90:13:0: internal compiler error: in gfc_conv_variable, at fortran/trans-expr.c:548


This has been extracted from the code found in http://dx.doi.org/10.1145/1644001.1644004.
Comment 7 janus 2010-06-11 02:50:19 UTC
At r160588 of gfortran 4.6 trunk, the patch in comment #3 fixes the codes in comment #1 and #6, but comment #0 seems to get stuck in some kind of infinite loop.
Comment 8 janus 2010-06-11 02:56:27 UTC
(In reply to comment #7)
> At r160588 of gfortran 4.6 trunk, the patch in comment #3 fixes the codes in
> comment #1 and #6, but comment #0 seems to get stuck in some kind of infinite
> loop.

Btw, it also fixes the test cases in PR43896, which is a duplicate.
Comment 9 janus 2010-06-11 16:46:29 UTC
Subject: Bug 42051

Author: janus
Date: Fri Jun 11 16:45:48 2010
New Revision: 160622

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=160622
Log:
2010-06-11  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/42051
	PR fortran/43896
	* trans-expr.c (gfc_conv_derived_to_class): Handle array-valued
	functions with CLASS formal arguments.


2010-06-11  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/42051
	PR fortran/43896
	* gfortran.dg/class_23.f03: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/class_23.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog

Comment 10 janus 2010-06-15 19:08:32 UTC
Comment #1 is fixed by r160622, but the original test case still does not work (hangs in some kind of infinite loop for me, at r160801 on x86_64-unknown-linux-gnu).
Comment 11 janus 2010-06-15 19:38:37 UTC
Here is a reduced test case, based on comment #0:

module grid_module
  implicit none 
  type grid
  end type
  type field
    type(grid) :: mesh
  end type
contains
  real function return_x(this)
    class(grid) :: this
  end function
end module 

module field_module
  use grid_module, only: field,return_x
  implicit none 
contains
  subroutine output(this)
    class(field) :: this
    print *,return_x(this%mesh)
  end subroutine
end module

end


This gives me a segfault ICE.
Comment 12 janus 2010-06-15 19:41:20 UTC
(In reply to comment #11)
> This gives me a segfault ICE.

... with the following backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000553b5d in gfc_find_symtree (st=0x41, name=0x7ffff7f2de50 "vtype$grid") at /home/jweil/gcc46/trunk/gcc/fortran/symbol.c:2390
2390          c = strcmp (name, st->name);
(gdb) bt
#0  0x0000000000553b5d in gfc_find_symtree (st=0x41, name=0x7ffff7f2de50 "vtype$grid") at /home/jweil/gcc46/trunk/gcc/fortran/symbol.c:2390
#1  0x0000000000553b08 in gfc_delete_symtree (root=0x18081d0, name=0x7ffff7f2de50 "vtype$grid") at /home/jweil/gcc46/trunk/gcc/fortran/symbol.c:2371
#2  0x0000000000554772 in gfc_undo_symbols () at /home/jweil/gcc46/trunk/gcc/fortran/symbol.c:2843
#3  0x0000000000515f65 in decode_statement () at /home/jweil/gcc46/trunk/gcc/fortran/parse.c:271
#4  0x00000000005177be in next_free () at /home/jweil/gcc46/trunk/gcc/fortran/parse.c:723
#5  0x0000000000517b8e in next_statement () at /home/jweil/gcc46/trunk/gcc/fortran/parse.c:908
#6  0x000000000051c190 in gfc_parse_file () at /home/jweil/gcc46/trunk/gcc/fortran/parse.c:4299


Seems to be related to PR 44064 (or even a duplicate).
Comment 13 Mikael Morin 2010-06-27 13:14:05 UTC
Hello, 

The problem is that the oop code can add new symbols to the symtrees at resolution or code generation time, which is unexpected. 
gfc_get_sym_tree (and thus gfc_get_symbol) has the side effect of keeping the new/changed symbols in the changed_syms pointer chain.
When code generation is done, the namespace is freed, but changed_syms is still pointing to the oop-generated (and freed) symbols. 
if we are not at the end of file at that point, we will continue parsing and the next gfc_undo_symbol call will try to free these already freed symbols. 

To fix this, we should either call gfc_commit_symbols/gfc_undo_symbols when done, or use a custom combination of gfc_new_symbol/gfc_new_symtree/gfc_find_symbol instead of plain gfc_get_symbol (that's how it is done for example in gfc_build_class_symbol).

The patch at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43829#c20 uses the first solution in gfc_find_derived_vtab. I don't know, however if it will catch all cases, or whether there would be more appropriate places for it. 

Another solution would be to take changed_syms into account for reference counting (gfc_symbol's refs member) but that would probably mean unreleased memory in the end. 


I'm leaving this assigned to Janus because, as OOP master, he knows best the place(s) where the change(s) have to be applied, for better cleanness, bullet-proof-ness, and any-other-reasons-ness. 
Comment 14 Mikael Morin 2010-06-27 13:16:27 UTC
(In reply to comment #13)
> I'm leaving this assigned to Janus because, as OOP master, he knows best the
> place(s) where the change(s) have to be applied, for better cleanness,
> bullet-proof-ness, and any-other-reasons-ness. 
> 

Well, in fact it is not assigned to Janus.
Anyway I'm leaving as is.
Comment 15 paul.richard.thomas@gmail.com 2010-06-27 14:39:36 UTC
Subject: Re:  [OOP] ICE on array-valued function with CLASS 
	formal argument

> ------- Comment #14 from mikael at gcc dot gnu dot org  2010-06-27 13:16 -------
> (In reply to comment #13)
>> I'm leaving this assigned to Janus because, as OOP master, he knows best the
>> place(s) where the change(s) have to be applied, for better cleanness,
>> bullet-proof-ness, and any-other-reasons-ness.
>>
>
> Well, in fact it is not assigned to Janus.
> Anyway I'm leaving as is.

I tell you what - I'll unassign myself.  I am too tied up with daytime
work to contribute over much to gfortran.

What time I have, must go to array descriptors and the backlog of
fixes that are ready to go.

That said, I have been meaning to write to Janus about the overall
direction of OOP before he gets into the Summer of Code.  I will do
that today :-)

Cheers

Paul


Comment 16 Daniel Franke 2010-07-15 21:37:17 UTC
> > (In reply to comment #13)
> >> I'm leaving this assigned to Janus because, as OOP master, he knows best the
> >> place(s) where the change(s) have to be applied, for better cleanness,
> >> bullet-proof-ness, and any-other-reasons-ness.
> >
> > Well, in fact it is not assigned to Janus.
> 
> I tell you what - I'll unassign myself.  I am too tied up with daytime
> work to contribute over much to gfortran.

Reassigning to Janus then ...
Comment 17 Mikael Morin 2010-07-28 20:01:05 UTC
(In reply to comment #16)
> Reassigning to Janus then ...
> 
I will submit my patch.
Comment 18 Mikael Morin 2010-07-29 11:23:02 UTC
Subject: Bug 42051

Author: mikael
Date: Thu Jul 29 11:22:40 2010
New Revision: 162674

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162674
Log:
2010-07-29  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/42051
	PR fortran/44064
	* class.c (gfc_find_derived_vtab): Accept or discard newly created
	symbols before returning.

2010-07-29  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/42051
	PR fortran/44064
	* gfortran.dg/pr42051.f03: New testcase.


Added:
    trunk/gcc/testsuite/gfortran.dg/pr42051.f03
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/class.c
    trunk/gcc/testsuite/ChangeLog

Comment 19 Mikael Morin 2010-07-29 11:52:35 UTC
Backport on 4.5 pending...
Comment 20 Mikael Morin 2010-07-31 10:27:53 UTC
Subject: Bug 42051

Author: mikael
Date: Sat Jul 31 10:27:36 2010
New Revision: 162776

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162776
Log:
2010-07-31  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/42051
	PR fortran/44064
	* symbol.c (changed_syms): Made non-static.
	* parse.c (changed_syms): Declare new external. 
	(next_statement): Assert changed_syms is NULL at the beginning.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/parse.c
    trunk/gcc/fortran/symbol.c

Comment 21 Mikael Morin 2010-08-02 15:31:08 UTC
Subject: Bug 42051

Author: mikael
Date: Mon Aug  2 15:30:47 2010
New Revision: 162821

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162821
Log:
2010-08-02  Mikael Morin  <mikael@gcc.gnu.org>
	    Janus Weil  <janus@gcc.gnu.org>

	PR fortran/42051
	PR fortran/44064
	PR fortran/45151
	* intrinsic.c (gfc_get_intrinsic_sub_symbol): Commit changed symbol. 
	* symbol.c (gen_cptr_param, gen_fptr_param, gen_shape_param,
	gfc_copy_formal_args, gfc_copy_formal_args_intr,
	gfc_copy_formal_args_ppc, generate_isocbinding_symbol): Ditto.
	* parse.c (parse_derived_contains, parse_spec, parse_progunit): 
	Call reject_statement in case of error. 
	(match_deferred_characteritics): Call gfc_undo_symbols in case match
	fails.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/intrinsic.c
    trunk/gcc/fortran/parse.c
    trunk/gcc/fortran/symbol.c

Comment 22 Mikael Morin 2010-08-04 14:17:59 UTC
Subject: Bug 42051

Author: mikael
Date: Wed Aug  4 14:17:31 2010
New Revision: 162865

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162865
Log:
2010-08-04  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/42051
	PR fortran/44064
	* symbol.c (changed_syms): Made static again.
	(gfc_symbol_state): Don't conditionalize on GFC_DEBUG. 
	Changed conditional internal error into assert.
	Rename function to ...
	(gfc_enforce_clean_symbol_state): ... this.
	* gfortran.h (gfc_symbol_state, gfc_enforce_clean_symbol_state): 
	Rename the former to the latter.
	* parse.c (decode_statement, decode_omp_directive,
	decode_gcc_attribute): Update callers accordingly. Don't conditionalize
	on GFC_DEBUG.
	(changed_syms): Remove declaration.
	(next_statement): Use gfc_enforce_clean_symbol_state.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/parse.c
    trunk/gcc/fortran/symbol.c

Comment 23 janus 2010-08-05 12:11:35 UTC
For me all the test cases seems to be working now. Mikael, can we close this?
Comment 24 Mikael Morin 2010-08-05 12:59:13 UTC
(In reply to comment #23)
> For me all the test cases seems to be working now. Mikael, can we close this?
> 

I'll backport the fixes to 4.5.
Comment 25 Mikael Morin 2010-08-05 21:08:53 UTC
Subject: Bug 42051

Author: mikael
Date: Thu Aug  5 21:08:36 2010
New Revision: 162921

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162921
Log:
2010-08-05  Mikael Morin  <mikael@gcc.gnu.org>
	    Janus Weil  <janus@gcc.gnu.org>

	PR fortran/42051
	PR fortran/44064
	PR fortran/45151
	* intrinsic.c (gfc_get_intrinsic_sub_symbol): Commit changed symbol. 
	* symbol.c (gen_cptr_param, gen_fptr_param, gen_shape_param,
	gfc_copy_formal_args, gfc_copy_formal_args_intr,
	gfc_copy_formal_args_ppc, generate_isocbinding_symbol): Ditto.
	(gfc_find_derived_vtab): Commit newly created symbols.
	* parse.c (parse_derived_contains, parse_spec, parse_progunit): 
	Call reject_statement in case of error. 
	(match_deferred_characteritics): Call gfc_undo_symbols in case match
	fails.


Modified:
    branches/gcc-4_5-branch/gcc/fortran/ChangeLog
    branches/gcc-4_5-branch/gcc/fortran/intrinsic.c
    branches/gcc-4_5-branch/gcc/fortran/parse.c
    branches/gcc-4_5-branch/gcc/fortran/symbol.c

Comment 26 Mikael Morin 2010-08-05 21:11:05 UTC
Done
Comment 27 janus 2011-08-16 21:22:36 UTC
Author: janus
Date: Tue Aug 16 21:22:31 2011
New Revision: 177800

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=177800
Log:
2011-08-16  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/42051
	PR fortran/43896
	PR fortran/49962
	* trans-expr.c (gfc_conv_derived_to_class): Handle array-valued
	functions with CLASS formal arguments.


2011-08-16  Paul Thomas  <pault@gcc.gnu.org>

	PR fortran/42051
	PR fortran/43896
	PR fortran/49962
	* gfortran.dg/class_23.f03: New test.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gfortran.dg/class_23.f03
Modified:
    branches/gcc-4_5-branch/gcc/fortran/ChangeLog
    branches/gcc-4_5-branch/gcc/fortran/trans-expr.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog