Fortran : ICE in build_field PR95614 (2nd attempt)

Message ID 0fd8ef3c-109e-48ad-0fa4-4c56f4662ef6@codethink.co.uk
State New
Headers show
Series
  • Fortran : ICE in build_field PR95614 (2nd attempt)
Related show

Commit Message

Mark Eggleston Sept. 29, 2020, 1:03 p.m.
For review.

When the first attempt was committed the result was PR97224 i.e. it 
broke the build of SPECCPU 2006 Games.

I've changed the condition under which the error is produced. It was 
produced in the local symbol was also found as a global symbol and the 
the type of the symbol was not GSYM_UNKNOWN and not GSYM_COMMON.  This 
meant that subroutine names in commons in the SPECCPU source code were 
rejected.

The condition no produces an error if the global symbol is either 
GSYM_MODULE or GSYM_PROGRAM.

The relevant section in the standard (19.3.1 (2)):

"Within its scope, a local identifier of an entity of class (1) or class 
(4) shall not be the same as a global identifier used in that scope 
unless the global identifier

  * is used only as the use-name of a rename in a USE statement,
  * is a common block name (19.3.2),
  * is an external procedure name that is also a generic name, or
  * is an external function name and the inclusive scope is its defining
    subprogram (19.3.3)."

I've added two new test cases for subroutine and function.

I'm not certain about the restriction that the external procedure should 
be a generic name. I have found the earlier standards somewhat confusing 
on the subject, so I haven't determined whether there should be any 
standards dependent code.

-- 
https://www.codethink.co.uk/privacy.html

Comments

Mark Eggleston Oct. 13, 2020, 7:16 a.m. | #1
**ping**

previously omitted commit message added

On 29/09/2020 14:03, Mark Eggleston wrote:
> For review.

>

> When the first attempt was committed the result was PR97224 i.e. it 

> broke the build of SPECCPU 2006 Games.

>

> I've changed the condition under which the error is produced. It was 

> produced in the local symbol was also found as a global symbol and the 

> the type of the symbol was not GSYM_UNKNOWN and not GSYM_COMMON.  This 

> meant that subroutine names in commons in the SPECCPU source code were 

> rejected.

>

> The condition no produces an error if the global symbol is either 

> GSYM_MODULE or GSYM_PROGRAM.

>

> The relevant section in the standard (19.3.1 (2)):

>

> "Within its scope, a local identifier of an entity of class (1) or 

> class (4) shall not be the same as a global identifier used in that 

> scope unless the global identifier

>

>  * is used only as the use-name of a rename in a USE statement,

>  * is a common block name (19.3.2),

>  * is an external procedure name that is also a generic name, or

>  * is an external function name and the inclusive scope is its defining

>    subprogram (19.3.3)."

>

> I've added two new test cases for subroutine and function.

>

> I'm not certain about the restriction that the external procedure 

> should be a generic name. I have found the earlier standards somewhat 

> confusing on the subject, so I haven't determined whether there should 

> be any standards dependent code.

>

[PATCH] Fortran  :  ICE in build_field PR95614

Local identifiers can not be the same as a module name. Original
patch by Steve Kargl resulted in name clashes between common block
names and local identifiers.  A local identifier can be the same as
a global identier if that identifier is not a module or a program.
The original patch was modified to reject global identifiers that
represent a module or a program.

2020-09-29  Steven G. Kargl  <kargl@gcc.gnu.org>
         Mark Eggleston  <markeggleston@gcc.gnu.org>

gcc/fortran/

     PR fortran/95614
     * decl.c (gfc_get_common): Use gfc_match_common_name instead
     of match_common_name.
     * decl.c (gfc_bind_idents): Use gfc_match_common_name instead
     of match_common_name.
     * match.c : Rename match_common_name to gfc_match_common_name.
     * match.c (gfc_match_common): Use gfc_match_common_name instead
     of match_common_name.
     * match.h : Rename match_common_name to gfc_match_common_name.
     * resolve.c (resolve_common_vars): Check each symbol in a
     common block has a global symbol.  If there is a global symbol
     issue an error if the symbol type is a module or a program.

2020-09-29  Mark Eggleston <markeggleston@gcc.gnu.org>

gcc/testsuite/

     PR fortran/95614
     * gfortran.dg/pr95614_1.f90: New test.
     * gfortran.dg/pr95614_2.f90: New test.
     * gfortran.dg/pr95614_3.f90: New test.
     * gfortran.dg/pr95614_4.f90: New test.

-- 
https://www.codethink.co.uk/privacy.html
Richard Biener via Gcc-patches Oct. 13, 2020, 7:58 a.m. | #2
Hi Mark,

OK for master. If you want to backport, that's fine by me but please give
it a few weeks.

Thanks for fixing this.

Paul


On Tue, 13 Oct 2020 at 08:17, Mark Eggleston <mark.eggleston@codethink.co.uk>
wrote:

> **ping**

>

> previously omitted commit message added

>

> On 29/09/2020 14:03, Mark Eggleston wrote:

> > For review.

> >

> > When the first attempt was committed the result was PR97224 i.e. it

