[patch, fortran] PR93473 - ICE on valid with long module + submodule names

[Andrew Benson]

I created PR93473 for this problem: The following code causes a bogus "symbol 
is already defined" error (using git commit 
472dc648ce3e7661762931d584d239611ddca964):

module aModestlyLongModuleName
  
  type :: aTypeWithASignificantlyLongNameButStillAllowedOK
  end type aTypeWithASignificantlyLongNameButStillAllowedOK
  
  interface
 module function aFunctionWithALongButStillAllowedName(parameters) 
result(self)
   type(aTypeWithASignificantlyLongNameButStillAllowedOK) :: self
 end function aFunctionWithALongButStillAllowedName
  end interface
  
end module aModestlyLongModuleName

submodule (aModestlyLongModuleName) 
aTypeWithASignificantlyLongNameButStillAllowedOK_

contains

  module procedure aFunctionWithALongButStillAllowedName
 class(*), pointer :: genericObject
  end procedure aFunctionWithALongButStillAllowedName

end submodule aTypeWithASignificantlyLongNameButStillAllowedOK_

submodule 
(aModestlyLongModuleName:aTypeWithASignificantlyLongNameButStillAllowedOK_) 
aSubmoduleWithASignificantlyLongButStillAllowedName__
end submodule aSubmoduleWithASignificantlyLongButStillAllowedName__



$ gfortran -v 
Using built-in specs.
COLLECT_GCC=gfortran
COLLECT_LTO_WRAPPER=/data001/abenson/Galacticus/Tools_Devel_Install/bin/../
libexec/gcc/x86_64-pc-linux-gnu/10.0.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-git/configure --prefix=/home/abenson/Galacticus/
Tools_Devel --enable-languages=c,c++,fortran --disable-multilib
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 10.0.1 20200127 (experimental) (GCC) 


$ gfortran -c 1057.F90 -o test.o  -ffree-line-length-none
f951: internal compiler error: Segmentation fault
0xe1021f crash_signal
../../gcc-git/gcc/toplev.c:328
0x7fd1480c91ef ???
/data001/abenson/Galacticus/Tools/glibc-2.12.1/signal/../sysdeps/unix/
sysv/linux/x86_64/sigaction.c:0
0x891106 do_traverse_symtree
../../gcc-git/gcc/fortran/symbol.c:4173
0x85739b parse_module
../../gcc-git/gcc/fortran/parse.c:6111
0x85782d gfc_parse_file()
../../gcc-git/gcc/fortran/parse.c:6427
0x8a7f2f gfc_be_parse_file
../../gcc-git/gcc/fortran/f95-lang.c:210
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.


The problem occurs in set_syms_host_assoc() where the "parent1" and "parent2" 
variables have a maximum length of GFC_MAX_SYMBOL_LEN+1. This is insufficient 
when the parent names are a module+submodule name concatenated with a ".". The 
patch above fixes this by increasing their length to 2*GFC_MAX_SYMBOL_LEN+2.

A patch to fix this is attached. The patch regression tests cleanly - ok to 
commit?

