Re: [PATCH, Fortran] PR fortran/60289 First try on: Fixing character array allocation for class(*) type variable

Re: [PATCH, Fortran] PR fortran/60289 First try on: Fixing character array allocation for class(*) type variable

[Dominique Dhumieres]

> My knowledge of unlimited polymorphic types is limited, but I think that
> this is not correct.

My knowledge of unlimited polymorphic types is even more limited,
then I cannot comment about the correctness of the patch. However

> The new length of the string has to be stored somewhere.  If you don't
> do that, for example the intrinsic len() function won't work.

if I add a line

            if (len(P) /= 20) call abort()

in the 'type is (character(*))' block, the test still succeeds.

Dominique

Re: [PATCH, Fortran] PR fortran/60289 First try on: Fixing character array allocation for class(*) type variable

[Mikael Morin]

Le 17/08/2014 14:46, Dominique Dhumieres a écrit :
>> My knowledge of unlimited polymorphic types is limited, but I think that
>> this is not correct.
> 
> My knowledge of unlimited polymorphic types is even more limited,
> then I cannot comment about the correctness of the patch. However
> 
>> The new length of the string has to be stored somewhere.  If you don't
>> do that, for example the intrinsic len() function won't work.
> 
> if I add a line
> 
>             if (len(P) /= 20) call abort()
> 
> in the 'type is (character(*))' block, the test still succeeds.
> 
Yeah, well.  Good to know.
Then I don't understand what the allocation code is doing. :-/

Sorry, I can't devote more time to that right now.
Mikael

Re: [PATCH, Fortran] PR fortran/60289 First try on: Fixing character array allocation for class(*) type variable

[Mikael Morin]

Le 17/08/2014 14:46, Dominique Dhumieres a écrit :
>> My knowledge of unlimited polymorphic types is limited, but I think that
>> this is not correct.
> 
> My knowledge of unlimited polymorphic types is even more limited,
> then I cannot comment about the correctness of the patch. However
> 
>> The new length of the string has to be stored somewhere.  If you don't
>> do that, for example the intrinsic len() function won't work.
> 
> if I add a line
> 
>             if (len(P) /= 20) call abort()
> 
> in the 'type is (character(*))' block, the test still succeeds.
> 
> Dominique
> 
Here is a failing testcase.
I must admit it wasn't easy to find, and it's probably at least as much
a bug with select as with allocate.
It seems that for unlimited polymorphic entities, the _size vtable field
is used as character length.
So there is no need to explicitly set the character length as the whole
vtable is overwritten upon allocation.  And the patch works.
I don't know how we cope with non-default kind though (which is what the
testcase is about).

Mikael


$ ./unlimited_polymorphic_20
 here
 there

Program aborted. Backtrace:
#0  0x7FDFBA026B57
#1  0x7FDFBA028272
#2  0x7FDFBA0F3218
#3  0x400E24 in test_sub at unlimited_polymorphic_20.f90:24
(discriminator 1)
#4  0x400C0D in test at unlimited_polymorphic_20.f90:6
Abandon
$



program test
    call test_sub
  contains

    subroutine test_sub
    implicit none

    class(*), pointer :: P

    allocate(character(len=20, kind=4)::P)

    select type(P)
        type is (character(len=*, kind=4))
            print *, "here"
            P ="some test string"
            if (P .ne. 4_"some test string") then
                call abort()
            end if
            print *, "there"
            if (len(P) /= 20) call abort
            print *, "not there"
        class default
            print *, "bah"
            call abort()
    end select

    deallocate(P)
    end subroutine test_sub
end program test

Re: [PATCH, Fortran] PR fortran/60289 First try on: Fixing character array allocation for class(*) type variable

[Dominique Dhumieres]

> Here is a failing testcase.

I was about to post the same test. The test fails with two counts:
(1) len(P) == 80,
(2) deallocate(P) fails with

a.out(882,0x7fff75e1d310) malloc: *** error for object 0x7fc801c04978: incorrect checksum for freed object - object was probably modified after being freed.
...

Dominique

Re: [PATCH, Fortran] PR fortran/60289 First try on: Fixing character array allocation for class(*) type variable

[Paul Richard Thomas]

Dear All,