> > broke the build of SPECCPU 2006 Games.

> >

> > I've changed the condition under which the error is produced. It was

> > produced in the local symbol was also found as a global symbol and the

> > the type of the symbol was not GSYM_UNKNOWN and not GSYM_COMMON.  This

> > meant that subroutine names in commons in the SPECCPU source code were

> > rejected.

> >

> > The condition no produces an error if the global symbol is either

> > GSYM_MODULE or GSYM_PROGRAM.

> >

> > The relevant section in the standard (19.3.1 (2)):

> >

> > "Within its scope, a local identifier of an entity of class (1) or

> > class (4) shall not be the same as a global identifier used in that

> > scope unless the global identifier

> >

> >  * is used only as the use-name of a rename in a USE statement,

> >  * is a common block name (19.3.2),

> >  * is an external procedure name that is also a generic name, or

> >  * is an external function name and the inclusive scope is its defining

> >    subprogram (19.3.3)."

> >

> > I've added two new test cases for subroutine and function.

> >

> > I'm not certain about the restriction that the external procedure

> > should be a generic name. I have found the earlier standards somewhat

> > confusing on the subject, so I haven't determined whether there should

> > be any standards dependent code.

> >

> [PATCH] Fortran  :  ICE in build_field PR95614

>

> Local identifiers can not be the same as a module name. Original

> patch by Steve Kargl resulted in name clashes between common block

> names and local identifiers.  A local identifier can be the same as

> a global identier if that identifier is not a module or a program.

> The original patch was modified to reject global identifiers that

> represent a module or a program.

>

> 2020-09-29  Steven G. Kargl  <kargl@gcc.gnu.org>

>          Mark Eggleston  <markeggleston@gcc.gnu.org>

>

> gcc/fortran/

>

>      PR fortran/95614

>      * decl.c (gfc_get_common): Use gfc_match_common_name instead

>      of match_common_name.

>      * decl.c (gfc_bind_idents): Use gfc_match_common_name instead

>      of match_common_name.

>      * match.c : Rename match_common_name to gfc_match_common_name.

>      * match.c (gfc_match_common): Use gfc_match_common_name instead

>      of match_common_name.

>      * match.h : Rename match_common_name to gfc_match_common_name.

>      * resolve.c (resolve_common_vars): Check each symbol in a

>      common block has a global symbol.  If there is a global symbol

>      issue an error if the symbol type is a module or a program.

>

> 2020-09-29  Mark Eggleston <markeggleston@gcc.gnu.org>

>

> gcc/testsuite/

>

>      PR fortran/95614

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

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

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

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

>

> --

> https://www.codethink.co.uk/privacy.html

>

>


-- 
"If you can't explain it simply, you don't understand it well enough" -
Albert Einstein

Patch

From 56cd489ae564640e6cd397250f71947d768b796c Mon Sep 17 00:00:00 2001
From: Mark Eggleston <markeggleston@gcc.gnu.org>
Date: Thu, 11 Jun 2020 14:33:51 +0100
Subject: [PATCH] Fortran  :  ICE in build_field PR95614

Local identifiers can not be the same as a module name.  Original
patch by Steve Kargl resulted in name clashes between common block
names and local identifiers.  A local identifier can be the same as
a global identier if that identifier is not a module or a program.
The original patch was modified to reject global identifiers that
represent a module or a program.

2020-09-29  Steven G. Kargl  <kargl@gcc.gnu.org>
	    Mark Eggleston  <markeggleston@gcc.gnu.org>

gcc/fortran/

	PR fortran/95614
	* decl.c (gfc_get_common): Use gfc_match_common_name instead
	of match_common_name.
	* decl.c (gfc_bind_idents): Use gfc_match_common_name instead
	of match_common_name.
	* match.c : Rename match_common_name to gfc_match_common_name.
	* match.c (gfc_match_common): Use gfc_match_common_name instead
	of match_common_name.
	* match.h : Rename match_common_name to gfc_match_common_name.
	* resolve.c (resolve_common_vars): Check each symbol in a
	common block has a global symbol.  If there is a global symbol
	issue an error if the symbol type is a module or a program.

2020-09-29  Mark Eggleston  <markeggleston@gcc.gnu.org>

gcc/testsuite/

	PR fortran/95614
	* gfortran.dg/pr95614_1.f90: New test.
	* gfortran.dg/pr95614_2.f90: New test.
	* gfortran.dg/pr95614_3.f90: New test.
	* gfortran.dg/pr95614_4.f90: New test.
---
 gcc/fortran/decl.c                      | 4 ++--
 gcc/fortran/match.c                     | 5 +++--
 gcc/fortran/match.h                     | 6 ++----
 gcc/fortran/resolve.c                   | 7 +++++++
 gcc/testsuite/gfortran.dg/pr95614_1.f90 | 6 ++++++
 gcc/testsuite/gfortran.dg/pr95614_2.f90 | 6 ++++++
 gcc/testsuite/gfortran.dg/pr95614_3.f90 | 9 +++++++++
 gcc/testsuite/gfortran.dg/pr95614_4.f90 | 9 +++++++++
 8 files changed, 44 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr95614_1.f90
 create mode 100644 gcc/testsuite/gfortran.dg/pr95614_2.f90
 create mode 100644 gcc/testsuite/gfortran.dg/pr95614_3.f90
 create mode 100644 gcc/testsuite/gfortran.dg/pr95614_4.f90

diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 326e6f5db7a..9bfaa60418a 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -6007,7 +6007,7 @@  get_bind_c_idents (void)
       found_id = MATCH_YES;
       gfc_get_ha_symbol (name, &tmp_sym);
     }
-  else if (match_common_name (name) == MATCH_YES)
+  else if (gfc_match_common_name (name) == MATCH_YES)
     {
       found_id = MATCH_YES;
       com_block = gfc_get_common (name, 0);
@@ -6052,7 +6052,7 @@  get_bind_c_idents (void)
 	      found_id = MATCH_YES;
 	      gfc_get_ha_symbol (name, &tmp_sym);
 	    }
-	  else if (match_common_name (name) == MATCH_YES)
+	  else if (gfc_match_common_name (name) == MATCH_YES)
 	    {
 	      found_id = MATCH_YES;
 	      com_block = gfc_get_common (name, 0);
diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index cb09c5f8ec5..bee73e7b008 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -5166,7 +5166,8 @@  gfc_get_common (const char *name, int from_module)
 
 /* Match a common block name.  */
 
-match match_common_name (char *name)
+match
+gfc_match_common_name (char *name)
 {
   match m;
 
@@ -5218,7 +5219,7 @@  gfc_match_common (void)
 
   for (;;)
     {
-      m = match_common_name (name);
+      m = gfc_match_common_name (name);
       if (m == MATCH_ERROR)
 	goto cleanup;
 
diff --git a/gcc/fortran/match.h b/gcc/fortran/match.h
index 7bf70d77016..4ccb5961d2b 100644
--- a/gcc/fortran/match.h
+++ b/gcc/fortran/match.h
@@ -103,11 +103,9 @@  match gfc_match_call (void);
 
 /* We want to use this function to check for a common-block-name
    that can exist in a bind statement, so removed the "static"
-   declaration of the function in match.c.
+   declaration of the function in match.c. */
  
-   TODO: should probably rename this now that it'll be globally seen to
-   gfc_match_common_name.  */
-match match_common_name (char *name);
+match gfc_match_common_name (char *name);
 
 match gfc_match_common (void);
 match gfc_match_block_data (void);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index f4ce49f8432..a210f9aad43 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -936,9 +936,16 @@  static void
 resolve_common_vars (gfc_common_head *common_block, bool named_common)
 {
   gfc_symbol *csym = common_block->head;
+  gfc_gsymbol *gsym;
 
   for (; csym; csym = csym->common_next)
     {
+      gsym = gfc_find_gsymbol (gfc_gsym_root, csym->name);
+      if (gsym && (gsym->type == GSYM_MODULE || gsym->type == GSYM_PROGRAM))
+	gfc_error_now ("Global entity %qs at %L cannot appear in a "
+			"COMMON block at %L", gsym->name,
+			&gsym->where, &csym->common_block->where);
+
       /* gfc_add_in_common may have been called before, but the reported errors
 	 have been ignored to continue parsing.
 	 We do the checks again here.  */
diff --git a/gcc/testsuite/gfortran.dg/pr95614_1.f90 b/gcc/testsuite/gfortran.dg/pr95614_1.f90
new file mode 100644
index 00000000000..f835143365a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr95614_1.f90
@@ -0,0 +1,6 @@ 
+! { dg-do compile }
+
+module m   ! { dg-error ".1." }
+  common m ! { dg-error "cannot appear in a COMMON" }
+end
+
diff --git a/gcc/testsuite/gfortran.dg/pr95614_2.f90 b/gcc/testsuite/gfortran.dg/pr95614_2.f90
new file mode 100644
index 00000000000..9d69a506384
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr95614_2.f90
@@ -0,0 +1,6 @@ 
+! { dg-do compile }
+
+module m        ! { dg-error ".1." }
+  common /xc/ m ! { dg-error "cannot appear in a COMMON" }
+end
+
diff --git a/gcc/testsuite/gfortran.dg/pr95614_3.f90 b/gcc/testsuite/gfortran.dg/pr95614_3.f90
new file mode 100644
index 00000000000..7a66bec46fb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr95614_3.f90
@@ -0,0 +1,9 @@ 
+! { dg-do compile }
+
+subroutine s
+end subroutine
+
+program pr95614
+  common /c1/ s
+  s = 9.0
+end program
diff --git a/gcc/testsuite/gfortran.dg/pr95614_4.f90 b/gcc/testsuite/gfortran.dg/pr95614_4.f90
new file mode 100644
index 00000000000..48f9b9b6825
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr95614_4.f90
@@ -0,0 +1,9 @@ 
+! { dg-do compile }
+
+function f()
+  f = 1.0
+end function
+
+program pr95614
+  common /c1/ f
+end program
-- 
2.11.0