[PR,fortran/89077] - ICE using * as len specifier for character parameter

Message ID 5C575745.60609@gmx.de
State New
Headers show
Series
  • [PR,fortran/89077] - ICE using * as len specifier for character parameter
Related show

Commit Message

Harald Anlauf Feb. 3, 2019, 9:04 p.m.
The attached patch fixes an ICE-on-valid that probably goes back to
rev.128130.  Apparently the patch applied back then did not check
this code path which resulted in a NULL pointer dereference.  This
is remedied by the new testcase base on comment #0 in this PR.

The PR mentions another wrong-code issue to be addressed separately.

OK for trunk?  And shall this fix be backported?

Thanks,
Harald

2019-02-03  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/89077
	* decl.c (add_init_expr_to_sym): Copy length of string initializer
	to declared symbol.

2019-02-03  Harald Anlauf  <anlauf@gmx.de>

	PR fortran/89077
	* gfortran.dg/pr89077.f90: New test.

Index: gcc/testsuite/gfortran.dg/pr89077.f90
===================================================================
--- gcc/testsuite/gfortran.dg/pr89077.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr89077.f90	(working copy)
@@ -0,0 +1,11 @@
+! { dg-do run }
+!
+! PR fortran/89077 - ICE using * as len specifier for character parameter
+
+program test
+  implicit none
+  integer :: i
+  character(*), parameter :: s = 'abcdef'
+  character(*), parameter :: t = transfer ([(s(i:i), i=1,len(s))], s)
+  if (len (t) /= len (s) .or. t /= s) stop 1
+end

Comments

Paul Richard Thomas Feb. 4, 2019, 9:49 a.m. | #1
Hi Harald,

After nearly 12 years, I have been found out!

OK for trunk. Since it has been such a long time, I suggest that you
limit your backporting efforts to 8-branch and, at the most, 7-branch.

Will you attempt to tackle the other issues in the PR?

Thanks

Paul

On Sun, 3 Feb 2019 at 21:04, Harald Anlauf <anlauf@gmx.de> wrote:
>

> The attached patch fixes an ICE-on-valid that probably goes back to

> rev.128130.  Apparently the patch applied back then did not check

> this code path which resulted in a NULL pointer dereference.  This

> is remedied by the new testcase base on comment #0 in this PR.

>

> The PR mentions another wrong-code issue to be addressed separately.

>

> OK for trunk?  And shall this fix be backported?

>

> Thanks,

> Harald

>

> 2019-02-03  Harald Anlauf  <anlauf@gmx.de>

>

>         PR fortran/89077

>         * decl.c (add_init_expr_to_sym): Copy length of string initializer

>         to declared symbol.

>

> 2019-02-03  Harald Anlauf  <anlauf@gmx.de>

>

>         PR fortran/89077

>         * gfortran.dg/pr89077.f90: New test.

>



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein
Harald Anlauf Feb. 4, 2019, 8:55 p.m. | #2
Hi Paul,

On 02/04/19 10:49, Paul Richard Thomas wrote:
> Hi Harald,

> 

> After nearly 12 years, I have been found out!


you cannot hide forever.

> OK for trunk. Since it has been such a long time, I suggest that you

> limit your backporting efforts to 8-branch and, at the most, 7-branch.


Sure.  In fact, I plan to wait with backports in the hope to better
understand the other issues in the PR.

> Will you attempt to tackle the other issues in the PR?


I am still trying, but also wondering why such simple things as

  integer :: i
  character(*), parameter :: s = 'abcdef'
  character(1)            :: t = transfer ([ s(1:1)       ], 'x') ! no ICE
  character(1)            :: u = transfer ([(s(i:i),i=1,1)], 'x') ! ICE
  print *, len (t), len (u)      ! ICE happens when u is referenced
end

are going down so different routes during simplification. :-(
Any hint is highly welcome!

Thanks,
Harald

> Thanks

> 

> Paul

> 

> On Sun, 3 Feb 2019 at 21:04, Harald Anlauf <anlauf@gmx.de> wrote:

>>

>> The attached patch fixes an ICE-on-valid that probably goes back to

>> rev.128130.  Apparently the patch applied back then did not check

>> this code path which resulted in a NULL pointer dereference.  This

>> is remedied by the new testcase base on comment #0 in this PR.

>>

>> The PR mentions another wrong-code issue to be addressed separately.

>>

>> OK for trunk?  And shall this fix be backported?

>>

>> Thanks,

>> Harald

>>

>> 2019-02-03  Harald Anlauf  <anlauf@gmx.de>

>>

>>         PR fortran/89077

>>         * decl.c (add_init_expr_to_sym): Copy length of string initializer

>>         to declared symbol.

>>

>> 2019-02-03  Harald Anlauf  <anlauf@gmx.de>

>>

>>         PR fortran/89077

>>         * gfortran.dg/pr89077.f90: New test.

>>

> 

> 



-- 
Harald Anlauf
Dieburger Str. 17
60386 Frankfurt
Tel.: (069) 4014 8318
Steve Kargl Feb. 4, 2019, 9:07 p.m. | #3
On Mon, Feb 04, 2019 at 09:55:39PM +0100, Harald Anlauf wrote:
> 

> On 02/04/19 10:49, Paul Richard Thomas wrote:

> > Hi Harald,

> > 

> > After nearly 12 years, I have been found out!

> 

> you cannot hide forever.

> 

> > OK for trunk. Since it has been such a long time, I suggest that you

> > limit your backporting efforts to 8-branch and, at the most, 7-branch.

> 

> Sure.  In fact, I plan to wait with backports in the hope to better

> understand the other issues in the PR.

> 

> > Will you attempt to tackle the other issues in the PR?

> 

> I am still trying, but also wondering why such simple things as

> 

>   integer :: i

>   character(*), parameter :: s = 'abcdef'

>   character(1)            :: t = transfer ([ s(1:1)       ], 'x') ! no ICE

>   character(1)            :: u = transfer ([(s(i:i),i=1,1)], 'x') ! ICE

>   print *, len (t), len (u)      ! ICE happens when u is referenced

> end

> 

> are going down so different routes during simplification. :-(

> Any hint is highly welcome!


The best explanation I have is that gfortran sort of doesn't
know how to handle implied-do-loops.  Sometimes gfortran
does what you expect, and sometimes you're off in the weeds
trying to understand why 'i' is handled correctly.

-- 
Steve

Patch

Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c	(revision 268502)
+++ gcc/fortran/decl.c	(working copy)
@@ -1921,7 +1921,7 @@ 
 		    }
 		  else if (init->ts.u.cl && init->ts.u.cl->length)
 		    sym->ts.u.cl->length =
-				gfc_copy_expr (sym->value->ts.u.cl->length);
+				gfc_copy_expr (init->ts.u.cl->length);
 		}
 	    }
 	  /* Update initializer character length according symbol.  */