There are known issues with unlimited polymorphic variables
representing characters : see
https://groups.google.com/forum/#!topic/comp.lang.fortran/aRz3HMpblTs
and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55901

One way or another, the variable itself needs to carry the string
length and the kind.  With fixed length and kind=1, the vtable 'size'
does the job.

Tobias has suggested more than once that we use the new array
descriptor as the class container as well. We either do this or add
fields to the class container.

Cheers

Paul

On 17 August 2014 18:39, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
>> Here is a failing testcase.
>
> I was about to post the same test. The test fails with two counts:
> (1) len(P) == 80,
> (2) deallocate(P) fails with
>
> a.out(882,0x7fff75e1d310) malloc: *** error for object 0x7fc801c04978: incorrect checksum for freed object - object was probably modified after being freed.
> ...
>
> Dominique



-- 
The knack of flying is learning how to throw yourself at the ground and miss.
       --Hitchhikers Guide to the Galaxy

Re: [RFC, PATCH, Fortran] PR fortran/60289 Fixing character array allocation for class(*) type variable

[Andre Vehreschild]

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

Dear all,

please find attached a patch fixing the PR60289 with my recent patch on
PR60255. Please understand, that for this patch to work correctly you need to
have my previous patch from

https://gcc.gnu.org/ml/fortran/2014-12/msg00092.html

applied already to your gfortran sources.

I am still collecting comments on the 60255 patch and applying them. Therefore
both patches are on "request for comment" now. So feel free to comment.

Bootstrapping and regtesting is running currently.

- Andre

On Mon, 18 Aug 2014 10:36:44 +0200
Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:

> Dear All,
> 
> There are known issues with unlimited polymorphic variables
> representing characters : see
> https://groups.google.com/forum/#!topic/comp.lang.fortran/aRz3HMpblTs
> and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55901
> 
> One way or another, the variable itself needs to carry the string
> length and the kind.  With fixed length and kind=1, the vtable 'size'
> does the job.
> 
> Tobias has suggested more than once that we use the new array
> descriptor as the class container as well. We either do this or add
> fields to the class container.
> 
> Cheers
> 
> Paul
> 
> On 17 August 2014 18:39, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> >> Here is a failing testcase.
> >
> > I was about to post the same test. The test fails with two counts:
> > (1) len(P) == 80,
> > (2) deallocate(P) fails with
> >
> > a.out(882,0x7fff75e1d310) malloc: *** error for object 0x7fc801c04978:
> > incorrect checksum for freed object - object was probably modified after
> > being freed. ...
> >
> > Dominique
> 
> 
> 


-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 9291018 * Email: vehre@gmx.de 

[-- Attachment #2: pr60289_3.patch --]
[-- Type: text/x-patch, Size: 3021 bytes --]

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 9d7d3c2..214b64d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -6877,7 +6877,9 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code)
       goto failure;
     }
 