-Andrew
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 4bff0c8..cbace25 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -6045,8 +6045,8 @@ set_syms_host_assoc (gfc_symbol *sym)
 {
   gfc_component *c;
   const char dot[2] = ".";
-  char parent1[GFC_MAX_SYMBOL_LEN + 1];
-  char parent2[GFC_MAX_SYMBOL_LEN + 1];
+  char parent1[2 * GFC_MAX_SYMBOL_LEN + 2];
+  char parent2[2 * GFC_MAX_SYMBOL_LEN + 2];
 
   if (sym == NULL)
 return;
diff --git a/gcc/testsuite/gfortran.dg/pr93473.f90 b/gcc/testsuite/gfortran.dg/pr93473.f90
new file mode 100644
index 000..dda8525
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr93473.f90
@@ -0,0 +1,28 @@
+! { dg-do compile }
+! { dg-options "-ffree-line-length-none" }
+! PR fortran/93473
+module aModestlyLongModuleName
+  
+  type :: aTypeWithASignificantlyLongNameButStillAllowedOK
+  end type aTypeWithASignificantlyLongNameButStillAllowedOK
+  
+  interface
+ module function aFunctionWithALongButStillAllowedName(parameters) result(self)
+   type(aTypeWithASignificantlyLongNameButStillAllowedOK) :: self
+ end function aFunctionWithALongButStillAllowedName
+  end interface
+  
+end module aModestlyLongModuleName
+
+submodule (aModestlyLongModuleName) aTypeWithASignificantlyLongNameButStillAllowedOK_
+
+contains
+
+  module procedure aFunctionWithALongButStillAllowedName
+ class(*), pointer :: genericObject
+  end procedure aFunctionWithALongButStillAllowedName
+
+end submodule aTypeWithASignificantlyLongNameButStillAllowedOK_
+
+submodule (aModestlyLongModuleName:aTypeWithASignificantlyLongNameButStillAllowedOK_) aSubmoduleWithASignificantlyLongButStillAllowedName__
+end submodule aSubmoduleWithASignificantlyLongButStillAllowedName__
2020-01-27  Andrew Benson  

	PR fortran/93473
	* parse.c: Increase length of char variables to allow them to hold
	a concatenated module + submodule name.

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

Re: [patch, fortran] PR93473 - ICE on valid with long module + submodule names

[Tobias Burnus]

On 1/28/20 12:41 AM, Andrew Benson wrote:

The problem occurs in set_syms_host_assoc() where the "parent1" and "parent2"
variables have a maximum length of GFC_MAX_SYMBOL_LEN+1. This is insufficient
when the parent names are a module+submodule name concatenated with a ".". The
patch above fixes this by increasing their length to 2*GFC_MAX_SYMBOL_LEN+2.

A patch to fix this is attached. The patch regression tests cleanly - ok to
commit?


The patch is okay, but can you add a comment – similar to the other 
patch – which makes clear why one needs twice the normal 
GFC_MAX_SYMBOL_LEN (+2)? You currently have:



const char dot[2] = ".";
-  char parent1[GFC_MAX_SYMBOL_LEN + 1];
-  char parent2[GFC_MAX_SYMBOL_LEN + 1];
+  char parent1[2 * GFC_MAX_SYMBOL_LEN + 2];
+  char parent2[2 * GFC_MAX_SYMBOL_LEN + 2];


Tobias

Re: [patch, fortran] PR93473 - ICE on valid with long module + submodule names

[Andrew Benson]

On Tuesday, January 28, 2020 9:27:58 AM PST Tobias Burnus wrote:
> On 1/28/20 12:41 AM, Andrew Benson wrote:
> > The problem occurs in set_syms_host_assoc() where the "parent1" and
> > "parent2" variables have a maximum length of GFC_MAX_SYMBOL_LEN+1. This
> > is insufficient when the parent names are a module+submodule name
> > concatenated with a ".". The patch above fixes this by increasing their
> > length to 2*GFC_MAX_SYMBOL_LEN+2.
> > 
> > A patch to fix this is attached. The patch regression tests cleanly - ok
> > to
> > commit?
> 
> The patch is okay, but can you add a comment – similar to the other
> patch – which makes clear why one needs twice the normal
> 
> GFC_MAX_SYMBOL_LEN (+2)? You currently have:
> > const char dot[2] = ".";
> > 
> > -  char parent1[GFC_MAX_SYMBOL_LEN + 1];
> > -  char parent2[GFC_MAX_SYMBOL_LEN + 1];
> > +  char parent1[2 * GFC_MAX_SYMBOL_LEN + 2];
> > +  char parent2[2 * GFC_MAX_SYMBOL_LEN + 2];

Yes - I've added a comment here explaining the naming convention. An updated 
patch is attached.

-Andrew

-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus
2020-01-28  Andrew Benson  

	PR fortran/93473
	* parse.c: Increase length of char variables to allow them to hold
	a concatenated module + submodule name.

	* gfortran.dg/pr93473.f90: New test.
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index 4bff0c8..f71a95d 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -6045,8 +6045,9 @@ set_syms_host_assoc (gfc_symbol *sym)
 {
   gfc_component *c;
   const char dot[2] = ".";
-  char parent1[GFC_MAX_SYMBOL_LEN + 1];
-  char parent2[GFC_MAX_SYMBOL_LEN + 1];
+  /* Symbols take the form module.submodule_ or module.name_. */
+  char parent1[2 * GFC_MAX_SYMBOL_LEN + 2];
+  char parent2[2 * GFC_MAX_SYMBOL_LEN + 2];
 
   if (sym == NULL)
 return;
diff --git a/gcc/testsuite/gfortran.dg/pr93473.f90 b/gcc/testsuite/gfortran.dg/pr93473.f90
new file mode 100644
index 000..dda8525
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr93473.f90
@@ -0,0 +1,28 @@
+! { dg-do compile }
+! { dg-options "-ffree-line-length-none" }
+! PR fortran/93473
+module aModestlyLongModuleName
+  
+  type :: aTypeWithASignificantlyLongNameButStillAllowedOK
+  end type aTypeWithASignificantlyLongNameButStillAllowedOK
+  
+  interface
+ module function aFunctionWithALongButStillAllowedName(parameters) result(self)
+   type(aTypeWithASignificantlyLongNameButStillAllowedOK) :: self
+ end function aFunctionWithALongButStillAllowedName
+  end interface
+  
+end module aModestlyLongModuleName
+
+submodule (aModestlyLongModuleName) aTypeWithASignificantlyLongNameButStillAllowedOK_
+
+contains
+
+  module procedure aFunctionWithALongButStillAllowedName
+ class(*), pointer :: genericObject
+  end procedure aFunctionWithALongButStillAllowedName
+
+end submodule aTypeWithASignificantlyLongNameButStillAllowedOK_
+
+submodule (aModestlyLongModuleName:aTypeWithASignificantlyLongNameButStillAllowedOK_) aSubmoduleWithASignificantlyLongButStillAllowedName__
+end submodule aSubmoduleWithASignificantlyLongButStillAllowedName__

Re: [patch, fortran] PR93473 - ICE on valid with long module + submodule names

[Tobias Burnus]

LGTM now.

Thanks,

Tobias

On 1/28/20 5:41 PM, Andrew Benson wrote:

On Tuesday, January 28, 2020 9:27:58 AM PST Tobias Burnus wrote:

On 1/28/20 12:41 AM, Andrew Benson wrote:

The problem occurs in set_syms_host_assoc() where the "parent1" and
"parent2" variables have a maximum length of GFC_MAX_SYMBOL_LEN+1. This
is insufficient when the parent names are a module+submodule name
concatenated with a ".". The patch above fixes this by increasing their
length to 2*GFC_MAX_SYMBOL_LEN+2.

A patch to fix this is attached. The patch regression tests cleanly - ok
to
commit?

The patch is okay, but can you add a comment – similar to the other
patch – which makes clear why one needs twice the normal

GFC_MAX_SYMBOL_LEN (+2)? You currently have:

 const char dot[2] = ".";

-  char parent1[GFC_MAX_SYMBOL_LEN + 1];
-  char parent2[GFC_MAX_SYMBOL_LEN + 1];
+  char parent1[2 * GFC_MAX_SYMBOL_LEN + 2];
+  char parent2[2 * GFC_MAX_SYMBOL_LEN + 2];

Yes - I've added a comment here explaining the naming convention. An updated
patch is attached.

-Andrew

Re: [patch, fortran] PR93473 - ICE on valid with long module + submodule names

[Andrew Benson]

Thanks - committed as a83b5cc5828ee34471de415e8893242dd3b0a91b 

https://gcc.gnu.org/git/gitweb.cgi?
p=gcc.git;a=commit;h=a83b5cc5828ee34471de415e8893242dd3b0a91b

I'll close the PR.

Thanks,
Andrew

On Tuesday, January 28, 2020 6:32:21 PM PST Tobias Burnus wrote:
> LGTM now.
> 
> Thanks,
> 
> Tobias
> 
> On 1/28/20 5:41 PM, Andrew Benson wrote:
> > On Tuesday, January 28, 2020 9:27:58 AM PST Tobias Burnus wrote:
> >> On 1/28/20 12:41 AM, Andrew Benson wrote:
> >>> The problem occurs in set_syms_host_assoc() where the "parent1" and
> >>> "parent2" variables have a maximum length of GFC_MAX_SYMBOL_LEN+1. This
> >>> is insufficient when the parent names are a module+submodule name
> >>> concatenated with a ".". The patch above fixes this by increasing their
> >>> length to 2*GFC_MAX_SYMBOL_LEN+2.
> >>> 
> >>> A patch to fix this is attached. The patch regression tests cleanly - ok
> >>> to
> >>> commit?
> >> 
> >> The patch is okay, but can you add a comment – similar to the other
> >> patch – which makes clear why one needs twice the normal
> >> 
> >> GFC_MAX_SYMBOL_LEN (+2)? You currently have:
> >>>  const char dot[2] = ".";
> >>> 
> >>> -  char parent1[GFC_MAX_SYMBOL_LEN + 1];
> >>> -  char parent2[GFC_MAX_SYMBOL_LEN + 1];
> >>> +  char parent1[2 * GFC_MAX_SYMBOL_LEN + 2];
> >>> +  char parent2[2 * GFC_MAX_SYMBOL_LEN + 2];
> > 
> > Yes - I've added a comment here explaining the naming convention. An
> > updated patch is attached.
> > 
> > -Andrew


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus