[patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

Hello world,

the attached patch fixes a regression where a segfault occured at
runtime for the attached test case.  The regression made it out
into the wild with the 8.3 release. Unfortunately, it was discovered too
late before release to do anything about it.

In effect, this patch reverts a part of r265171 for one special case: If
the argument to gfc_find_and_cut_at_last_class_ref is the
MOLD argument to allocate.

Of course, there could also be a more profound way of fixing this
bug :-)

Regression-tested on x86_64-pc-linux-gnu.  OK for trunk and gcc-8?

Regards

	Thomas

2019-02-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/89174
         * trans-expr.c (gfc_find_and_cut_at_last_class_ref): Add is_mold
         to garguments. If we are dealing with a MOLD, call
         gfc_expr_to_initialize().
         * trans-stmt.c (gfc_trans_allocate): For MOLD, pass is_mold=true
         to gfc_find_and_cut_at_last_class_ref.
         * trans.h (gfc_find_and_cut_at_last_class_ref): Add optional
         argument is_mold with default false.

2019-02-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

         PR fortran/89174
         * gfortran.dg/allocate_with_mold_3.f90: New test.

[-- Attachment #2: p1.diff --]
[-- Type: text/x-patch, Size: 1855 bytes --]

Index: trans-expr.c
===================================================================
--- trans-expr.c	(Revision 269161)
+++ trans-expr.c	(Arbeitskopie)
@@ -352,7 +352,7 @@ gfc_vptr_size_get (tree vptr)
    of refs following.  */
 
 gfc_expr *
-gfc_find_and_cut_at_last_class_ref (gfc_expr *e)
+gfc_find_and_cut_at_last_class_ref (gfc_expr *e, bool is_mold)
 {
   gfc_expr *base_expr;
   gfc_ref *ref, *class_ref, *tail = NULL, *array_ref;
@@ -394,7 +394,10 @@ gfc_expr *
       e->ref = NULL;
     }
 
-  base_expr = gfc_copy_expr (e);
+  if (is_mold)
+    base_expr = gfc_expr_to_initialize (e);
+  else
+    base_expr = gfc_copy_expr (e);
 
   /* Restore the original tail expression.  */
   if (class_ref)
Index: trans-stmt.c
===================================================================
--- trans-stmt.c	(Revision 269161)
+++ trans-stmt.c	(Arbeitskopie)
@@ -6641,7 +6641,7 @@ gfc_trans_allocate (gfc_code * code)
 	  /* Use class_init_assign to initialize expr.  */
 	  gfc_code *ini;
 	  ini = gfc_get_code (EXEC_INIT_ASSIGN);
-	  ini->expr1 = gfc_find_and_cut_at_last_class_ref (expr);
+	  ini->expr1 = gfc_find_and_cut_at_last_class_ref (expr, true);
 	  tmp = gfc_trans_class_init_assign (ini);
 	  gfc_free_statements (ini);
 	  gfc_add_expr_to_block (&block, tmp);
Index: trans.h
===================================================================
--- trans.h	(Revision 269161)
+++ trans.h	(Arbeitskopie)
@@ -412,7 +412,7 @@ tree gfc_class_data_get (tree);
 tree gfc_class_vptr_get (tree);
 tree gfc_class_len_get (tree);
 tree gfc_class_len_or_zero_get (tree);
-gfc_expr * gfc_find_and_cut_at_last_class_ref (gfc_expr *);
+gfc_expr * gfc_find_and_cut_at_last_class_ref (gfc_expr *, bool is_mold = false);
 /* Get an accessor to the class' vtab's * field, when a class handle is
    available.  */
 tree gfc_class_vtab_hash_get (tree);

[-- Attachment #3: allocate_with_mold_3.f90 --]
[-- Type: text/x-fortran, Size: 472 bytes --]

! { dg-do  run }
! PR fortran/89174 - this used to segfault on execution.
! Test case by Neil Carlson.
module mod
  type :: array_data
    class(*), allocatable :: mold
  contains
    procedure :: push
  end type
contains
  subroutine push(this, value)
    class(array_data), intent(inout) :: this
    class(*), intent(in) :: value
    allocate(this%mold, mold=value) ! <== SEGFAULTS HERE
  end subroutine
end module

use mod
type(array_data) :: foo
call foo%push(42)
end

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Paul Richard Thomas]

Hi Thomas,

> Of course, there could also be a more profound way of fixing this
> bug :-)

I wish that it were the case. I am unable to decide whether the design
choices made for an F95 compiler have cause the add-ons to be
intrinsically complicated or whether or whether F2018, extensions and
legacy support is the cause of the complexity - probably both.  Either
way, profundity is rarely to be found in bugfixes.

>
> Regression-tested on x86_64-pc-linux-gnu.  OK for trunk and gcc-8?

Yes, that's good for both branches. Thanks for the fix.

Paul

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Steve Kargl]

