[fortran] PR91717 - ICE on concatenating deferred-length character and character literal
diff mbox series

Message ID CAGkQGi+Lh_2w+BP6c-2h_ru-5R91DF3=CK8+VvQE0ELUwTOMpQ@mail.gmail.com
State New
Headers show
Series
  • [fortran] PR91717 - ICE on concatenating deferred-length character and character literal
Related show

Commit Message

Paul Richard Thomas Sept. 11, 2019, 5:44 a.m. UTC
Hi All,

I nearly committed this patch as 'obvious' but noticed a fair number
of changes in 10-branch in dependency analysis. This is in fact a
10-regression. I'll wait for an OK.

Bootstrapped and regtested on FC30/x86_64 - OK to commit?

Paul

2019-09-11  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/91717
    * dependency.c (gfc_dep_resolver): Flag identical components
    and exit with return value 1 if set and no array refs.

2019-09-11  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/91717
    * gfortran.dg/dependency_55.f90 : New test.

Comments

Steve Kargl Sept. 11, 2019, 5:50 a.m. UTC | #1
On Wed, Sep 11, 2019 at 06:44:50AM +0100, Paul Richard Thomas wrote:
> ===================================================================
> *** gcc/testsuite/gfortran.dg/dependency_55.f90	(nonexistent)
> --- gcc/testsuite/gfortran.dg/dependency_55.f90	(working copy)
> ***************
> *** 0 ****
> --- 1,17 ----
> + ! { dg-do run }
> + !
> + ! Test the fix for PR91717 in which the concatenation operation ICEd.
> + !
> + ! Contributed by Damian Rouson  <damian@sourceryinstitute.org>
> + !
> +   type core
> +     character (len=:), allocatable :: msg
> +   end type
> + 
> +   type(core) :: my_core
> + 
> +   my_core%msg = my_core%msg//"my message is: "

my_core%msg is undefined on the RHS.  This is invalid 
Fortran.  Not sure whether your patch is correct or not.
Paul Richard Thomas Sept. 11, 2019, 10:27 p.m. UTC | #2
Hi Steve,

