[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
commit r10-7115-gdf15a82804e1f7f4a7432670b33387779de46549
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Mar 10 17:51:46 2020 -0400

    c++: Fix ICE with omitted template args [PR93956].

    reshape_init only wants to work on BRACE_ENCLOSED_INITIALIZER_P, i.e. raw
    initializer lists, and here was getting a CONSTRUCTOR that had already been
    processed for type A<int>.  maybe_aggr_guide should also use that test.

    gcc/cp/ChangeLog
    2020-03-10  Jason Merrill  <jason@redhat.com>

            PR c++/93956
            * pt.c (maybe_aggr_guide): Check BRACE_ENCLOSED_INITIALIZER_P.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |pault at gcc dot gnu.org,
                   |                            |tkoenig at gcc dot gnu.org
   Last reconfirmed|                            |2020-04-14
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to martin from comment #0)
> The following code (compiled without any options) prints "15000, 10000" for
> the second call to foo, instead of the expected "20000, 10000". The first
> call to foo gives the expected result. This is also reproducible with
> gfortran-10 branch.
> 
> This code is a variation of the one reported in bug 93918 to further
> understand temporary array creation.
> 
> program array_temps
> implicit none
> 
> type :: tt
>    integer :: u = 1
>    integer :: v = 2
> end type tt
> 
> type(tt), dimension(:), pointer :: r
> integer :: n
> integer, dimension(:), pointer :: p
> 
> allocate(r(1:n))

This test case does not define n.

Assuming you want to write n=10000 in there,
I concur that the output should be as you describe.

I assume that the span is not used correctly in the
function.

The test case works if you do

p => r(:)%v

instead of the call to get().

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[mscfd at gmx dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #3 from martin <mscfd at gmx dot net> ---
Sorry, indeed the line 'n=10000' is missing. Not sure how that single line got
lost.

There are a few possibilities as a work around for this simple case, but the
context where I came upon this bug is (as often) more involved. There the get
routine has an additional argument selecting the component of the type and the
aim was to put this selection process into a separate subroutine or function.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #4 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Taking the slightly modified test case

program array_temps
  implicit none

  type :: tt
     integer :: u = 1
     integer :: v = 2
  end type tt

  type(tt), dimension(:), pointer :: r
  integer :: n
  integer, dimension(:), pointer :: p

  n = 10
  allocate(r(1:n))
  p => get(r)
  call foo(p, n)
  print *,sum(p)

  deallocate(r)

contains

   subroutine foo(a, n)
      integer, dimension(:), intent(in) :: a
      integer, intent(in) :: n
      print *, sum(a(1:n)), n
   end subroutine foo

   function get(x) result(q)
      type(tt), dimension(:), target, intent(in) :: x
      integer, dimension(:), pointer :: q
      q => x(:)%v
   end function get
end program array_temps

and looking at -fdump-tree-original shows something strange.

get looks good:

  {
    integer(kind=8) D.3946;
    integer(kind=8) D.3947;

    D.3946 = ubound.0;
    __result->span = 8;
    __result->dtype = {.elem_len=4, .rank=1, .type=1};
    D.3947 = stride.1;
    __result->dim[0].lbound = 1;
    __result->dim[0].ubound = D.3946;
    __result->dim[0].stride = NON_LVALUE_EXPR <D.3947>;
    __result->data = (void *) &(*x.0)[0].v;
    __result->offset = -NON_LVALUE_EXPR <D.3947>;
  }

so the result for span is set.

The call to get and foo does not look to bad, either:

    {
      struct array01_integer(kind=4) ptrtemp.15;
      struct array01_tt * D.4002;
      struct tt[0:] * ifm.16;
      integer(kind=8) D.4004;
      integer(kind=8) D.4005;

      ptrtemp.15.span = 4;
      D.4002 = &r;
      ifm.16 = (struct tt[0:] *) D.4002->data;
      D.4004 = (D.4002->dim[0].ubound - D.4002->dim[0].lbound) + 1;
      D.4005 = -NON_LVALUE_EXPR <D.4002->dim[0].stride>;
      get (&ptrtemp.15, D.4002);
      p = ptrtemp.15;
    }
    foo (&p, &n);

But it seems that foo does not use the span at all.

OK, so what about the test case

program array_temps
  implicit none

  type :: tt
     integer :: u = 1
     integer :: v = 2
  end type tt

  type(tt), dimension(:), pointer :: r
  integer :: n
  integer, dimension(:), pointer :: p

  n = 10
  allocate(r(1:n))
  p => r%v
  call foo(p, n)
  print *,sum(p)

deallocate(r)

contains

   subroutine foo(a, n)
      integer, dimension(:), intent(in) :: a
      integer, intent(in) :: n
      print *, sum(a(1:n)), n
   end subroutine foo

end program array_temps

?

There, we actually convert the argument on call to foo:

   p = r;
    p.data = (void *) &(*(struct tt[0:] *) r.data)[0].v;
    p.span = r.span;
    p.dim[0].ubound = p.dim[0].ubound + (1 - p.dim[0].lbound);
    p.offset = p.offset - (1 - p.dim[0].lbound) * p.dim[0].stride;
    p.dim[0].lbound = 1;
    {
      integer(kind=4)[0:] * D.3975;
      integer(kind=8) D.3976;
      integer(kind=8) D.3977;
      integer(kind=8) D.3978;
      integer(kind=8) D.3979;
      struct array01_integer(kind=4) atmp.11;
      logical(kind=4) D.3987;
      integer(kind=8) D.3988;
      void * restrict D.3989;
      void * restrict D.3990;
      integer(kind=8) D.3991;
      integer(kind=4)[0:] * D.3995;
      integer(kind=8) D.3996;
      integer(kind=8) D.3997;
      integer(kind=8) D.3998;
      integer(kind=8) D.3999;

      D.3975 = (integer(kind=4)[0:] *) p.data;
      D.3976 = p.offset;
      D.3977 = p.dim[0].lbound;
      D.3978 = p.dim[0].ubound;
      D.3979 = D.3978 - D.3977;
            typedef integer(kind=4) [0:];
      atmp.11.dtype = {.elem_len=4, .rank=1, .type=1};
      atmp.11.dim[0].stride = 1;
      atmp.11.dim[0].lbound = 0;
      atmp.11.dim[0].ubound = D.3979;
      D.3987 = D.3979 < 0;
      D.3988 = D.3979 + 1;
      atmp.11.span = 4;
      D.3989 = (void * restrict) __builtin_malloc (D.3987 ? 1 : MAX_EXPR
<(unsigned long) (D.3988 * 4), 1>);
      D.3990 = D.3989;
      atmp.11.data = D.3990;
      atmp.11.offset = 0;
      D.3991 = NON_LVALUE_EXPR <D.3977>;
      {
        integer(kind=8) S.12;
        integer(kind=8) D.3993;

        D.3993 = p.dim[0].stride;
        S.12 = 0;
        while (1)
          {
            if (S.12 > D.3979) goto L.3;
            (*(integer(kind=4)[0:] * restrict) atmp.11.data)[S.12] =
*((integer(kind=4) *) D.3975 + (sizetype) (((S.12 + D.3991) * D.3993 + D.3976)
* p.span));
            S.12 = S.12 + 1;
          }
        L.3:;
      }
      foo (&atmp.11, &n);
      __builtin_free ((void *) atmp.11.data);

This is not ideal from a performance perspective, but at least
it generated correct code.

So, it appears that somewhere, that conversion goes missing
(and it would also be enough just to convert the descriptor,
no real need to copy the data).

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
So, the problem seems to be that sym->attr.subref_array_pointer is
not set on the original test case.  It should be set by
p => get(r(:)) (or by an equivalent call get2(r)) because
we don't know what the particular subroutine may be doing.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |tkoenig at gcc dot gnu.org

--- Comment #6 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Let's see if I can get this fixed.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #7 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Created attachment 48328
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48328&action=edit
Patch which sould create the right temporaries

Well, this should to the trick - at least fixes the test case.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Thomas Kथà¤nig <tkoenig@gcc.gnu.org>:

https://gcc.gnu.org/g:06eca1acafa27e19e82dc73927394a7a4d0bdbc5

commit r10-7920-g06eca1acafa27e19e82dc73927394a7a4d0bdbc5
Author: Thomas König <tkoenig@gcc.gnu.org>
Date:   Thu Apr 23 20:30:01 2020 +0200

    Fix PR 93956, wrong pointer when returned via function.

    This one took a bit of detective work.  When array pointers point
    to components of derived types, we currently set the span field
    and then create an array temporary when we pass the array
    pointer to a procedure as a non-pointer or non-target argument.
    (This is inefficient, but that's for another release).

    Now, the compiler detected this case when there was a direct assignment
    like p => a%b, but not when p was returned either as a function result
    or via an argument.  This patch fixes that.

    2020-04-23  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            * expr.c (gfc_check_pointer_assign): Also set subref_array_pointer
            when a function returns a pointer.
            * interface.c (gfc_set_subref_array_pointer_arg): New function.
            (gfc_procedure_use): Call it.

    2020-04-23  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            * gfortran.dg/pointer_assign_13.f90: New test.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Thomas Kथà¤nig
<tkoenig@gcc.gnu.org>:

https://gcc.gnu.org/g:2a732dbdfcc0a3bc2b4bdb5387fffa193fea6df6

commit r9-8541-g2a732dbdfcc0a3bc2b4bdb5387fffa193fea6df6
Author: Thomas König <tkoenig@gcc.gnu.org>
Date:   Fri Apr 24 08:22:48 2020 +0200

    Fix PR 93956, wrong pointer when returned via function.

    Backport from trunk.

    This one took a bit of detective work.  When array pointers point
    to components of derived types, we currently set the span field
    and then create an array temporary when we pass the array
    pointer to a procedure as a non-pointer or non-target argument.
    (This is inefficient, but that's for another release).

    Now, the compiler detected this case when there was a direct assignment
    like p => a%b, but not when p was returned either as a function result
    or via an argument.  This patch fixes that.

    2020-04-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            * expr.c (gfc_check_pointer_assign): Also set subref_array_pointer
            when a function returns a pointer.
            * interface.c (gfc_set_subref_array_pointer_arg): New function.
            (gfc_procedure_use): Call it.

    2020-04-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            * gfortran.dg/pointer_assign_13.f90: New test.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Thomas Kथà¤nig
<tkoenig@gcc.gnu.org>:

https://gcc.gnu.org/g:aadc54867cc200ad7d073222769b9de7f13b5bcd

commit r8-10218-gaadc54867cc200ad7d073222769b9de7f13b5bcd
Author: Thomas König <tkoenig@gcc.gnu.org>
Date:   Fri Apr 24 09:26:48 2020 +0200

    Fix PR 93956, wrong pointer when returned via function.

    Backport from trunk.

    This one took a bit of detective work.  When array pointers point
    to components of derived types, we currently set the span field
    and then create an array temporary when we pass the array
    pointer to a procedure as a non-pointer or non-target argument.
    (This is inefficient, but that's for another release).

    Now, the compiler detected this case when there was a direct assignment
    like p => a%b, but not when p was returned either as a function result
    or via an argument.  This patch fixes that.

    2020-04-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            * expr.c (gfc_check_pointer_assign): Also set subref_array_pointer
            when a function returns a pointer.
            * interface.c (gfc_set_subref_array_pointer_arg): New function.
            (gfc_procedure_use): Call it.

    2020-04-24  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            * gfortran.dg/pointer_assign_13.f90: New test.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #11 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Fixed on all open branches.

Thanks a lot for the bug report!

Regards

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #12 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Fixed.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Thomas Kथà¤nig <tkoenig@gcc.gnu.org>:

https://gcc.gnu.org/g:2bf7698e0d2312410e7aaab5ee8447e25d8bf8a6

commit r10-7971-g2bf7698e0d2312410e7aaab5ee8447e25d8bf8a6
Author: Thomas Koenig <tkoenig@gcc.gnu.org>
Date:   Sun Apr 26 14:51:01 2020 +0200

    Add ChangeLog changes from previous commit, r10-7920.

    PR fortran/93956

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Thomas Kथà¤nig <tkoenig@gcc.gnu.org>:

https://gcc.gnu.org/g:d8df7c404e233abb1e26d8b8370c460732904531

commit r10-8001-gd8df7c404e233abb1e26d8b8370c460732904531
Author: Thomas Koenig <tkoenig@gcc.gnu.org>
Date:   Mon Apr 27 23:49:36 2020 +0200

    Revert r10-7920-g06eca1acafa27e19e82dc73927394a7a4d0bdbc5 .

    2020-04-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            PR fortran/94788
            * expr.c (gfc_check_pointer_assign): Revert patch for PR 93956.
            * interface.c: Likewise.

    2020-04-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

            PR fortran/93956
            PR fortran/94788
            * gfortran.dg/pointer_assign_13.f90: Remove.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=94788

--- Comment #15 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Reverted on trunk to fix PR 94788.

I will revert the commit to the branches shortly.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[pault at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #16 from Paul Thomas <pault at gcc dot gnu.org> ---
Hi Thomas,

I am sorry that you are having such a hassle with this - perhaps this helps?

> 
> I assume that the span is not used correctly in the
> function.

Yes, that is correct. Within 'foo', we have 

    a.0 = (integer(kind=4)[0:D.3967] * restrict) a->data;

and the sum is inlined as:

        while (1)
          {
            if (S.10 > D.3961) goto L.1;
            val.9 = (*a.0)[S.10 * D.3963 + D.3960] + val.9;
            S.10 = S.10 + 1;
          }
        L.1:;

so that the sum becomes: sum(r(1:500)%u) + sum(r(1:500)%v).

This fixes the problem at the expense of more temporaries but regtests fine:

diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index a9fa03ad153..8ca2b9bb3cb 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -1098,10 +1098,14 @@ is_subref_array (gfc_expr * e)
   bool seen_array;
   gfc_symbol *sym;

-  if (e->expr_type != EXPR_VARIABLE)
-    return false;

-  sym = e->symtree->n.sym;
+  if (e->expr_type == EXPR_VARIABLE)
+    sym = e->symtree->n.sym;
+  else if (e->expr_type == EXPR_FUNCTION
+          && gfc_expr_attr (e).subref_array_pointer)
+    return true;
+  else
+    return false;

   if (sym->attr.subref_array_pointer)
     return true;
@@ -4242,7 +4246,7 @@ gfc_check_pointer_assign (gfc_expr *lvalue, gfc_expr
*rvalue,
   if (rvalue->expr_type == EXPR_NULL)
     return true;

-  if (rvalue->expr_type == EXPR_VARIABLE && is_subref_array (rvalue))
+  if (is_subref_array (rvalue))
     lvalue->symtree->n.sym->attr.subref_array_pointer = 1;

   attr = gfc_expr_attr (rvalue);


Paul

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #17 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Thomas Kथà¤nig
<tkoenig@gcc.gnu.org>:

https://gcc.gnu.org/g:cb2c76c8b156c6d8541ddb3aa894568a2de3b02b

commit r9-8557-gcb2c76c8b156c6d8541ddb3aa894568a2de3b02b
Author: Thomas Koenig <tkoenig@gcc.gnu.org>
Date:   Fri May 1 14:45:56 2020 +0200

    Revert patch for PR fortran/93956.

    2020-04-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

                PR fortran/93956
                PR fortran/94788
                * expr.c (gfc_check_pointer_assign): Revert patch for PR 93956.
                * interface.c: Likewise.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

--- Comment #18 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Thomas Kथà¤nig
<tkoenig@gcc.gnu.org>:

https://gcc.gnu.org/g:b92bb10248a8f99cecf71a54c56bd4d8c75a322f

commit r8-10228-gb92bb10248a8f99cecf71a54c56bd4d8c75a322f
Author: Thomas Koenig <tkoenig@gcc.gnu.org>
Date:   Fri May 1 14:45:56 2020 +0200

    Revert patch for PR fortran/93956.

    2020-04-27  Thomas Koenig  <tkoenig@gcc.gnu.org>

                PR fortran/93956
                PR fortran/94788
                * expr.c (gfc_check_pointer_assign): Revert patch for PR 93956.
                * interface.c: Likewise.

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956
Bug 93956 depends on bug 94788, which changed state.

Bug 94788 Summary: [8/9 Regression] Severe regression leading to double free in tcache
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94788

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

[Bug fortran/93956] Wrong array creation with p => array_dt(1:n)%component

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93956

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|tkoenig at gcc dot gnu.org         |unassigned at gcc dot gnu.org
             Status|REOPENED                    |NEW