[Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Andre Vehreschild]

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

Hi all,

attached patch fixes a crash in coarray programs when an allocatable derived
typed coarray was freed explicitly. The generated cleanup code did not take
into account, that the coarray may have been deallocated already. The patch
fixes this by moving the statements accessing components inside the derived type
into the block guard by its allocated check.

Regtested ok on f37/x86_64. Ok for master?

Regards,
	Andre
--
Andre Vehreschild * Email: vehre ad gmx dot de

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: caf_finalizer_fix.patch --]
[-- Type: text/x-patch, Size: 2601 bytes --]

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index e0fc8ebc46b..8e94a9a469f 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -9320,6 +9320,12 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl, tree dest,
       gfc_add_expr_to_block (&fnblock, tmp);
     }

+  /* Still having a descriptor array of rank == 0 here, indicates an
+     allocatable coarrays.  Dereference it correctly.  */
+  if (GFC_DESCRIPTOR_TYPE_P (decl_type))
+    {
+      decl = build_fold_indirect_ref (gfc_conv_array_data (decl));
+    }
   /* Otherwise, act on the components or recursively call self to
      act on a chain of components.  */
   for (c = der_type->components; c; c = c->next)
@@ -11507,7 +11513,11 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block)
     {
       int rank;
       rank = sym->as ? sym->as->rank : 0;
-      tmp = gfc_deallocate_alloc_comp (sym->ts.u.derived, descriptor, rank);
+      tmp = gfc_deallocate_alloc_comp (sym->ts.u.derived, descriptor, rank,
+				       (sym->attr.codimension
+					&& flag_coarray == GFC_FCOARRAY_LIB)
+				       ? GFC_STRUCTURE_CAF_MODE_IN_COARRAY
+				       : 0);
       gfc_add_expr_to_block (&cleanup, tmp);
     }

@@ -11521,9 +11531,11 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block)
 					NULL_TREE, NULL_TREE, true, e,
 					sym->attr.codimension
 					? GFC_CAF_COARRAY_DEREGISTER
-					: GFC_CAF_COARRAY_NOCOARRAY);
+					: GFC_CAF_COARRAY_NOCOARRAY,
+					NULL_TREE, gfc_finish_block (&cleanup));
       if (e)
 	gfc_free_expr (e);
+      gfc_init_block (&cleanup);
       gfc_add_expr_to_block (&cleanup, tmp);
     }

diff --git a/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90 b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90
new file mode 100644
index 00000000000..e8a74db2c18
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90
@@ -0,0 +1,29 @@
+! { dg-do run }
+
+program alloc_comp_6
+
+  implicit none
+
+  type :: foo
+    real :: x
+    integer, allocatable :: y(:)
+  end type
+
+  call check()
+
+contains
+
+  subroutine check()
+    block
+      type(foo), allocatable :: example[:] ! needs to be a coarray
+
+      allocate(example[*])
+      allocate(example%y(10))
+      example%x = 3.4
+      example%y = 4
+
+      deallocate(example)
+    end block  ! example%y shall not be accessed here by the finalizer,
+               ! because example is already deallocated
+  end subroutine check
+end program alloc_comp_6

