Bug 35830 - ICE with PROCEDURE(<interface>) containing array formal arguments
Summary: ICE with PROCEDURE(<interface>) containing array formal arguments
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.4.0
: P3 normal
Target Milestone: ---
Assignee: Tobias Burnus
URL:
Keywords: ice-on-valid-code, wrong-code
Depends on:
Blocks:
 
Reported: 2008-04-05 08:14 UTC by Tobias Burnus
Modified: 2008-06-08 07:50 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-05-31 09:17:07


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2008-04-05 08:14:10 UTC
The following program gives an ICE w/ 4.3.0 and 4.4.0.

ff.f90:9: internal compiler error: Segmentation fault

==29131== Invalid read of size 4
==29131==    at 0x4C0459: gfc_is_nodesc_array (trans-types.c:1031)
==29131==    by 0x4C2DE5: gfc_sym_type (trans-types.c:1576)
==29131==    by 0x4C2FD7: gfc_get_function_type (trans-types.c:2009)
==29131==    by 0x4C3023: gfc_get_function_type (trans-types.c:2005)

program test
  implicit none
  interface
    subroutine one(a)
      integer a(:)
    end subroutine one
  end interface
contains
  subroutine foo(f)
    procedure(one) :: f
  end subroutine foo
end program test
Comment 1 Tobias Burnus 2008-04-05 14:56:59 UTC
(Problem was found when creating PR 35831.)

Janus, do you have time to look at it?

The invalid read happens for in gfc_is_nodesc_array. The problem is that sym->attr.dimension == 1, sym->dummy == 1 but sym->as == NULL. This of cause fails for:
      if (sym->as->type != AS_ASSUMED_SHAPE)
Hereby, sym->name is the dummy argument "a".