-  if (code->ext.alloc.ts.type == BT_CHARACTER && !e->ts.deferred)
+  /* Check F08:C632.  */
+  if (code->ext.alloc.ts.type == BT_CHARACTER && !e->ts.deferred
+      && !UNLIMITED_POLY (e))
     {
       int cmp = gfc_dep_compare_expr (e->ts.u.cl->length,
 				      code->ext.alloc.ts.u.cl->length);
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 0db668d..1c2599c 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -5126,8 +5126,10 @@ gfc_trans_allocate (gfc_code * code)
 	      gfc_add_block_to_block (&se.pre, &se_sz.pre);
 	      se_sz.expr = gfc_evaluate_now (se_sz.expr, &se.pre);
 	      gfc_add_block_to_block (&se.pre, &se_sz.post);
-	      /* Store the string length.  */
-	      tmp = al->expr->ts.u.cl->backend_decl;
+	      /* Store the string length.  Get the backend_decl of the _len
+		 component for that.  */
+	      tmp = gfc_class_len_get (gfc_get_symbol_decl (
+					 expr->symtree->n.sym));
 	      gfc_add_modify (&se.pre, tmp, fold_convert (TREE_TYPE (tmp),
 			      se_sz.expr));
               tmp = TREE_TYPE (gfc_typenode_for_spec (&code->ext.alloc.ts));
diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90
new file mode 100644
index 0000000..18a66b0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90
@@ -0,0 +1,57 @@
+! { dg-do run }
+! Testing fix for PR fortran/60289
+! Contributed by: Andre Vehreschild <vehre@gmx.de>
+!
+program test
+    implicit none
+
+    class(*), pointer :: P
+    integer :: string_len = 10 *2
+
+    allocate(character(string_len)::P)
+
+    select type(P)
+        type is (character(*))
+            P ="some test string"
+            if (P .ne. "some test string") then
+                call abort ()
+            end if
+            if (len(P) .ne. 20) then
+                call abort ()
+            end if
+            if (len(P) .eq. len("some test string")) then
+                call abort ()
+            end if
+        class default
+            call abort ()
+    end select
+
+    deallocate(P)
+
+    ! Now for kind=4 chars.
+
+    allocate(character(len=20,kind=4)::P)
+
+    select type(P)
+        type is (character(len=*,kind=4))
+            P ="some test string"
+            if (P .ne. 4_"some test string") then
+                call abort ()
+            end if
+            if (len(P) .ne. 20) then
+                call abort ()
+            end if
+            if (len(P) .eq. len("some test string")) then
+                call abort ()
+            end if
+        type is (character(len=*,kind=1))
+            call abort ()
+        class default
+            call abort ()
+    end select
+
+    deallocate(P)
+
+
+end program test
+

Re: [RFC, PATCH, Fortran] PR fortran/60289 Fixing character array allocation for class(*) type variable

[Andre Vehreschild]

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

Dear all,

sorry for the bad patch yesterday. Here is an update to that one. It fixes the
pr60289 is GNU-style checked, bootstraps and regtests ok on x86_64-linux-gnu.

Comments welcome.

Regards,
	Andre

On Sat, 20 Dec 2014 16:38:23 +0100
Andre Vehreschild <vehre@gmx.de> wrote:

> Dear all,
> 
> please find attached a patch fixing the PR60289 with my recent patch on
> PR60255. Please understand, that for this patch to work correctly you need to
> have my previous patch from
> 
> https://gcc.gnu.org/ml/fortran/2014-12/msg00092.html
> 
> applied already to your gfortran sources.
> 
> I am still collecting comments on the 60255 patch and applying them. Therefore
> both patches are on "request for comment" now. So feel free to comment.
> 
> Bootstrapping and regtesting is running currently.
> 
> - Andre
> 
> On Mon, 18 Aug 2014 10:36:44 +0200
> Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> 
> > Dear All,
> > 
> > There are known issues with unlimited polymorphic variables
> > representing characters : see
> > https://groups.google.com/forum/#!topic/comp.lang.fortran/aRz3HMpblTs
> > and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55901
> > 
> > One way or another, the variable itself needs to carry the string
> > length and the kind.  With fixed length and kind=1, the vtable 'size'
> > does the job.
> > 
> > Tobias has suggested more than once that we use the new array
> > descriptor as the class container as well. We either do this or add
> > fields to the class container.
> > 
> > Cheers
> > 
> > Paul
> > 
> > On 17 August 2014 18:39, Dominique Dhumieres <dominiq@lps.ens.fr> wrote:
> > >> Here is a failing testcase.
> > >
> > > I was about to post the same test. The test fails with two counts:
> > > (1) len(P) == 80,
> > > (2) deallocate(P) fails with
> > >
> > > a.out(882,0x7fff75e1d310) malloc: *** error for object 0x7fc801c04978:
> > > incorrect checksum for freed object - object was probably modified after
> > > being freed. ...
> > >
> > > Dominique
> > 
> > 
> > 
> 
> 


-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 9291018 * Email: vehre@gmx.de 

[-- Attachment #2: pr60289_4.patch --]
[-- Type: text/x-patch, Size: 3253 bytes --]

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 9d7d3c2..214b64d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -6877,7 +6877,9 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code)
       goto failure;
     }
 
-  if (code->ext.alloc.ts.type == BT_CHARACTER && !e->ts.deferred)
+  /* Check F08:C632.  */
+  if (code->ext.alloc.ts.type == BT_CHARACTER && !e->ts.deferred
+      && !UNLIMITED_POLY (e))
     {
       int cmp = gfc_dep_compare_expr (e->ts.u.cl->length,
 				      code->ext.alloc.ts.u.cl->length);
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 0db668d..7c5ebe0 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -5126,8 +5126,15 @@ gfc_trans_allocate (gfc_code * code)
 	      gfc_add_block_to_block (&se.pre, &se_sz.pre);
 	      se_sz.expr = gfc_evaluate_now (se_sz.expr, &se.pre);
 	      gfc_add_block_to_block (&se.pre, &se_sz.post);
-	      /* Store the string length.  */
-	      tmp = al->expr->ts.u.cl->backend_decl;
+	      /* Store the string length.  Get the backend_decl of the _len
+		 component for that.  */
+	      if ((expr->symtree->n.sym->ts.type == BT_CLASS
+		  || expr->symtree->n.sym->ts.type == BT_DERIVED)
+		  && expr->symtree->n.sym->ts.u.derived->attr.unlimited_polymorphic)
+		tmp = gfc_class_len_get (gfc_get_symbol_decl (
+					   expr->symtree->n.sym));
+	      else
+		tmp = al->expr->ts.u.cl->backend_decl;
 	      gfc_add_modify (&se.pre, tmp, fold_convert (TREE_TYPE (tmp),
 			      se_sz.expr));
               tmp = TREE_TYPE (gfc_typenode_for_spec (&code->ext.alloc.ts));
diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90
new file mode 100644
index 0000000..18a66b0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90
@@ -0,0 +1,57 @@
+! { dg-do run }
+! Testing fix for PR fortran/60289
+! Contributed by: Andre Vehreschild <vehre@gmx.de>
+!
+program test
+    implicit none
+
+    class(*), pointer :: P
+    integer :: string_len = 10 *2
+
+    allocate(character(string_len)::P)
+
+    select type(P)
+        type is (character(*))
+            P ="some test string"
+            if (P .ne. "some test string") then
+                call abort ()
+            end if
+            if (len(P) .ne. 20) then
+                call abort ()
+            end if
+            if (len(P) .eq. len("some test string")) then
+                call abort ()
+            end if
+        class default
+            call abort ()
+    end select
+
+    deallocate(P)
+
+    ! Now for kind=4 chars.
+
+    allocate(character(len=20,kind=4)::P)
+
+    select type(P)
+        type is (character(len=*,kind=4))
+            P ="some test string"
+            if (P .ne. 4_"some test string") then
+                call abort ()
+            end if
+            if (len(P) .ne. 20) then
+                call abort ()
+            end if
+            if (len(P) .eq. len("some test string")) then
+                call abort ()
+            end if
+        type is (character(len=*,kind=1))
+            call abort ()
+        class default
+            call abort ()
+    end select
+
+    deallocate(P)
+
+
+end program test
+

[PATCH, Fortran] PR fortran/60289 Fixing character array allocation for class(*) type variable

[Andre Vehreschild]

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

Hi all,

this patches fixes PR60289 for allocating unlimited polymorphic entities
retyping them to a char array. The patch depends on my former patch for pr60255
at:

https://gcc.gnu.org/ml/fortran/2014-12/msg00130.html

because it needs the _len component introduced. I have extend Janus' patch
given in the PR and added a testcase. 

This is the fifth version of the patch, where the previous hasn't gotten any
comments, so I think it is well enough for commit. What do you think?

Bootstraps and regtests ok on x86_64-linux-gnu.

Depends on: https://gcc.gnu.org/ml/fortran/2014-12/msg00130.html

Regards,
	Andre
-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 9291018 * Email: vehre@gmx.de 

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

gcc/fortran/ChangeLog:

2014-12-29  Andre Vehreschild  <vehre@gmx.de>, Janus Weil <janus@gcc.gnu.org>

	Initial patch by Janus Weil
	* resolve.c (resolve_allocate_expr): Add check for comp. only when 
		target is not unlimited polymorphic.
	* trans-stmt.c (gfc_trans_allocate): Assign correct value to _len
		component of unlimited polymorphic entities.

gcc/testsuite/ChangeLog:

2014-12-29  Andre Vehreschild  <vehre@gmx.de>

	* gfortran.dg/unlimited_polymorphic_21.f90: New test.



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

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 05a948b..6038dd5 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -6930,7 +6930,9 @@ resolve_allocate_expr (gfc_expr *e, gfc_code *code)
       goto failure;
     }
 
-  if (code->ext.alloc.ts.type == BT_CHARACTER && !e->ts.deferred)
+  /* Check F08:C632.  */
+  if (code->ext.alloc.ts.type == BT_CHARACTER && !e->ts.deferred
+      && !UNLIMITED_POLY (e))
     {
       int cmp = gfc_dep_compare_expr (e->ts.u.cl->length,
 				      code->ext.alloc.ts.u.cl->length);
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index c560d05..82ecf31 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -5139,8 +5139,15 @@ gfc_trans_allocate (gfc_code * code)
 	      gfc_add_block_to_block (&se.pre, &se_sz.pre);
 	      se_sz.expr = gfc_evaluate_now (se_sz.expr, &se.pre);
 	      gfc_add_block_to_block (&se.pre, &se_sz.post);
-	      /* Store the string length.  */
-	      tmp = al->expr->ts.u.cl->backend_decl;
+	      /* Store the string length.  Get the backend_decl of the _len
+		 component for that.  */
+	      if ((expr->symtree->n.sym->ts.type == BT_CLASS
+		  || expr->symtree->n.sym->ts.type == BT_DERIVED)
+		  && expr->ts.u.derived->attr.unlimited_polymorphic)
+		tmp = gfc_class_len_get (gfc_get_symbol_decl (
+					   expr->symtree->n.sym));
+	      else
+		tmp = al->expr->ts.u.cl->backend_decl;
 	      gfc_add_modify (&se.pre, tmp, fold_convert (TREE_TYPE (tmp),
 			      se_sz.expr));
               tmp = TREE_TYPE (gfc_typenode_for_spec (&code->ext.alloc.ts));
diff --git a/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90 b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90
new file mode 100644
index 0000000..18a66b0
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/unlimited_polymorphic_21.f90
@@ -0,0 +1,57 @@
+! { dg-do run }
+! Testing fix for PR fortran/60289
+! Contributed by: Andre Vehreschild <vehre@gmx.de>
+!
+program test
+    implicit none
+
+    class(*), pointer :: P
+    integer :: string_len = 10 *2
+
+    allocate(character(string_len)::P)
+
+    select type(P)
+        type is (character(*))
+            P ="some test string"
+            if (P .ne. "some test string") then
+                call abort ()
+            end if
+            if (len(P) .ne. 20) then
+                call abort ()
+            end if
+            if (len(P) .eq. len("some test string")) then
+                call abort ()
+            end if
+        class default
+            call abort ()
+    end select
+
+    deallocate(P)
+
+    ! Now for kind=4 chars.
+
+    allocate(character(len=20,kind=4)::P)
+
+    select type(P)
+        type is (character(len=*,kind=4))
+            P ="some test string"
+            if (P .ne. 4_"some test string") then
+                call abort ()
+            end if
+            if (len(P) .ne. 20) then
+                call abort ()
+            end if
+            if (len(P) .eq. len("some test string")) then
+                call abort ()
+            end if
+        type is (character(len=*,kind=1))
+            call abort ()
+        class default
+            call abort ()
+    end select
+
+    deallocate(P)
+
+
+end program test
+

[PING, PATCH, Fortran] PR fortran/60289 Fixing character array allocation for class(*) type variable

[Andre Vehreschild]

*ping*

On Mon, 29 Dec 2014 14:12:43 +0100
Andre Vehreschild <vehre@gmx.de> wrote:

> Hi all,
> 
> this patches fixes PR60289 for allocating unlimited polymorphic entities
> retyping them to a char array. The patch depends on my former patch for
> pr60255 at:
> 
> https://gcc.gnu.org/ml/fortran/2014-12/msg00130.html
> 
> because it needs the _len component introduced. I have extend Janus' patch
> given in the PR and added a testcase. 
> 
> This is the fifth version of the patch, where the previous hasn't gotten any
> comments, so I think it is well enough for commit. What do you think?
> 
> Bootstraps and regtests ok on x86_64-linux-gnu.
> 
> Depends on: https://gcc.gnu.org/ml/fortran/2014-12/msg00130.html
> 
> Regards,
> 	Andre


-- 
Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen
Tel.: +49 241 9291018 * Email: vehre@gmx.de