[-- Attachment #3: caf_finalizer_fix.clog --]
[-- Type: text/plain, Size: 670 bytes --]

    Fortran: Free alloc. comp. in allocated coarrays only.

    When freeing allocatable components of an allocatable coarray, add
    a check that the coarray is still allocated, before accessing the
    components.

    This patch adds to PR fortran/37336, but does not fix it completely.

    gcc/fortran/ChangeLog:
            PR fortran/37336
            * trans-array.cc (structure_alloc_comps): Deref coarray.
            (gfc_trans_deferred_array): Add freeing of components after
            check for allocated coarray.

    gcc/testsuite/ChangeLog:
            PR fortran/37336
            * gfortran.dg/coarray/alloc_comp_6.f90: New test.

Re: [Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Paul Richard Thomas]

Hi Andre,

The patch looks fine to me. Since you mention it in the comment, is it
worth declaring the derived type 'foo' in a module and giving it a
final routine?

Thanks for the patch.

Paul

On Thu, 28 Sept 2023 at 13:45, Andre Vehreschild via Fortran
<fortran@gcc.gnu.org> wrote:
>
> Hi all,
>
> attached patch fixes a crash in coarray programs when an allocatable derived
> typed coarray was freed explicitly. The generated cleanup code did not take
> into account, that the coarray may have been deallocated already. The patch
> fixes this by moving the statements accessing components inside the derived type
> into the block guard by its allocated check.
>
> Regtested ok on f37/x86_64. Ok for master?
>
> Regards,
>         Andre
> --
> Andre Vehreschild * Email: vehre ad gmx dot de

Re: [Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Andre Vehreschild]

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

Hi Paul,

thanks for the quick review. I've added a testcase with a module and a
finalizer in the derived type. This also is no problem.

Regtests ok on x86_64_linux_gnu/f37. Ok for trunk?

Regards,
	Andre

On Thu, 28 Sep 2023 19:21:12 +0100
Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:

> Hi Andre,
>
> The patch looks fine to me. Since you mention it in the comment, is it
> worth declaring the derived type 'foo' in a module and giving it a
> final routine?
>
> Thanks for the patch.
>
> Paul
>
> On Thu, 28 Sept 2023 at 13:45, Andre Vehreschild via Fortran
> <fortran@gcc.gnu.org> wrote:
> >
> > Hi all,
> >
> > attached patch fixes a crash in coarray programs when an allocatable derived
> > typed coarray was freed explicitly. The generated cleanup code did not take
> > into account, that the coarray may have been deallocated already. The patch
> > fixes this by moving the statements accessing components inside the derived
> > type into the block guard by its allocated check.
> >
> > Regtested ok on f37/x86_64. Ok for master?
> >
> > Regards,
> >         Andre
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de


--
Andre Vehreschild * Email: vehre ad gmx dot de

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: caf_finalizer_fix_v2.patch --]
[-- Type: text/x-patch, Size: 3844 bytes --]

diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc
index e0fc8ebc46b..8e94a9a469f 100644
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -9320,6 +9320,12 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl, tree dest,
       gfc_add_expr_to_block (&fnblock, tmp);
     }

+  /* Still having a descriptor array of rank == 0 here, indicates an
+     allocatable coarrays.  Dereference it correctly.  */
+  if (GFC_DESCRIPTOR_TYPE_P (decl_type))
+    {
+      decl = build_fold_indirect_ref (gfc_conv_array_data (decl));
+    }
   /* Otherwise, act on the components or recursively call self to
      act on a chain of components.  */
   for (c = der_type->components; c; c = c->next)
@@ -11507,7 +11513,11 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block)
     {
       int rank;
       rank = sym->as ? sym->as->rank : 0;
-      tmp = gfc_deallocate_alloc_comp (sym->ts.u.derived, descriptor, rank);
+      tmp = gfc_deallocate_alloc_comp (sym->ts.u.derived, descriptor, rank,
+				       (sym->attr.codimension
+					&& flag_coarray == GFC_FCOARRAY_LIB)
+				       ? GFC_STRUCTURE_CAF_MODE_IN_COARRAY
+				       : 0);
       gfc_add_expr_to_block (&cleanup, tmp);
     }

@@ -11521,9 +11531,11 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block)
 					NULL_TREE, NULL_TREE, true, e,
 					sym->attr.codimension
 					? GFC_CAF_COARRAY_DEREGISTER
-					: GFC_CAF_COARRAY_NOCOARRAY);
+					: GFC_CAF_COARRAY_NOCOARRAY,
+					NULL_TREE, gfc_finish_block (&cleanup));
       if (e)
 	gfc_free_expr (e);
+      gfc_init_block (&cleanup);
       gfc_add_expr_to_block (&cleanup, tmp);
     }

diff --git a/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90 b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90
new file mode 100644
index 00000000000..e8a74db2c18
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_6.f90
@@ -0,0 +1,29 @@
+! { dg-do run }
+
+program alloc_comp_6
+
+  implicit none
+
+  type :: foo
+    real :: x
+    integer, allocatable :: y(:)
+  end type
+
+  call check()
+
+contains
+
+  subroutine check()
+    block
+      type(foo), allocatable :: example[:] ! needs to be a coarray
+
+      allocate(example[*])
+      allocate(example%y(10))
+      example%x = 3.4
+      example%y = 4
+
+      deallocate(example)
+    end block  ! example%y shall not be accessed here by the finalizer,
+               ! because example is already deallocated
+  end subroutine check
+end program alloc_comp_6
diff --git a/gcc/testsuite/gfortran.dg/coarray/alloc_comp_7.f90 b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_7.f90
new file mode 100644
index 00000000000..5ebd31f3df7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/alloc_comp_7.f90
@@ -0,0 +1,49 @@
+! { dg-do run }
+
+module alloc_comp_module_7
+
+  public :: check
+
+  type :: foo
+    real :: x
+    integer, allocatable :: y(:)
+  contains
+    final :: foo_final
+  end type
+
+contains
+
+  subroutine foo_final(f)
+    type(foo), intent(inout) :: f
+
+    if (allocated(f%y)) then
+      f%y = -1
+    end if
+  end subroutine foo_final
+
+  subroutine check()
+    block
+      type(foo), allocatable :: example[:] ! needs to be a coarray
+
+      allocate(example[*])
+      allocate(example%y(10))
+      example%x = 3.4
+      example%y = 4
+
+      deallocate(example%y)
+      deallocate(example)
+    end block  ! example%y shall not be accessed here by the finalizer,
+               ! because example is already deallocated
+  end subroutine check
+end module alloc_comp_module_7
+
+program alloc_comp_7
+
+  use alloc_comp_module_7, only: check
+
+  implicit none
+
+  call check()
+
+end program alloc_comp_7
+

[-- Attachment #3: caf_finalizer_fix_v2.clog --]
[-- Type: text/plain, Size: 733 bytes --]

    Fortran: Free alloc. comp. in allocated coarrays only.

    When freeing allocatable components of an allocatable coarray, add
    a check that the coarray is still allocated, before accessing the
    components.

    This patch adds to PR fortran/37336, but does not fix it completely.

    gcc/fortran/ChangeLog:
            PR fortran/37336
            * trans-array.cc (structure_alloc_comps): Deref coarray.
            (gfc_trans_deferred_array): Add freeing of components after
            check for allocated coarray.

    gcc/testsuite/ChangeLog:
            PR fortran/37336
            * gfortran.dg/coarray/alloc_comp_6.f90: New test.
            * gfortran.dg/coarray/alloc_comp_7.f90: New test.

Re: [Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Paul Richard Thomas]

Hi Andre,

Yes indeed - it's fine for trunk and, I would suggest, 13-branch.

Cheers

Paul

On Fri, 29 Sept 2023 at 11:01, Andre Vehreschild <vehre@gmx.de> wrote:
>
> Hi Paul,
>
> thanks for the quick review. I've added a testcase with a module and a
> finalizer in the derived type. This also is no problem.
>
> Regtests ok on x86_64_linux_gnu/f37. Ok for trunk?
>
> Regards,
>         Andre
>
> On Thu, 28 Sep 2023 19:21:12 +0100
> Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
>
> > Hi Andre,
> >
> > The patch looks fine to me. Since you mention it in the comment, is it
> > worth declaring the derived type 'foo' in a module and giving it a
> > final routine?
> >
> > Thanks for the patch.
> >
> > Paul
> >
> > On Thu, 28 Sept 2023 at 13:45, Andre Vehreschild via Fortran
> > <fortran@gcc.gnu.org> wrote:
> > >
> > > Hi all,
> > >
> > > attached patch fixes a crash in coarray programs when an allocatable derived
> > > typed coarray was freed explicitly. The generated cleanup code did not take
> > > into account, that the coarray may have been deallocated already. The patch
> > > fixes this by moving the statements accessing components inside the derived
> > > type into the block guard by its allocated check.
> > >
> > > Regtested ok on f37/x86_64. Ok for master?
> > >
> > > Regards,
> > >         Andre
> > > --
> > > Andre Vehreschild * Email: vehre ad gmx dot de
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de

Re: [Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Andre Vehreschild]

Hi Paul,

thanks. Commit to trunk as a680274616ec6b26ccfdcee400ed7f54e341d40c
and backported to gcc-13 as d9b3269bdccac2db9200303494c4e82f2aeb7bbc

Thanks for the fast review.

Regards,
	Andre

On Fri, 29 Sep 2023 13:38:57 +0100
Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:

> Hi Andre,
>
> Yes indeed - it's fine for trunk and, I would suggest, 13-branch.
>
> Cheers
>
> Paul
>
> On Fri, 29 Sept 2023 at 11:01, Andre Vehreschild <vehre@gmx.de> wrote:
> >
> > Hi Paul,
> >
> > thanks for the quick review. I've added a testcase with a module and a
> > finalizer in the derived type. This also is no problem.
> >
> > Regtests ok on x86_64_linux_gnu/f37. Ok for trunk?
> >
> > Regards,
> >         Andre
> >
> > On Thu, 28 Sep 2023 19:21:12 +0100
> > Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> >
> > > Hi Andre,
> > >
> > > The patch looks fine to me. Since you mention it in the comment, is it
> > > worth declaring the derived type 'foo' in a module and giving it a
> > > final routine?
> > >
> > > Thanks for the patch.
> > >
> > > Paul
> > >
> > > On Thu, 28 Sept 2023 at 13:45, Andre Vehreschild via Fortran
> > > <fortran@gcc.gnu.org> wrote:
> > > >
> > > > Hi all,
> > > >
> > > > attached patch fixes a crash in coarray programs when an allocatable
> > > > derived typed coarray was freed explicitly. The generated cleanup code
> > > > did not take into account, that the coarray may have been deallocated
> > > > already. The patch fixes this by moving the statements accessing
> > > > components inside the derived type into the block guard by its
> > > > allocated check.
> > > >
> > > > Regtested ok on f37/x86_64. Ok for master?
> > > >
> > > > Regards,
> > > >         Andre
> > > > --
> > > > Andre Vehreschild * Email: vehre ad gmx dot de
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de


--
Andre Vehreschild * Email: vehre ad gmx dot de

Re: [Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Andre Vehreschild]

Hi all,

back porting to gcc-13 unfortunately caused a regression due to
gfc_deallocate_with_status() having a different parameter count. This is fixed
as obvious by 874b895fffd921659b37dc05bc94eea48e9a0157.

Sorry for breaking gfortran-13. I still don't know why it checkout fine on my
system in the beginning. I must have done something wrong.

Please accept my apologies and regards,
	Andre

On Fri, 29 Sep 2023 15:13:56 +0200
Andre Vehreschild via Fortran <fortran@gcc.gnu.org> wrote:

> Hi Paul,
>
> thanks. Commit to trunk as a680274616ec6b26ccfdcee400ed7f54e341d40c
> and backported to gcc-13 as d9b3269bdccac2db9200303494c4e82f2aeb7bbc
>
> Thanks for the fast review.
>
> Regards,
> 	Andre
>
> On Fri, 29 Sep 2023 13:38:57 +0100
> Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
>
> > Hi Andre,
> >
> > Yes indeed - it's fine for trunk and, I would suggest, 13-branch.
> >
> > Cheers
> >
> > Paul
> >
> > On Fri, 29 Sept 2023 at 11:01, Andre Vehreschild <vehre@gmx.de> wrote:
> > >
> > > Hi Paul,
> > >
> > > thanks for the quick review. I've added a testcase with a module and a
> > > finalizer in the derived type. This also is no problem.
> > >
> > > Regtests ok on x86_64_linux_gnu/f37. Ok for trunk?
> > >
> > > Regards,
> > >         Andre
> > >
> > > On Thu, 28 Sep 2023 19:21:12 +0100
> > > Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> > >
> > > > Hi Andre,
> > > >
> > > > The patch looks fine to me. Since you mention it in the comment, is it
> > > > worth declaring the derived type 'foo' in a module and giving it a
> > > > final routine?
> > > >
> > > > Thanks for the patch.
> > > >
> > > > Paul
> > > >
> > > > On Thu, 28 Sept 2023 at 13:45, Andre Vehreschild via Fortran
> > > > <fortran@gcc.gnu.org> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > attached patch fixes a crash in coarray programs when an allocatable
> > > > > derived typed coarray was freed explicitly. The generated cleanup code
> > > > > did not take into account, that the coarray may have been deallocated
> > > > > already. The patch fixes this by moving the statements accessing
> > > > > components inside the derived type into the block guard by its
> > > > > allocated check.
> > > > >
> > > > > Regtested ok on f37/x86_64. Ok for master?
> > > > >
> > > > > Regards,
> > > > >         Andre
> > > > > --
> > > > > Andre Vehreschild * Email: vehre ad gmx dot de
> > >
> > >
> > > --
> > > Andre Vehreschild * Email: vehre ad gmx dot de
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de


--
Andre Vehreschild * Email: vehre ad gmx dot de

Re: [Fortran, Patch, Coarray, PR 37336] Fix crash in finalizer when derived type coarray is already freed.

[Paul Richard Thomas]

Hi Andre,

All is well that ends well! Thanks for working on this.

Regards

Paul

On Sat, 30 Sept 2023 at 14:16, Andre Vehreschild <vehre@gmx.de> wrote:
>
> Hi all,
>
> back porting to gcc-13 unfortunately caused a regression due to
> gfc_deallocate_with_status() having a different parameter count. This is fixed
> as obvious by 874b895fffd921659b37dc05bc94eea48e9a0157.
>
> Sorry for breaking gfortran-13. I still don't know why it checkout fine on my
> system in the beginning. I must have done something wrong.
>
> Please accept my apologies and regards,
>         Andre
>
> On Fri, 29 Sep 2023 15:13:56 +0200
> Andre Vehreschild via Fortran <fortran@gcc.gnu.org> wrote:
>
> > Hi Paul,
> >
> > thanks. Commit to trunk as a680274616ec6b26ccfdcee400ed7f54e341d40c
> > and backported to gcc-13 as d9b3269bdccac2db9200303494c4e82f2aeb7bbc
> >
> > Thanks for the fast review.
> >
> > Regards,
> >       Andre
> >
> > On Fri, 29 Sep 2023 13:38:57 +0100
> > Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> >
> > > Hi Andre,
> > >
> > > Yes indeed - it's fine for trunk and, I would suggest, 13-branch.
> > >
> > > Cheers
> > >
> > > Paul
> > >
> > > On Fri, 29 Sept 2023 at 11:01, Andre Vehreschild <vehre@gmx.de> wrote:
> > > >
> > > > Hi Paul,
> > > >
> > > > thanks for the quick review. I've added a testcase with a module and a
> > > > finalizer in the derived type. This also is no problem.
> > > >
> > > > Regtests ok on x86_64_linux_gnu/f37. Ok for trunk?
> > > >
> > > > Regards,
> > > >         Andre
> > > >
> > > > On Thu, 28 Sep 2023 19:21:12 +0100
> > > > Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> > > >
> > > > > Hi Andre,
> > > > >
> > > > > The patch looks fine to me. Since you mention it in the comment, is it
> > > > > worth declaring the derived type 'foo' in a module and giving it a
> > > > > final routine?
> > > > >
> > > > > Thanks for the patch.
> > > > >
> > > > > Paul
> > > > >
> > > > > On Thu, 28 Sept 2023 at 13:45, Andre Vehreschild via Fortran
> > > > > <fortran@gcc.gnu.org> wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > attached patch fixes a crash in coarray programs when an allocatable
> > > > > > derived typed coarray was freed explicitly. The generated cleanup code
> > > > > > did not take into account, that the coarray may have been deallocated
> > > > > > already. The patch fixes this by moving the statements accessing
> > > > > > components inside the derived type into the block guard by its
> > > > > > allocated check.
> > > > > >
> > > > > > Regtested ok on f37/x86_64. Ok for master?
> > > > > >
> > > > > > Regards,
> > > > > >         Andre
> > > > > > --
> > > > > > Andre Vehreschild * Email: vehre ad gmx dot de
> > > >
> > > >
> > > > --
> > > > Andre Vehreschild * Email: vehre ad gmx dot de
> >
> >
> > --
> > Andre Vehreschild * Email: vehre ad gmx dot de
>
>
> --
> Andre Vehreschild * Email: vehre ad gmx dot de