First attempt of solving. This gets past the ICE, but the produced code which gives wrong results.
----------------------
--- symbol.c    (revision 133937)
+++ symbol.c    (working copy)
@@ -3626,5 +3643,6 @@ add_proc_interface (gfc_symbol *sym, ifs
    args based on the args of a given named interface.  */

-void copy_formal_args (gfc_symbol *dest, gfc_symbol *src)
+void
+copy_formal_args (gfc_symbol *dest, gfc_symbol *src)
 {
   gfc_formal_arglist *head = NULL;
@@ -3649,4 +3667,5 @@ void copy_formal_args (gfc_symbol *dest,
       formal_arg->sym->attr = curr_arg->sym->attr;
       formal_arg->sym->ts = curr_arg->sym->ts;
+      formal_arg->sym->as = curr_arg->sym->as;

       /* If this isn't the first arg, set up the next ptr.  For the
----------------------

That the result is wrong can be seen with the following program. ubound of the array is 32 instead of 3:
----------------------
module m
contains
  subroutine one(a)
      integer a(:)
      print *, lbound(a), ubound(a), size(a)
      if ((lbound(a,dim=1) /= 1) .or. (ubound(a,dim=1) /= 3)) &
        call abort()
      print *, a
      if (any(a /= [1,2,3])) call abort()
  end subroutine one
end module m

program test
  use m
  implicit none
  call foo(one)
contains
  subroutine foo(f)
    ! The following interface block is needed
    ! for NAG f95 as it wrongly does not like
    ! use-associated interfaces for PROCEDURE
    ! (It is not needed for gfortran)
    interface
      subroutine one(a)
        integer a(:)
      end subroutine
    end interface
    procedure(one) :: f
    call f([1,2,3])
  end subroutine foo
end program test
Comment 2 Janus Weil 2008-04-05 18:03:11 UTC
(In reply to comment #1)
> @@ -3649,4 +3667,5 @@ void copy_formal_args (gfc_symbol *dest,
>        formal_arg->sym->attr = curr_arg->sym->attr;
>        formal_arg->sym->ts = curr_arg->sym->ts;
> +      formal_arg->sym->as = curr_arg->sym->as;

I guess one should rather use:

formal_arg->sym->as = gfc_copy_array_spec (curr_arg->sym->as);

With this addition your test case works at least with an explicit-size array:

integer a(1:3)

With an assumed-size "integer a(:)" it still fails.
Will try to find out more tomorrow.

> module m
> contains
>   subroutine one(a)
>       integer a(:)
>       print *, lbound(a), ubound(a), size(a)
>       if ((lbound(a,dim=1) /= 1) .or. (ubound(a,dim=1) /= 3)) &
>         call abort()
>       print *, a
>       if (any(a /= [1,2,3])) call abort()
>   end subroutine one
> end module m
> 
> program test
>   use m
>   implicit none
>   call foo(one)
> contains
>   subroutine foo(f)
>     ! The following interface block is needed
>     ! for NAG f95 as it wrongly does not like
>     ! use-associated interfaces for PROCEDURE
>     ! (It is not needed for gfortran)
>     interface
>       subroutine one(a)
>         integer a(:)
>       end subroutine
>     end interface
>     procedure(one) :: f
>     call f([1,2,3])
>   end subroutine foo
> end program test
> 

Comment 3 Janus Weil 2008-04-07 22:01:54 UTC
Another thing I just noticed is that dummy procedures are currently not checked for being called with the right arguments (-> compare_actual_formal), e.g. in the above test case "call f([1,2,3])" could also be called with a different number of arguments like "call f(1,2)" without any error message.
In fact this check is currently omitted for any procedure which has the EXTERNAL attribute (which includes all procedures declared via the "PROCEDURE()::" statement). But I guess this check should better be omitted only for procedures which have an implicit interface, right?
This can be cured with the following patch:

Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(revision 133905)
+++ gcc/fortran/interface.c	(working copy)
@@ -2419,8 +2419,7 @@ gfc_procedure_use (gfc_symbol *sym, gfc_
 	}
     }
 
-  if (sym->attr.external
-      || sym->attr.if_source == IFSRC_UNKNOWN)
+  if (sym->attr.if_source == IFSRC_UNKNOWN)
     {
       gfc_actual_arglist *a;
       for (a = *ap; a; a = a->next)
Comment 4 Tobias Burnus 2008-04-08 08:36:18 UTC
(In reply to comment #3)
> Another thing I just noticed is that dummy procedures are currently not checked
> for being called with the right arguments (-> compare_actual_formal),

If we are lucky this fixes PR 35831.
Comment 5 Janus Weil 2008-04-09 18:48:28 UTC
(In reply to comment #4)
> If we are lucky this fixes PR 35831.

Actually it does not, but I think I know how to fix it.

Comment 6 Janus Weil 2008-05-15 21:48:16 UTC
I noticed that while the test case from comment #1 still fails, the following variation actually works with the patch from comment #2:

module m
contains
  subroutine one(a)
      integer a(:)
      print *, lbound(a), ubound(a), size(a)
      if ((lbound(a,dim=1) /= 1) .or. (ubound(a,dim=1) /= 3)) &
        call abort()
      print *, a
      if (any(a /= [1,2,3])) call abort()
  end subroutine one
end module m

program test
  use m
  implicit none
  call foo(one)
contains
  subroutine foo(f)
    interface
      subroutine f(a)
        integer a(:)
      end subroutine
    end interface
    call f([1,2,3])
  end subroutine foo
end program test

The only difference being that f is specified by an INTERFACE statement instead of a PROCEDURE statement.
Comment 7 janus 2008-05-28 21:28:42 UTC
Subject: Bug 35830

Author: janus
Date: Wed May 28 21:27:56 2008
New Revision: 136130

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=136130
Log:
2008-05-28  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/36325
	PR fortran/35830
	* interface.c (gfc_procedure_use): Enable argument checking for
	external procedures with explicit interface.
	* symbol.c (check_conflict): Fix conflict checking for externals.
	(copy_formal_args): Fix handling of arrays.
	* resolve.c (resolve_specific_f0, resolve_specific_s0): Fix handling
	of intrinsics.
	* parse.c (parse_interface): Non-abstract INTERFACE statement implies
	EXTERNAL attribute.


2008-05-28  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/36325
	PR fortran/35830
	* gfortran.dg/interface_23.f90: New.
	* gfortran.dg/gomp/reduction3.f90: Fixed invalid code.
	* gfortran.dg/proc_decl_12.f90: New:
	* gfortran.dg/external_procedures_1.f90: Fixed error message.

Added:
    trunk/gcc/testsuite/gfortran.dg/interface_23.f90
    trunk/gcc/testsuite/gfortran.dg/proc_decl_12.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/interface.c
    trunk/gcc/fortran/parse.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/symbol.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/external_procedures_1.f90
    trunk/gcc/testsuite/gfortran.dg/gomp/reduction3.f90

Comment 8 janus 2008-05-28 21:34:24 UTC
rev. 136130 contains the fixes from comment #2 and comment #3, but the test case from comment #1 is still failing.
Comment 9 janus 2008-05-31 09:17:07 UTC
Ok, apparently rev. 136130 *did* in fact also fix the test case in comment #1 (although I somehow assumed otherwise), and therefore this whole PR is fixed.

Should I add another test case with an assumed shape array (or simply change proc_decl_12.f90 to have an assumed shape instead of an explicit shape array)?
Or is proc_decl_12.f90 enough as it is?
Comment 10 Tobias Burnus 2008-06-02 08:44:38 UTC
> Should I add another test case with an assumed shape array (or simply change
> proc_decl_12.f90 to have an assumed shape instead of an explicit shape array)?
> Or is proc_decl_12.f90 enough as it is?

In principle, no test case should be removed - only new ones added. (I try to find time this evening to look at the PRs and at your patch, http://gcc.gnu.org/ml/fortran/2008-06/msg00001.html )
Comment 11 Tobias Burnus 2008-06-07 17:04:12 UTC
> rev. 136130 contains the fixes from comment #2 and comment #3, but the test
> case from comment #1 is still failing.
Diff between dumped tree from comment #6 (working) and comment #1 (failing):

-  f (&parm.38);
+  D.1203 = _gfortran_internal_pack (&parm.38);
+  f (D.1203);
[...]

That means that the argument of f is regarded as assumed-size array ("a(*)") instead of as assumed-size array ("a(:)"). This happens for instance when the interface of "f" is not known (or when as->type is wrong).
Comment 12 Tobias Burnus 2008-06-07 17:30:35 UTC
It turned out to be a attr-related thing. f needs to be 0 in the following and it was 1:
trans-expr.c:2525  gfc_conv_function_call
              int f;
              f = (fsym != NULL)
                  && !(fsym->attr.pointer || fsym->attr.allocatable)
                  && fsym->as->type != AS_ASSUMED_SHAPE;
              f = f || !sym->attr.always_explicit;

The problem is that sym->attr.always_explicit is 0 instead of 1.

Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c       (Revision 136517)
+++ gcc/fortran/resolve.c       (Arbeitskopie)
@@ -7905,6 +7905,7 @@ resolve_symbol (gfc_symbol *sym)
          sym->ts.interface = ifc;
          sym->attr.function = ifc->attr.function;
          sym->attr.subroutine = ifc->attr.subroutine;
+         sym->attr.always_explicit = ifc->attr.always_explicit;
          copy_formal_args (sym, ifc);
        }
       else if (sym->ts.interface->name[0] != '\0')
Comment 13 Tobias Burnus 2008-06-07 17:43:42 UTC
MINE :-)
Comment 14 Tobias Burnus 2008-06-08 07:49:40 UTC
Subject: Bug 35830

Author: burnus
Date: Sun Jun  8 07:48:53 2008
New Revision: 136554

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=136554
Log:
2008-06-08  Tobias Burnus  <burnus@net-b.de>

       PR fortran/35830
       * resolve.c (resolve_symbol): Copy more attributes for
       PROCEDUREs with interfaces.

2008-06-08  Tobias Burnus  <burnus@net-b.de>

       PR fortran/35830
       * proc_decl_13.f90: New.
       * proc_decl_14.f90: New.
       * proc_decl_15.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/proc_decl_13.f90
    trunk/gcc/testsuite/gfortran.dg/proc_decl_14.f90
    trunk/gcc/testsuite/gfortran.dg/proc_decl_15.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog

Comment 15 Tobias Burnus 2008-06-08 07:50:01 UTC
FIXED on the trunk (4.4.0).