[Bug fortran/49324] New: iso_varying_string and reshape fail

[Bug fortran/49324] New: iso_varying_string and reshape fail

[jjcogliati-r1 at yahoo dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

           Summary: iso_varying_string and reshape fail
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jjcogliati-r1@yahoo.com


Iso varying string seems to fail with reshape.

I am using the iso_varying_string module available from 
http://www.fortran.com/iso_varying_string.f95

My test case is:
!--------------
program test_ivs
  use iso_varying_string
  implicit none

  type(varying_string),dimension(:,:),allocatable :: array2d
  type(varying_string) :: extra
  integer :: i,j

  allocate(array2d(2,3))

  extra = "four"

  array2d(:,:) = reshape((/ var_str("1"), &
       var_str("2"), var_str("3"), &
       extra, var_str("5"), &
       var_str("six") /), (/ 2, 3 /))


  print *,"array2d second ",ubound(array2d),(("'"//char(array2d(i,j))//"'
",i=1,size(array2d,1)),j=1,size(array2d,2))

end program test_ivs
!-----------------

With this test case, I get the output:

 array2d second            2           3 '' '0' 'P' 'P!&\x02' '0' '!&' 


If I modify it to:
!---------------------
program test_ivs_no_reshape
  use iso_varying_string
  implicit none

  type(varying_string),dimension(:,:),allocatable :: array2d
  type(varying_string) :: extra
  integer :: i,j

  allocate(array2d(2,3))

  extra = "four"

  array2d(:,1) = (/ var_str("1"), var_str("2") /)
  array2d(:,2) = (/ var_str("3"), extra /)
  array2d(:,3) = (/ var_str("5"), var_str("six") /)

  print *,"array2d first ",ubound(array2d),(("'"//char(array2d(i,j))//"'
",i=1,size(array2d,1)),j=1,size(array2d,2))

end program test_ivs_no_reshape
!------------------

I get the following output:
 array2d first            2           3 '1' '2' '3' 'four' '5' 'six' 

which is what I expected. 

$ gfortran -v
Using built-in specs.
COLLECT_GCC=gfortran
COLLECT_LTO_WRAPPER=/home/jjc/gcc/gcc_460_install/libexec/gcc/x86_64-unknown-linux-gnu/4.6.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-4.6.0/configure --prefix=/home/jjc/gcc/gcc_460_install/
Thread model: posix
gcc version 4.6.0 (GCC) 

Compiling:
$ gfortran -Wall -c iso_varying_string.f95
$ gfortran -Wall -o test_ivs.f90 test_ivs.f90 iso_varying_string.o
$ ./test_ivs
 array2d second            2           3 '' '0' 'P' 'PQ�' '0' 'Q�'

[Bug fortran/49324] iso_varying_string and reshape fail

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

Tobias Burnus <burnus at gcc dot gnu.org> changed:

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

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-08 16:22:37 UTC ---
Not an analysis, just some observations ...

I get different results with different compilers:
gfortran 4.7:
 array2d second            2           3 '' '@' '`' '`A�▒' '@' 'A�'
ifort 11.1:
  array2d second            2           3 '/' '/' '/' '/' '/' '/'
NAG 5.1 and pgf90 10.1-0:
  array2d second  2 3 '' '' '' 'four' '' ''
pathf95 and g95:
 array2d second  2 3 '1' '2' '3' 'four' '5' 'six'

[Neither NAG with -C=all -C=undefined, nor ifort -check all, nor gfortran
-fcheck=all show an error.]


I tried also the other ISO Varying String implementations at
ftp://ftp.nag.co.uk/sc22wg5/ISO_VARYING_STRING/Sample_Module/ , but I fail to
get a consistent result with those.

ifort with either iso_vst.f90 or iso_vsta.f90:
 array2d second            2           3 '1' '1' '1' '1' '1' '1'
gfortran, g95, NAG, pgf90 and pathf95 with iso_vst.f90
and with g95, pgf90 and pathf95 for iso_vsta.f90:
 array2d second            2           3 '1' '2' '3' 'four' '5' 'six'
gfortran with iso_vsta.f90:
 array2d second            2           3 '' '�' '�' '�jb' '�' 'b'
NAG with iso_vsta.f90:
  ALLOCATABLE array allocatable array component is not currently allocated
  Program terminated by fatal error
  In S_ASS_S, line 342 of iso_vsta.f90
  Called by TEST_IVS, line 13 of hj4f.f90

[Bug fortran/49324] iso_varying_string and reshape fail

[jjcogliati-r1 at yahoo dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #2 from Joshua Cogliati <jjcogliati-r1 at yahoo dot com> 2011-06-08 16:30:16 UTC ---
(In reply to comment #1)
> Not an analysis, just some observations ...
> 
> I get different results with different compilers:
> gfortran 4.7:
>  array2d second            2           3 '' '@' '`' '`A�▒' '@' 'A�'
> [Neither NAG with -C=all -C=undefined, nor ifort -check all, nor gfortran
> -fcheck=all show an error.]
> 
> 
> I tried also the other ISO Varying String implementations at
> ftp://ftp.nag.co.uk/sc22wg5/ISO_VARYING_STRING/Sample_Module/ , but I fail to
> get a consistent result with those.
> 
> gfortran, g95, NAG, pgf90 and pathf95 with iso_vst.f90
> and with g95, pgf90 and pathf95 for iso_vsta.f90:
>  array2d second            2           3 '1' '2' '3' 'four' '5' 'six'
> gfortran with iso_vsta.f90:
>  array2d second            2           3 '' '�' '�' '�jb' '�' 'b'

Hm.  http://www.fortran.com/iso_varying_string.f95 and iso_vsta.f90 use:

TYPE VARYING_STRING
 PRIVATE 
 CHARACTER,DIMENSION(:),ALLOCATABLE :: chars
ENDTYPE VARYING_STRING 

iso_vst.f90 uses:
TYPE VARYING_STRING
 PRIVATE 
 CHARACTER,DIMENSION(:),POINTER :: chars => NULL()
ENDTYPE VARYING_STRING 


so for gfortran at least, it seems to happen when there is a allocatable
character array, but not a pointer.

[Bug fortran/49324] iso_varying_string and reshape fail

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.06.08 22:12:30
                 CC|                            |pault at gcc dot gnu.org
     Ever Confirmed|0                           |1
      Known to fail|                            |4.3.4, 4.4.0, 4.5.1, 4.6.0,
                   |                            |4.7.0
           Severity|normal                      |major

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-08 22:12:30 UTC ---
First: The module http://www.fortran.com/iso_varying_string.f95 is invalid, cf.
PR 49331. (That shouldn't affect this PR.)

Secondly, there seems to be a serious bug in gfortran - and several other
compilers - in the handling of constructors of derived types with allocatable
components. With gfortran, I see three bugs for the following program:

- There is no reallocate on assignment
- The memory is multiple times freed for a normal constructor
- With reshape, the result is wrong (+ one has an invalid free)

Expected: With and without the explicit allocate of z and z2, the program
should print:
 11 11 22 22 22
 11 11 22 22 22

Actual result (w/ explicit allocate)
          11          11          22          22          22
           0           0           0           0  -202116348
  (Plus segfault)


implicit none
type t
  integer, allocatable :: A(:)
end type t

type(t) :: x, y
type(t), allocatable :: z(:), z2(:)

allocate (x%A(2))
allocate (y%A(3))
x%A = 11
y%A = 22

! BUG 1: Realloc LHS does not work,
! hence the following lines are required:
allocate (z(2))
allocate (z2(2))

z = [ x, y ]
print *, z(1)%a, z(2)%a  ! OK, if allocated

! BUG 2: Crash with invalid FREE at the the program

! BUG 3: The following produces garbage
z2 = reshape ([ x, y ], [ 2 ])
print *, z2(1)%a, z2(2)%a
end

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Wrong result with           |Deep copy missing for array
                   |constructor of derived      |constructors of DT w/
                   |types w/ allocatable        |allocatable components
                   |components                  |

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-09 07:26:41 UTC ---
Paul: As you (with Erik) have implemented alloc components: Do you know the
purpose of the "if (r_is_var)"? (See bottom of this comment.) I wouldn't mind
if you could work on this PR as I would like to concentrate on other gfortran
and in particular urgent non-gfortran issues...

 * * *

The cause of the failure is not really surprising; the issue is related to the
missing deep copy of the arguments:

       D.1604 = (struct t[0:] * restrict) z.data;
       (*(struct t[2] * restrict) atmp.8.data)[0] = x;
       (*(struct t[2] * restrict) atmp.8.data)[1] = y;
       (*D.1604)[(S.11 + D.1613) + D.1605]
             = (*(struct t[2] * restrict) atmp.8.data)[S.11];

The code is OK if there are no allocatable components. However, with
allocatable components, one cannot simply assign the structure, but one needs
to deep copy the (allocatable) components. For ALLOCATE (..., SOURCE=...) and
for intrinsic assignment, a similar issue exists (and is mostly solved, except
for polymorphic variables, cf. PR46174).


If one looks at gfc_trans_scalar_assign, which is called in
gfc_trans_assignment_1, one finds:

5308 else if (ts.type == BT_DERIVED && ts.u.derived->attr.alloc_comp)
...
5341   /* Do a deep copy if the rhs is a variable, if it is not the
5342      same as the lhs.  */
5343   if (r_is_var)
5344     {
5345       tmp = gfc_copy_alloc_comp (ts.u.derived, rse->expr, lse->expr, 0);
5346       tmp = build3_v (COND_EXPR, cond, build_empty_stmt (input_location),
5347                       tmp);
5348       gfc_add_expr_to_block (&block, tmp);
5349     }

The "r_is_var" is given by (cf. trans-expr.c:6156):
  expr_is_variable (expr2) || scalar_to_array
and as expr2 is an EXPR_ARRAY and not a variable ...

(For non-variables, the check "Are the rhs and the lhs the same?" does not make
sense, cf. trans-expr.c:5312-5319.)

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-09 22:22:39 UTC ---
The following should partially helps: It solves the segfault for
  z(:) = [ x, y ]
and does a deep copy.

TODO:
- I miss a freeing of the components of the LHS, there is currently just a
malloc, cf. 
- Realloc on assign does not work properly:
  * Valgrind shows: "Conditional jump or move depends on uninitialised value"
  * Values print Ok, but afterwards it segfaults
- with reshape, the values are still wrong.

--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -5347,6 +5347,9 @@ gfc_trans_scalar_assign (gfc_se * lse, gfc_se * rse,
gfc_typespec ts,
                          tmp);
          gfc_add_expr_to_block (&block, tmp);
        }
+      else
+       gfc_add_expr_to_block (&block,gfc_copy_alloc_comp (ts.u.derived,
+                                                          rse->expr,
lse->expr, 0));
     }
   else if (ts.type == BT_DERIVED || ts.type == BT_CLASS)
     {

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-10 08:35:02 UTC ---
(In reply to comment #5)
> The following should partially help

It does - but it also leads to two test suite failures:
  gfortran.dg/alloc_comp_assign_5.f90 - failure around line 53. Output:
      "if (ctr /= 1) call abort ()"
    fails as ctr is 5 instead of 1
  gfortran.dg/alloc_comp_assign_6.f90
    "ERROR: should be ef, got: AA           2"

Thus, the patch of comment 5 should be scratched. Next try: Use in the argument
"|| expr_type == EXPR_ARRAY" (in gfc_conv_expr_descriptor and
gfc_trans_assignment_1), which seems to work.

 * * *

The realloc on assignment still does not work: I get a segfault for the simple
example:

  type t
    integer, allocatable :: A(:)
  end type t
  type(t) :: x
  type(t), allocatable :: z(:)
  print *, "(1)"
  z = [ x ]
  print *, "(2)"
  print *, allocated(z)
  end

Excerpt (from the optimized dump):
  D.1622_32 = __builtin_malloc (48);
  SR.15_179 = MEM[(struct t[0:] *)D.1622_33].data;

[...]

<bb 12>:
  if (SR.15_179 != 0B)
    goto <bb 6>;

The problem is: D.1622_33.data is never set. Depending on the malloc
implementation, the returned memory may or may not be filled with NULL. If one
forces that the returned memory is NULL, it works (by chance), if not, one gets
- like I do - a segfault. Also valgrind shows a "Conditional jump or move
depends on uninitialised value" warning.

 * * *

For RESHAPE, gfortran seems to be a nontrivial bug: While the argument contains
the allocatable components, one simply calls:

          _gfortran_reshape (&D.1662, D.1685, D.1691, 0B, 0B);

And for obvious reasons, doing reshape does not know anything about allocatable
components and does only a shallow copy:
   memcpy(rptr, src, size);

The solution is presumably to do a normal assignment - and only modify the
shape in _gfortran_reshape.

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-10 12:45:13 UTC ---
Submitted patch, which fixes - except of RESHAPE - the issue of comment 0 and
comment 3:
  http://gcc.gnu.org/ml/fortran/2011-06/msg00095.html

TODO: See comment 6: Namely, for realloc on assignment, the malloced memory is
not NULLified - and RESHAPE is wrong (missing deep copy).

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #8 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-11 22:08:49 UTC ---
Author: burnus
Date: Sat Jun 11 22:08:46 2011
New Revision: 174959

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174959
Log:
2011-06-12  Tobias Burnus

        PR fortran/49324
        * trans-expr.c (gfc_trans_assignment_1): Tell
        gfc_trans_scalar_assign to also deep-copy RHS nonvariables
        with allocatable components.
        * trans-array.c (gfc_conv_expr_descriptor): Ditto.

2011-06-12  Tobias Burnus

        PR fortran/49324
        * gfortran.dg/alloc_comp_assign_11.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/alloc_comp_assign_11.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #9 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-14 13:07:07 UTC ---
(Comment fixed 8 fixed the missing deep copy.)

Regarding the reallocate (cf. comment 6, but using scalars to reduce the dump
size): For
  type t
    integer, allocatable :: A
  end type t
  type(t), allocatable :: z
...
  z = [ x ]

One gets the following dump:

/* If not initialized. For arrays: Also realloc, if the size is wrong.  */
        if (z != 0B) goto L.1;
        z = (struct t *) __builtin_malloc (8);

So far so good, but one then has

        D.1552 = *z;
            if (D.1552.a != 0B)
                __builtin_free ((void *) D.1552.a);

which is only OK if "z.data" has neither been just allocated nor reallocated.
If it has, there are two problems: (a) The old memory is not freed. (b) if
malloc/realloc does not return nullified memory, free() operates on some random
pointer.

The question is how to handle it best? The assignment is handled in
gfc_trans_assignment_1, which calls gfc_alloc_allocatable_for_assignment for
the realloc and gfc_trans_scalar_assign for the assignment.

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #10 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-06-17 06:11:35 UTC ---
Author: burnus
Date: Fri Jun 17 06:11:31 2011
New Revision: 175137

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175137
Log:
2011-06-17  Tobias Burnus

        PR fortran/49324
        * trans-expr.c (gfc_trans_assignment_1): Tell
        gfc_trans_scalar_assign to also deep-copy RHS nonvariables
        with allocatable components.
        * trans-array.c (gfc_conv_expr_descriptor): Ditto.

2011-06-17  Tobias Burnus

        PR fortran/49324
        * gfortran.dg/alloc_comp_assign_11.f90: New.


Added:
    branches/gcc-4_6-branch/gcc/testsuite/gfortran.dg/alloc_comp_assign_11.f90
Modified:
    branches/gcc-4_6-branch/gcc/fortran/ChangeLog
    branches/gcc-4_6-branch/gcc/fortran/trans-array.c
    branches/gcc-4_6-branch/gcc/fortran/trans-expr.c
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[jjcogliati-r1 at yahoo dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

Joshua Cogliati <jjcogliati-r1 at yahoo dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.6.1

--- Comment #11 from Joshua Cogliati <jjcogliati-r1 at yahoo dot com> 2011-07-11 16:37:28 UTC ---
Still in 4.6.1, using original test case.

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49324

--- Comment #12 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-07-11 16:52:54 UTC ---
(In reply to comment #11)
> Still in 4.6.1, using original test case.

Well, that's the reason that this PR is not closed.

Fixed (in 4.6 and 4.7-trunk): Deep copy of array constructor.

To be done: Moving the copying for RESHAPE from the library to the front end -
as the front end (now) knows to deal with deep copying.

[Bug fortran/49324] Deep copy missing for array constructors of DT w/ allocatable components

[damian at archaeologic dot codes]

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

Damian Rouson <damian at archaeologic dot codes> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |damian at archaeologic dot codes

--- Comment #13 from Damian Rouson <damian at archaeologic dot codes> ---
Is this related to 100650?