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

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

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

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>

> 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