[Fortran] PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)

Message ID CAKwh3qjzrsjqebAw-MF882W-vmtOcvXj8_uRVsiEX6VojyPXxw@mail.gmail.com
State New
Headers show
Series
  • [Fortran] PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)
Related show

Commit Message

Janus Weil March 11, 2019, 9:09 p.m.
Hi all,

the attached patch fixes an ICE-on-invalid problem with parametrized
derived types, more precisely a PDT without any parameters, and
regtests cleanly x86_64-linux-gnu. For the relevant standard quote,
see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89601#c4.

Technically the ICE is a regression, but since it happens on invalid
code only, backporting is not essential IMHO (but might still be
useful). Ok for trunk? And 8-branch?

Cheers,
Janus

Comments

Steve Kargl March 11, 2019, 10:33 p.m. | #1
On Mon, Mar 11, 2019 at 10:09:02PM +0100, Janus Weil wrote:
> 

> Technically the ICE is a regression, but since it happens on invalid

> code only, backporting is not essential IMHO (but might still be

> useful). Ok for trunk? And 8-branch?


Looks good to me with a minor suggestion.  See below.
You may want to wait a bit or ping Paul to give him
a chance to peek at the patch.

>    if (gfc_match_char (')') == MATCH_YES)

> -    goto ok;

> +  {        

> +    if (typeparam)

> +      {

> +	gfc_error_now ("A parameter name is required at %C");


Can you insert "type" so the error reads "A type parameter name"?
Unfortunately, the words parameter has a few too many meanings.

-- 
Steve
Janus Weil March 12, 2019, noon | #2
Hi Steve,

> > Technically the ICE is a regression, but since it happens on invalid

> > code only, backporting is not essential IMHO (but might still be

> > useful). Ok for trunk? And 8-branch?

>

> Looks good to me with a minor suggestion.  See below.


thanks for the comments. Updated patch attached.


> You may want to wait a bit or ping Paul to give him

> a chance to peek at the patch.


I'm putting Paul in CC and will wait until tomorrow night before committing.


> >    if (gfc_match_char (')') == MATCH_YES)

> > -    goto ok;

> > +  {

> > +    if (typeparam)

> > +      {

> > +     gfc_error_now ("A parameter name is required at %C");

>

> Can you insert "type" so the error reads "A type parameter name"?

> Unfortunately, the words parameter has a few too many meanings.


True. I'm using 'type parameter list' now.

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index a3b47ac4980..834c73fc27e 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,10 @@
+2019-03-12  Janus Weil  <janus@gcc.gnu.org>
+
+	PR fortran/89601
+	* decl.c (gfc_match_formal_arglist): Reject empty type parameter lists.
+	(gfc_match_derived_decl): Mark as PDT only if type parameter list was
+	matched successfully.
+
 2019-03-11  Jakub Jelinek  <jakub@redhat.com>
 
 	PR fortran/89651
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index a29e2db0bd6..3ebce880b39 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -6275,7 +6275,16 @@ gfc_match_formal_arglist (gfc_symbol *progname, int st_flag,
     }
 
   if (gfc_match_char (')') == MATCH_YES)
-    goto ok;
+  {        
+    if (typeparam)
+      {
+	gfc_error_now ("A type parameter list is required at %C");
+	m = MATCH_ERROR;
+	goto cleanup;
+      }
+    else
+      goto ok;
+  }
 
   for (;;)
     {
@@ -10217,13 +10226,14 @@ gfc_match_derived_decl (void)
       m = gfc_match_formal_arglist (sym, 0, 0, true);
       if (m != MATCH_YES)
 	gfc_error_recovery ();
+      else
+	sym->attr.pdt_template = 1;
       m = gfc_match_eos ();
       if (m != MATCH_YES)
 	{
 	  gfc_error_recovery ();
 	  gfc_error_now ("Garbage after PARAMETERIZED TYPE declaration at %C");
 	}
-      sym->attr.pdt_template = 1;
     }
 
   if (extended && !sym->components)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9b20bdfe787..8c8d282491b 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-12  Janus Weil  <janus@gcc.gnu.org>
+
+	PR fortran/89601
+	* gfortran.dg/pdt_16.f03: Modified to avoid follow-up errors.
+	* gfortran.dg/pdt_30.f90: New test case.
+
 2019-03-12  Jakub Jelinek  <jakub@redhat.com>
 
 	PR middle-end/89663
diff --git a/gcc/testsuite/gfortran.dg/pdt_16.f03 b/gcc/testsuite/gfortran.dg/pdt_16.f03
index 067d87d660d..22c6b83a084 100644
--- a/gcc/testsuite/gfortran.dg/pdt_16.f03
+++ b/gcc/testsuite/gfortran.dg/pdt_16.f03
@@ -12,7 +12,6 @@ end
 program p
    type t(a                  ! { dg-error "Expected parameter list" }
       integer, kind :: a
-      real(a) :: x
    end type
    type u(a, a)              ! { dg-error "Duplicate name" }
       integer, kind :: a     ! { dg-error "already declared" }
diff --git a/gcc/testsuite/gfortran.dg/pdt_30.f90 b/gcc/testsuite/gfortran.dg/pdt_30.f90
new file mode 100644
index 00000000000..47f7c775465
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pdt_30.f90
@@ -0,0 +1,17 @@
+! { dg-do compile }
+!
+! PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)
+!
+! Contributed by Arseny Solokha <asolokha@gmx.com>
+
+program vw
+  interface
+     real function ul (ki)
+       real :: ki
+     end function ul
+  end interface
+  type :: q8 ()  ! { dg-error "A type parameter list is required" }
+     procedure (ul), pointer, nopass :: pj
+  end type q8
+  type (q8) :: ki
+end program vw
Janus Weil March 13, 2019, 7:56 p.m. | #3
I have just committed the updated patch to trunk as r269658. If anyone
thinks this should be backported to 8-branch, please let me know.

Cheers,
Janus


Am Di., 12. März 2019 um 13:00 Uhr schrieb Janus Weil <janus@gcc.gnu.org>:
>

> Hi Steve,

>

> > > Technically the ICE is a regression, but since it happens on invalid

> > > code only, backporting is not essential IMHO (but might still be

> > > useful). Ok for trunk? And 8-branch?

> >

> > Looks good to me with a minor suggestion.  See below.

>

> thanks for the comments. Updated patch attached.

>

>

> > You may want to wait a bit or ping Paul to give him

> > a chance to peek at the patch.

>

> I'm putting Paul in CC and will wait until tomorrow night before committing.

>

>

> > >    if (gfc_match_char (')') == MATCH_YES)

> > > -    goto ok;

> > > +  {

> > > +    if (typeparam)

> > > +      {

> > > +     gfc_error_now ("A parameter name is required at %C");

> >

> > Can you insert "type" so the error reads "A type parameter name"?

> > Unfortunately, the words parameter has a few too many meanings.

>

> True. I'm using 'type parameter list' now.

>

> Cheers,

> Janus

Patch

diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index 1c738baedaa..5733bf2ac82 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,10 @@ 
+2019-03-11  Janus Weil  <janus@gcc.gnu.org>
+
+	PR fortran/89601
+	* decl.c (gfc_match_formal_arglist): Reject empty type parameter lists.
+	(gfc_match_derived_decl): Mark as PDT only if type parameter list was
+	matched successfully.
+
 2019-03-11  Martin Liska  <mliska@suse.cz>
 
 	* decl.c (match_record_decl): Wrap an option name
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index a29e2db0bd6..39ec413a953 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -6275,7 +6275,16 @@  gfc_match_formal_arglist (gfc_symbol *progname, int st_flag,
     }
 
   if (gfc_match_char (')') == MATCH_YES)
-    goto ok;
+  {        
+    if (typeparam)
+      {
+	gfc_error_now ("A parameter name is required at %C");
+	m = MATCH_ERROR;
+	goto cleanup;
+      }
+    else
+      goto ok;
+  }
 
   for (;;)
     {
@@ -10217,13 +10226,14 @@  gfc_match_derived_decl (void)
       m = gfc_match_formal_arglist (sym, 0, 0, true);
       if (m != MATCH_YES)
 	gfc_error_recovery ();
+      else
+	sym->attr.pdt_template = 1;
       m = gfc_match_eos ();
       if (m != MATCH_YES)
 	{
 	  gfc_error_recovery ();
 	  gfc_error_now ("Garbage after PARAMETERIZED TYPE declaration at %C");
 	}
-      sym->attr.pdt_template = 1;
     }
 
   if (extended && !sym->components)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index aec924f27aa..a571002772c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@ 
+2019-03-11  Janus Weil  <janus@gcc.gnu.org>
+
+	PR fortran/89601
+	* gfortran.dg/pdt_16.f03: Modified to avoid follow-up errors.
+	* gfortran.dg/pdt_30.f90: New test case.
+
 2019-03-11  Martin Liska  <mliska@suse.cz>
 
 	* g++.dg/conversion/simd3.C (foo): Wrap option names
diff --git a/gcc/testsuite/gfortran.dg/pdt_16.f03 b/gcc/testsuite/gfortran.dg/pdt_16.f03
index 067d87d660d..22c6b83a084 100644
--- a/gcc/testsuite/gfortran.dg/pdt_16.f03
+++ b/gcc/testsuite/gfortran.dg/pdt_16.f03
@@ -12,7 +12,6 @@  end
 program p
    type t(a                  ! { dg-error "Expected parameter list" }
       integer, kind :: a
-      real(a) :: x
    end type
    type u(a, a)              ! { dg-error "Duplicate name" }
       integer, kind :: a     ! { dg-error "already declared" }
diff --git a/gcc/testsuite/gfortran.dg/pdt_30.f90 b/gcc/testsuite/gfortran.dg/pdt_30.f90
new file mode 100644
index 00000000000..d07b52f6fab
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pdt_30.f90
@@ -0,0 +1,17 @@ 
+! { dg-do compile }
+!
+! PR 89601: [8/9 Regression] [PDT] ICE: Segmentation fault (in resolve_component)
+!
+! Contributed by Arseny Solokha <asolokha@gmx.com>
+
+program vw
+  interface
+     real function ul (ki)
+       real :: ki
+     end function ul
+  end interface
+  type :: q8 ()  ! { dg-error "A parameter name is required" }
+     procedure (ul), pointer, nopass :: pj
+  end type q8
+  type (q8) :: ki
+end program vw