On Sun, Feb 24, 2019 at 05:42:14PM +0000, Paul Richard Thomas wrote:
> 
> > Of course, there could also be a more profound way of fixing this
> > bug :-)
> 
> I wish that it were the case. I am unable to decide whether the design
> choices made for an F95 compiler have cause the add-ons to be
> intrinsically complicated or whether or whether F2018, extensions and
> legacy support is the cause of the complexity - probably both.  Either
> way, profundity is rarely to be found in bugfixes.
> 

flang project was going to port PGI's frontend to use llvm as 
it middle/backend.  A part of the port was to update PGI bits
to include modern Fortran.  The flang developers have decided
to write an entirely new, from scratch, frontend.  My conclusion
is our 15+ year-old decisions and the addition of newer features
(both modern Fortran and legacy) have complicated parts of the
compiler to an almost unmanageable state.  You can probably 
appreciate this better than most of us.

-- 
Steve

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Dominique d'Humières]

Hi Thomas,

I see a double space in

! { dg-do  run }

Is this intended? If yes, it should probably be documented, otherwise it should be fixed.

TIA

Dominique

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Thomas Koenig]

Hi Dominique,

> I see a double space in
> 
> ! { dg-do  run }
> 
> Is this intended?

Yes, it is.  This is for tests which should not be run with all
the options cycling, but only once.

I think this was introduced quite some time ago, not sure if it
was ever documented anywhere.  I guess we should do so.

Regards

	Thomas

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Steve Kargl]

On Mon, Feb 25, 2019 at 07:15:32PM +0100, Thomas Koenig wrote:
> Hi Dominique,
> 
> > I see a double space in
> > 
> > ! { dg-do  run }
> > 
> > Is this intended?
> 
> Yes, it is.  This is for tests which should not be run with all
> the options cycling, but only once.
> 
> I think this was introduced quite some time ago, not sure if it
> was ever documented anywhere.  I guess we should do so.
> 

Probably want to document this in the testcase.

! { dg-do  run }
!        ^^
! Above two spaces are intentional.  It prevents cycling through options.

-- 
Steve

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Thomas König]

Hi Steve,

>> I think this was introduced quite some time ago, not sure if it
>> was ever documented anywhere.  I guess we should do so.
>>
> Probably want to document this in the testcase.

I just checked and found 77 occurences in the test suite (most of
them mine, to be sure).  So, maybe an entry in the wiki would be
more appropriate.

I am just trying to think of who introduced this (or if this
is even gfortran specific), but coming up blank.  Does anybody
remember?

Regards

	Thomas

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Steve Kargl]

On Mon, Feb 25, 2019 at 10:03:29PM +0100, Thomas König wrote:
> Hi Steve,
> 
> >> I think this was introduced quite some time ago, not sure if it
> >> was ever documented anywhere.  I guess we should do so.
> >>
> > Probably want to document this in the testcase.
> 
> I just checked and found 77 occurences in the test suite (most of
> them mine, to be sure).  So, maybe an entry in the wiki would be
> more appropriate.
> 
> I am just trying to think of who introduced this (or if this
> is even gfortran specific), but coming up blank.  Does anybody
> remember?
> 

It looks like a fortuitous accident.  

% svn annotate gcc/testsuite/lib/gfortran-dg.exp
 84792       tobi       # look if this is dg-do-run test, in which case
 84792       tobi       # we cycle through the option list, otherwise we don't
 84792       tobi       if [expr [search_for $test "dg-do run"]] {
135381      janis           set option_list $torture_with_loops
 84792       tobi       } else {
 84792       tobi           set option_list [list { -O } ]
 84792       tobi       }

The string "dg-do  run" does not match, so the torture loops are run.

-- 
Steve

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Steve Kargl]

On Mon, Feb 25, 2019 at 01:58:27PM -0800, Steve Kargl wrote:
> On Mon, Feb 25, 2019 at 10:03:29PM +0100, Thomas König wrote:
> > Hi Steve,
> > 
> > >> I think this was introduced quite some time ago, not sure if it
> > >> was ever documented anywhere.  I guess we should do so.
> > >>
> > > Probably want to document this in the testcase.
> > 
> > I just checked and found 77 occurences in the test suite (most of
> > them mine, to be sure).  So, maybe an entry in the wiki would be
> > more appropriate.
> > 
> > I am just trying to think of who introduced this (or if this
> > is even gfortran specific), but coming up blank.  Does anybody
> > remember?
> > 
> 
> It looks like a fortuitous accident.  
> 
> % svn annotate gcc/testsuite/lib/gfortran-dg.exp
>  84792       tobi       # look if this is dg-do-run test, in which case
>  84792       tobi       # we cycle through the option list, otherwise we don't
>  84792       tobi       if [expr [search_for $test "dg-do run"]] {
> 135381      janis           set option_list $torture_with_loops
>  84792       tobi       } else {
>  84792       tobi           set option_list [list { -O } ]
>  84792       tobi       }
> 
> The string "dg-do  run" does not match, so the torture loops are run.
> 

Whoop. s/are run/are not run/

IIRC, r84792 is the earliest svn revision for gfortran.
Need to go back to the old cvs repository to find out 
more.
-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow