[Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Andre Vehreschild]

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

Hi all,

attached patch fixes a segfault when allocating a coarray of a type
that has allocatable components. Before the patch the compiler tried
to ref the component to nullify from the coarray's base address and not
from its .data component. The proposed patch fixes this by preventing
the nullify of the components in the array_allocate() for coarrays,
because the components are nullified again afterwards by copying a
fully nullified copy of the type to the coarray's data component.

There albeit is an alternative to this patch:

trans-array.c: 5556+

-       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, se->expr,
+       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, coarray ?
+  						pointer : se->expr, 
				      ref->u.ar.as->rank);

The above adds a second nullify to the generated code before the one
done the object copy mentioned above. 

Because I am unsure which patch is best, I propose both. I do favor of
course the one without the duplicate nullify as attached.

Bootstrapped and regtested ok on x86_64-linux-gnu/F23. Ok for trunk?

The patch also applies (with a small delta) to gcc-5 w/o any
regressions. Ok for gcc-5-branch?

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

[-- Attachment #2: pr65795_1.clog --]
[-- Type: application/octet-stream, Size: 463 bytes --]

gcc/fortran/ChangeLog:

2016-04-03  Andre Vehreschild  <vehre@gcc.gnu.org>

	PR fortran/65795
	* trans-array.c (gfc_array_allocate): When the array is a coarray,
	do not nullyfing its allocatable components in array_allocate, because
	the nullify missed the array ref and nullifies the wrong component.
	Cosmetics.

gcc/testsuite/ChangeLog:

2016-04-03  Andre Vehreschild  <vehre@gcc.gnu.org>

	PR fortran/65795
	* gfortran.dg/coarray_allocate_6.f08: New test.



[-- Attachment #3: pr65795_1.patch --]
[-- Type: text/x-patch, Size: 1400 bytes --]

diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
index 649b80f..825dfb8 100644
--- a/gcc/fortran/trans-array.c
+++ b/gcc/fortran/trans-array.c
@@ -5550,8 +5550,8 @@ gfc_array_allocate (gfc_se * se, gfc_expr * expr, tree status, tree errmsg,
   else
       gfc_add_expr_to_block (&se->pre, set_descriptor);
 
-  if ((expr->ts.type == BT_DERIVED)
-	&& expr->ts.u.derived->attr.alloc_comp)
+  if (expr->ts.type == BT_DERIVED && expr->ts.u.derived->attr.alloc_comp
+      && !coarray)
     {
       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, se->expr,
 				    ref->u.ar.as->rank);
diff --git a/gcc/testsuite/gfortran.dg/coarray_allocate_6.f08 b/gcc/testsuite/gfortran.dg/coarray_allocate_6.f08
new file mode 100644
index 0000000..8394c30
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_allocate_6.f08
@@ -0,0 +1,27 @@
+! { dg-do run }
+! { dg-options "-fcoarray=single" }
+
+! Contributed by Tobias Burnus  <burnus@gcc.gnu.org>
+! Test fix for pr65795.
+
+implicit none
+
+type t2
+  integer, allocatable :: x
+end type t2
+
+type t3
+  type(t2), allocatable :: caf[:]
+end type t3
+
+!type(t3), save, target :: c, d
+type(t3), target :: c, d
+integer :: stat
+
+allocate(c%caf[*], stat=stat)
+end
+
+! Besides checking that the executable does not crash anymore, check
+! that the cause has been remove.
+! { dg-final { scan-tree-dump-not "c.caf.x = 0B" "original" } }
+

Re: [Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Jerry DeLisle]

On 04/03/2016 06:35 AM, Andre Vehreschild wrote:
> Hi all,
> 
> attached patch fixes a segfault when allocating a coarray of a type
> that has allocatable components. Before the patch the compiler tried
> to ref the component to nullify from the coarray's base address and not
> from its .data component. The proposed patch fixes this by preventing
> the nullify of the components in the array_allocate() for coarrays,
> because the components are nullified again afterwards by copying a
> fully nullified copy of the type to the coarray's data component.
> 
> There albeit is an alternative to this patch:
> 
> trans-array.c: 5556+
> 
> -       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, se->expr,
> +       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, coarray ?
> +  						pointer : se->expr, 
> 				      ref->u.ar.as->rank);
> 
> The above adds a second nullify to the generated code before the one
> done the object copy mentioned above. 
> 
> Because I am unsure which patch is best, I propose both. I do favor of
> course the one without the duplicate nullify as attached.
> 
> Bootstrapped and regtested ok on x86_64-linux-gnu/F23. Ok for trunk?
> 
> The patch also applies (with a small delta) to gcc-5 w/o any
> regressions. Ok for gcc-5-branch?
> 
> Regards,
> 	Andre
> 

OK for trunk and gcc-5. Go with your preferred.

Jerry

Re: [Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Damian Rouson]

Hi Andre,

Thanks for submitting this patch .  What can a program do with the object after it has been allocated?  Below is a simplified version of the code submitted in pr65795 and the compile-time error that results from attempting accessing the co-indexed component of the allocated object.  Does the patch address this error?

Damian

$ cat bug65795.f90 
implicit none

type t2
  integer, allocatable :: x
end type t2

type t3
  type(t2), allocatable :: caf[:]
end type t3

type(t3) :: c

allocate(c%caf[*])
c%caf%x = this_image()
c%caf = c%caf[1]
end

$ caf bug65795.f90 
bug65795.f90:15:8:

 c%caf = c%caf[1]
        1
Error: Sorry, coindexed coarray at (1) with allocatable component is not yet supported

$ gfortran --version
GNU Fortran (MacPorts gcc6 6-20160306_0) 6.0.0 20160306 (experimental)




> On Apr 3, 2016, at 6:35 AM, Andre Vehreschild <vehre@gmx.de> wrote:
> 
> Hi all,
> 
> attached patch fixes a segfault when allocating a coarray of a type
> that has allocatable components. Before the patch the compiler tried
> to ref the component to nullify from the coarray's base address and not
> from its .data component. The proposed patch fixes this by preventing
> the nullify of the components in the array_allocate() for coarrays,
> because the components are nullified again afterwards by copying a
> fully nullified copy of the type to the coarray's data component.
> 
> There albeit is an alternative to this patch:
> 
> trans-array.c: 5556+
> 
> -       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, se->expr,
> +       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, coarray ?
> +  						pointer : se->expr, 
> 				      ref->u.ar.as->rank);
> 
> The above adds a second nullify to the generated code before the one
> done the object copy mentioned above. 
> 
> Because I am unsure which patch is best, I propose both. I do favor of
> course the one without the duplicate nullify as attached.
> 
> Bootstrapped and regtested ok on x86_64-linux-gnu/F23. Ok for trunk?
> 
> The patch also applies (with a small delta) to gcc-5 w/o any
> regressions. Ok for gcc-5-branch?
> 
> Regards,
> 	Andre
> -- 
> Andre Vehreschild * Email: vehre ad gmx dot de 
> <pr65795_1.clog><pr65795_1.patch>

________________________________
Damian Rouson, Ph.D., P.E.
President, Sourcery Institute
http://www.sourceryinstitute.org
+1-510-600-2992 (mobile)

Re: [Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Damian Rouson]

> On Apr 3, 2016, at 3:04 PM, Andre Vehreschild <vehre@gmx.de> wrote:
> 
> Hi Damian,
> 
> To say it quite bluntly, I don't know. I took care of the ICE only, but I don't have a deeper understanding of the coarray usage, therefore I can't answer your question.

Hi Andre,

No problem.  Thanks for the quick reply.

> What should the meaning of the line in question be? Doesn't it overwrite the allocated reference with the one of image 1? And how would you expect to continue from there?

It’s just a check to see what the compiler will do.  It could be thought of as a poorly written broadcast.  To be a correct broadcast, it would require a “sync all” just after the first assignment. Then the second assignment would give every image a copy of the caf component that was on image 1, which has an x component with the value 1.   Even with this correction, it would of course exhibit poor scaling due to network contention and it would be better to call co_broadcast.  

I just wrote it to see if there had been additional progress toward supporting derived type coarrays with allocatable or pointer components.  If so, that will be of great interest to the users of OpenCoarrays and I would announce it on the OpenCoarrays mailing list.

Damian

Re: [Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Dominique d'Humières]

Andre,

Does not the test gfortran.dg/coarray_allocate_6.f08 require a -fdump-tree-original in the dg-options list?

Thanks for the patch,

Dominique

Re: [Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Dominique d'Humières]

> Le 3 avr. 2016 à 23:38, Andre Vehreschild <vehre@gmx.de> a écrit :
> 
> Hi Dominique,
> 
> I thought that was implicit, isn't it?

I don’t think so and I see in the log files

gcc/testsuite/gfortran4/gfortran.sum:UNRESOLVED: gfortran.dg/coarray_allocate_6.f08   -O0   scan-tree-dump-not original "c.caf.x = 0B"

> Is only the removal of the file implicit?

Yes,

> I will add the option to be on the safe side.

Thanks,

Dominique

> 
> - Andre
> 
> Am 3. April 2016 22:57:48 MESZ, schrieb "Dominique d'Humières" <dominiq@lps.ens.fr>:
> Andre,
> 
> Does not the test gfortran.dg/coarray_allocate_6.f08 require a -fdump-tree-original in the dg-options list?
> 
> Thanks for the patch,
> 
> Dominique
> 
> 
> -- 
> Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
> Tel.: +49 241 929 10 18 * vehre@gmx.de

Re: [Fortran, Patch, pr65795, v1] Segfault (invalid write) for ALLOCATE statement involving COARRAYS

[Andre Vehreschild]

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

Hi Jerry, hi Dominique,

Jerry, thank you for the review. Committed to

trunk as r234710 and to
gcc-5-branch as r234711

including the additional option Dominique noted I had missed. Thank for
pointing that out, Dominique.

Regards,
	Andre

On Sun, 3 Apr 2016 23:46:34 +0200
Dominique d'Humières <dominiq@lps.ens.fr> wrote:

> > Le 3 avr. 2016 à 23:38, Andre Vehreschild <vehre@gmx.de> a écrit :
> > 
> > Hi Dominique,
> > 
> > I thought that was implicit, isn't it?  
> 
> I don’t think so and I see in the log files
> 
> gcc/testsuite/gfortran4/gfortran.sum:UNRESOLVED: gfortran.dg/coarray_allocate_6.f08   -O0   scan-tree-dump-not original "c.caf.x = 0B"
> 
> > Is only the removal of the file implicit?  
> 
> Yes,
> 
> > I will add the option to be on the safe side.  
> 
> Thanks,
> 
> Dominique
> 
> > 
> > - Andre
> > 
> > Am 3. April 2016 22:57:48 MESZ, schrieb "Dominique d'Humières" <dominiq@lps.ens.fr>:
> > Andre,
> > 
> > Does not the test gfortran.dg/coarray_allocate_6.f08 require a -fdump-tree-original in the dg-options list?
> > 
> > Thanks for the patch,
> > 
> > Dominique
> > 
> > 
> > -- 
> > Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
> > Tel.: +49 241 929 10 18 * vehre@gmx.de  
> 


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

[-- Attachment #2: submit.trunk.diff --]
[-- Type: text/x-patch, Size: 2394 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(Revision 234709)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,11 @@
+2016-04-04  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/65795
+	* trans-array.c (gfc_array_allocate): When the array is a coarray,
+	do not nullyfing its allocatable components in array_allocate, because
+	the nullify missed the array ref and nullifies the wrong component.
+	Cosmetics.
+
 2016-03-29  Andre Vehreschild  <vehre@gcc.gnu.org>
 
 	PR fortran/70397
Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c	(Revision 234709)
+++ gcc/fortran/trans-array.c	(Arbeitskopie)
@@ -5550,8 +5550,8 @@
   else
       gfc_add_expr_to_block (&se->pre, set_descriptor);
 
-  if ((expr->ts.type == BT_DERIVED)
-	&& expr->ts.u.derived->attr.alloc_comp)
+  if (expr->ts.type == BT_DERIVED && expr->ts.u.derived->attr.alloc_comp
+      && !coarray)
     {
       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, se->expr,
 				    ref->u.ar.as->rank);
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(Revision 234709)
+++ gcc/testsuite/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,8 @@
+2016-04-04  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/65795
+	* gfortran.dg/coarray_allocate_6.f08: New test.
+
 2016-04-04  Richard Biener  <rguenther@suse.de>
 
 	PR rtl-optimization/70484
Index: gcc/testsuite/gfortran.dg/coarray_allocate_6.f08
===================================================================
--- gcc/testsuite/gfortran.dg/coarray_allocate_6.f08	(nicht existent)
+++ gcc/testsuite/gfortran.dg/coarray_allocate_6.f08	(Arbeitskopie)
@@ -0,0 +1,27 @@
+! { dg-do run }
+! { dg-options "-fcoarray=single -fdump-tree-original" }
+
+! Contributed by Tobias Burnus  <burnus@gcc.gnu.org>
+! Test fix for pr65795.
+
+implicit none
+
+type t2
+  integer, allocatable :: x
+end type t2
+
+type t3
+  type(t2), allocatable :: caf[:]
+end type t3
+
+!type(t3), save, target :: c, d
+type(t3), target :: c, d
+integer :: stat
+
+allocate(c%caf[*], stat=stat)
+end
+
+! Besides checking that the executable does not crash anymore, check
+! that the cause has been remove.
+! { dg-final { scan-tree-dump-not "c.caf.x = 0B" "original" } }
+

[-- Attachment #3: submit.gcc-5.diff --]
[-- Type: text/x-patch, Size: 2404 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(Revision 234706)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,11 @@
+2016-04-04  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/65795
+	* trans-array.c (gfc_array_allocate): When the array is a coarray,
+	do not nullyfing its allocatable components in array_allocate, because
+	the nullify missed the array ref and nullifies the wrong component.
+	Cosmetics.
+
 2016-03-28  Andre Vehreschild  <vehre@gcc.gnu.org>
 
 	PR fortran/70397
Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c	(Revision 234706)
+++ gcc/fortran/trans-array.c	(Arbeitskopie)
@@ -5390,8 +5390,8 @@
   else
       gfc_add_expr_to_block (&se->pre, set_descriptor);
 
-  if ((expr->ts.type == BT_DERIVED)
-	&& expr->ts.u.derived->attr.alloc_comp)
+  if (expr->ts.type == BT_DERIVED && expr->ts.u.derived->attr.alloc_comp
+      && !coarray)
     {
       tmp = gfc_nullify_alloc_comp (expr->ts.u.derived, se->expr,
 				    ref->u.ar.as->rank);
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(Revision 234706)
+++ gcc/testsuite/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,8 @@
+2016-04-04  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/65795
+	* gfortran.dg/coarray_allocate_6.f08: New test.
+
 2016-04-01  Ilya Enkovich  <enkovich.gnu@gmail.com>
 
 	Backport from mainline r234666.
Index: gcc/testsuite/gfortran.dg/coarray_allocate_6.f08
===================================================================
--- gcc/testsuite/gfortran.dg/coarray_allocate_6.f08	(nicht existent)
+++ gcc/testsuite/gfortran.dg/coarray_allocate_6.f08	(Arbeitskopie)
@@ -0,0 +1,27 @@
+! { dg-do run }
+! { dg-options "-fcoarray=single -fdump-tree-original" }
+
+! Contributed by Tobias Burnus  <burnus@gcc.gnu.org>
+! Test fix for pr65795.
+
+implicit none
+
+type t2
+  integer, allocatable :: x
+end type t2
+
+type t3
+  type(t2), allocatable :: caf[:]
+end type t3
+
+!type(t3), save, target :: c, d
+type(t3), target :: c, d
+integer :: stat
+
+allocate(c%caf[*], stat=stat)
+end
+
+! Besides checking that the executable does not crash anymore, check
+! that the cause has been remove.
+! { dg-final { scan-tree-dump-not "c.caf.x = 0B" "original" } }
+