Being an allocatable component, this code appears on entry into scope:

  my_core.msg = 0B;
  my_core._msg_length = 0;
  {

Thus, according to the standard, my_core%msg is defined.

If you follow the backtrace from the ICE, you will see that something
like the patch is required. It is important that the dependency is
detected for this assignment even though no array references are
involved. There is certainly the need for a temporary, which the patch
generates.

Cheers

Paul

On Wed, 11 Sep 2019 at 06:50, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
>
> On Wed, Sep 11, 2019 at 06:44:50AM +0100, Paul Richard Thomas wrote:
> > ===================================================================
> > *** gcc/testsuite/gfortran.dg/dependency_55.f90       (nonexistent)
> > --- gcc/testsuite/gfortran.dg/dependency_55.f90       (working copy)
> > ***************
> > *** 0 ****
> > --- 1,17 ----
> > + ! { dg-do run }
> > + !
> > + ! Test the fix for PR91717 in which the concatenation operation ICEd.
> > + !
> > + ! Contributed by Damian Rouson  <damian@sourceryinstitute.org>
> > + !
> > +   type core
> > +     character (len=:), allocatable :: msg
> > +   end type
> > +
> > +   type(core) :: my_core
> > +
> > +   my_core%msg = my_core%msg//"my message is: "
>
> my_core%msg is undefined on the RHS.  This is invalid
> Fortran.  Not sure whether your patch is correct or not.
>
> --
> Steve
Steve Kargl Sept. 11, 2019, 11:05 p.m. UTC | #3
On Wed, Sep 11, 2019 at 11:27:56PM +0100, Paul Richard Thomas wrote:
> Hi Steve,
> 
> Being an allocatable component, this code appears on entry into scope:
> 
>   my_core.msg = 0B;
>   my_core._msg_length = 0;
>   {
> 
> Thus, according to the standard, my_core%msg is defined.
>

I'll politely defer to the Fortran standard.

  5.4.10 Allocatable variables

  The allocation status of an allocatable variable is either allocated
  or unallocated.  An allocatable variable becomes allocated as described
  in 9.7.1.3. It becomes unallocated as described in 9.7.3.2.

  An unallocated allocatable variable shall not be referenced or defined.

  7.5.4.6 Default initialization for components

  Allocatable components are always initialized to unallocated.

In your testcase, you have an unallocatable allocatable entity 
referenced on the RHS in the first line that involves my_core%msg.
From the Fortran standard's point of view, I believe the code is
nonconforming.

  type core
    character (len=:), allocatable :: msg
  end type

  type(core) :: my_core

  print *, allocated(my_core%msg)

! Comment out to avoid ICE.
!  my_core%msg on RHS is unallocated in next line.
!  my_core%msg = my_core%msg//"my message is: 
!  my_core%msg = my_core%msg//"Hello!"
!
!  if (my_core%msg .ne. "my message is: Hello!") stop 1
end

% gfcx -o z a.f90 && ./z
 F

> detected for this assignment even though no array references are
> involved. There is certainly the need for a temporary, which the patch
> generates.
> 

Your patch may fix the ICE, but if the code compiles and 
execute.  You are getting the "right" answer fortuitiouly.

Of course, I could be wrong.
Paul Richard Thomas Sept. 12, 2019, 8:55 a.m. UTC | #4
Hi Steve,

I have inserted a my_core%msg = '"" so that it is initialised. The
reallocation on assignment explicitly deals with an unallocated entity
or one of its allocatable components by allocation, rather than
reallocation.

I realise that my explanation of the patch might be a bit sparse. The
ultimate caller is frontend_passes.c(realloc_string_callback), which
looks for overlap even for scalar cases. The ICE came about because
there are no array references in the expressions being compared.
Flagging that there are identical component chains in this case and
returning 1 from gfc_dep_resolver covers this case.

OK to commit?

Cheers

Paul





realloc_string_callback)

On Thu, 12 Sep 2019 at 00:05, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
>
> On Wed, Sep 11, 2019 at 11:27:56PM +0100, Paul Richard Thomas wrote:
> > Hi Steve,
> >
> > Being an allocatable component, this code appears on entry into scope:
> >
> >   my_core.msg = 0B;
> >   my_core._msg_length = 0;
> >   {
> >
> > Thus, according to the standard, my_core%msg is defined.
> >
>
> I'll politely defer to the Fortran standard.
>
>   5.4.10 Allocatable variables
>
>   The allocation status of an allocatable variable is either allocated
>   or unallocated.  An allocatable variable becomes allocated as described
>   in 9.7.1.3. It becomes unallocated as described in 9.7.3.2.
>
>   An unallocated allocatable variable shall not be referenced or defined.
>
>   7.5.4.6 Default initialization for components
>
>   Allocatable components are always initialized to unallocated.
>
> In your testcase, you have an unallocatable allocatable entity
> referenced on the RHS in the first line that involves my_core%msg.
> From the Fortran standard's point of view, I believe the code is
> nonconforming.
>
>   type core
>     character (len=:), allocatable :: msg
>   end type
>
>   type(core) :: my_core
>
>   print *, allocated(my_core%msg)
>
> ! Comment out to avoid ICE.
> !  my_core%msg on RHS is unallocated in next line.
> !  my_core%msg = my_core%msg//"my message is:
> !  my_core%msg = my_core%msg//"Hello!"
> !
> !  if (my_core%msg .ne. "my message is: Hello!") stop 1
> end
>
> % gfcx -o z a.f90 && ./z
>  F
>
> > detected for this assignment even though no array references are
> > involved. There is certainly the need for a temporary, which the patch
> > generates.
> >
>
> Your patch may fix the ICE, but if the code compiles and
> execute.  You are getting the "right" answer fortuitiouly.
>
> Of course, I could be wrong.
>
> --
> Steve
Steve Kargl Sept. 12, 2019, 2:09 p.m. UTC | #5
On Thu, Sep 12, 2019 at 09:55:20AM +0100, Paul Richard Thomas wrote:
> 
> OK to commit?
> 

Yes.
Paul Richard Thomas Sept. 13, 2019, 5:44 a.m. UTC | #6
Thanks - committed as r275696.

Paul

On Thu, 12 Sep 2019 at 15:09, Steve Kargl
<sgk@troutmask.apl.washington.edu> wrote:
>
> On Thu, Sep 12, 2019 at 09:55:20AM +0100, Paul Richard Thomas wrote:
> >
> > OK to commit?
> >
>
> Yes.
>
> --
> Steve

Patch
diff mbox series

Index: gcc/fortran/dependency.c
===================================================================
*** gcc/fortran/dependency.c	(revision 275323)
--- gcc/fortran/dependency.c	(working copy)
*************** gfc_dep_resolver (gfc_ref *lref, gfc_ref
*** 2096,2101 ****
--- 2096,2102 ----
    int m;
    gfc_dependency fin_dep;
    gfc_dependency this_dep;
+   bool same_component = false;
  
    this_dep = GFC_DEP_ERROR;
    fin_dep = GFC_DEP_ERROR;
*************** gfc_dep_resolver (gfc_ref *lref, gfc_ref
*** 2115,2120 ****
--- 2116,2123 ----
  	     components.  */
  	  if (lref->u.c.component != rref->u.c.component)
  	    return 0;
+ 
+ 	  same_component = true;
  	  break;
  
  	case REF_SUBSTRING:
*************** gfc_dep_resolver (gfc_ref *lref, gfc_ref
*** 2280,2285 ****
--- 2283,2292 ----
    if (lref || rref)
      return 1;
  
+   /* This can result from concatenation of assumed length string components.  */
+   if (same_component && fin_dep == GFC_DEP_ERROR)
+     return 1;
+ 
    /* If we haven't seen any array refs then something went wrong.  */
    gcc_assert (fin_dep != GFC_DEP_ERROR);
  
Index: gcc/testsuite/gfortran.dg/dependency_55.f90
===================================================================
*** gcc/testsuite/gfortran.dg/dependency_55.f90	(nonexistent)
--- gcc/testsuite/gfortran.dg/dependency_55.f90	(working copy)
***************
*** 0 ****
--- 1,17 ----
+ ! { dg-do run }
+ !
+ ! Test the fix for PR91717 in which the concatenation operation ICEd.
+ !
+ ! Contributed by Damian Rouson  <damian@sourceryinstitute.org>
+ !
+   type core
+     character (len=:), allocatable :: msg
+   end type
+ 
+   type(core) :: my_core
+ 
+   my_core%msg = my_core%msg//"my message is: "
+   my_core%msg = my_core%msg//"Hello!"
+ 
+   if (my_core%msg .ne. "my message is: Hello!") stop 1
+ end