Bug 33941 - [4.3 Regression] gfortran creates module files it can't read
[4.3 Regression] gfortran creates module files it can't read
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: fortran
4.3.0
: P3 normal
: 4.3.0
Assigned To: Not yet assigned to anyone
: rejects-valid
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2007-10-29 17:03 UTC by Toby White
Modified: 2007-10-31 15:11 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-10-30 16:23:53


Attachments
file causes failure on compilation (20.27 KB, text/plain)
2007-10-29 17:04 UTC, Toby White
Details
dependency of problem file (4.26 KB, text/plain)
2007-10-30 10:48 UTC, Toby White
Details
test case for failure (35.76 KB, application/x-tar)
2007-10-30 10:55 UTC, Toby White
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Toby White 2007-10-29 17:03:53 UTC
The attached file is compiled fine by gfortran.

However, when the resulting module is USEd in another subprogram, then the following message results:

Fatal Error: Reading module m_common_attrs at line 44 column 39: Bad name

This is with gfortran 4.3.0, 200710005, rev 127783 under Cygwin.
Comment 1 Toby White 2007-10-29 17:04:55 UTC
Created attachment 14435 [details]
file causes failure on compilation
Comment 2 Tobias Burnus 2007-10-29 19:32:27 UTC
The example is incomplete as the module m_common_array_str is not in the example file.
Comment 3 Toby White 2007-10-30 10:48:48 UTC
Created attachment 14439 [details]
dependency of problem file
Comment 4 Toby White 2007-10-30 10:55:46 UTC
Created attachment 14440 [details]
test case for failure

Rather than uploading all the dependencies, here is a tarball. Unpack it, cd into common/ and then make.

The problem occurs in any module that USEs m_common_attrs
Comment 5 Dominique d'Humieres 2007-10-30 12:07:49 UTC
The make in the tar file fails with:

m_common_error.f90:4.35:

  use pxf, only: pxfabort, pxfflush
                                  1
Fatal Error: Can't open module file 'pxf.mod' for reading at (1): No such file or directory
make: *** [m_common_error.o] Error 1
Comment 6 Toby White 2007-10-30 12:23:24 UTC
So much for trying to cut down the testcase.

To replicate the bug

wget http://source.uszla.me.uk/FoX/FoX-3.0.tgz && \
tar xzf FoX-3.0.tgz && cd FoX-3.0 && \
./configure && make

(or if you have gfortran installed somewhere weird, then specify to configure as:

./configure FC=/path/to/gfortran
Comment 7 Dominique d'Humieres 2007-10-30 13:06:40 UTC
I see the failure for the FOX-3.0 tarball with both revisions 129038 and 129734.
Note that the make succeeds with gfortran 4.2.2. So this seems to be a 4.3 regression, but not a really recent one.
Comment 8 Dominique d'Humieres 2007-10-30 14:15:10 UTC
Comparing the working mod file to the non working one, I have found that "GT" was replaced by ">" and "LE" by "<=". I ignored the warning "If you edit this, you'll get what you deserve." and did the changes in the non working files and the error disappeared. So apparently the writing of mod files have been changed to write ">" and "<=", but the reading has not been modified to accomodate the change.

Note that the make failed at 

/Volumes/Interne-150/_opt/f1-gcc4.3w/bin/gfortran -c -g -O2 -I../objs/finclude   m_dom_dom.f90
m_dom_dom.f90:10630.37:

    character(len=getsystemId_len(np, associated(np))) :: c
                                    1
Error: Inquiry function 'associated' at (1) is not permitted in an initialization expression
...

I don't know if the error is another regression or a "compile invalid" in 4.2.2.
Comment 9 Dominique d'Humieres 2007-10-30 14:50:49 UTC
Reduced test case:

module m_common_elstack

  implicit none
  private

  ! Simple stack to keep track of which elements have appeared so far

  ! Initial stack size:
  integer, parameter :: STACK_SIZE_INIT = 10
  ! Multiplier when stack is exceeded:
  real, parameter :: STACK_SIZE_MULT = 1.5

  type :: elstack_item
    character, dimension(:), pointer :: data
  end type elstack_item

  type :: elstack_t
    private
    integer                                   :: n_items
    type(elstack_item), pointer, dimension(:) :: stack
  end type elstack_t

  public :: elstack_t

  public  :: pop_elstack

contains

  !-----------------------------------------------------------------
  function pop_elstack(elstack) result(item)
    type(elstack_t), intent(inout)     :: elstack
    character(len=merge(size(elstack%stack(elstack%n_items)%data), 0, elstack%n_items > 0)) :: item

    integer :: n

  end function pop_elstack

end module m_common_elstack

program test
use m_common_elstack
end program

[karma] bug/dir_com% gfc m_common_elstack_red.f90
Fatal Error: Reading module m_common_elstack at line 28 column 38: Bad name

line 28 of m_common_elstack.mod:

'' (OP (LOGICAL 4 0 0 LOGICAL ()) 0 > (VARIABLE (INTEGER 4 0 0 INTEGER ())

[karma] bug/dir_com% gfortran m_common_elstack_red.f90
[karma] bug/dir_com% gfortran -v
Using built-in specs.
Target: powerpc-apple-darwin8
Configured with: ../gcc-4.2.2/configure --prefix=/sw --prefix=/sw/lib/gcc4.2 --mandir=/sw/share/man --infodir=/sw/share/info --enable-languages=c,c++,fortran,objc,java --host=powerpc-apple-darwin8 --with-gmp=/sw --with-libiconv-prefix=/sw --with-system-zlib --x-includes=/usr/X11R6/include --x-libraries=/usr/X11R6/lib
Thread model: posix
gcc version 4.2.2

line 20/21 of m_common_elstack.mod:

(CONSTANT (INTEGER 4 ()) 0 '0')) ('' (OP (LOGICAL 4 ()) 0 GT (VARIABLE (
INTEGER 4 ()) 0 5 ((COMPONENT 6 8 'n_items'))) (CONSTANT (INTEGER 4 ())


Comment 10 Francois-Xavier Coudert 2007-10-30 16:23:26 UTC
Further reduced:

module foo
contains
  function pop(n) result(item)
    integer :: n
    character(len=merge(0, 0, n > 0)) :: item
  end function pop
end module foo

program test
  use foo
end program
Comment 11 Dominique d'Humieres 2007-10-30 16:45:30 UTC
The use of '<', '>', and '=' has been introduced in revision 126468. However looking at proc parse_atom in gcc/fortran/module.c, they do not seem to be in the recognized character set.
Comment 12 Tobias Burnus 2007-10-30 17:27:12 UTC
(In reply to comment #11)
> The use of '<', '>', and '=' has been introduced in revision 126468. However
> looking at proc parse_atom in gcc/fortran/module.c, they do not seem to be in
> the recognized character set.

Well, the patch which exactly does in code what you said in words is the following:

Index: gcc/fortran/module.c
===================================================================
--- gcc/fortran/module.c        (Revision 129766)
+++ gcc/fortran/module.c        (Arbeitskopie)
@@ -1195,6 +1195,11 @@ parse_atom (void)
     case 'X':
     case 'Y':
     case 'Z':
+    /* For intrinsic operators such as /= or >=.  */
+    case '=':
+    case '<':
+    case '>':
+    case '/':
       parse_name (c);
       return ATOM_NAME;

Comment 13 Dominique d'Humieres 2007-10-30 18:43:44 UTC
> Well, the patch which exactly does in code what you said in words
> is the following:

Obviously, I would have forgotten "/"! Is not it a good candidate for the "obvious" rule?

Comment 14 patchapp@dberlin.org 2007-10-30 19:40:37 UTC
Subject: Bug number PR33941

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-10/msg01832.html
Comment 15 Dominique d'Humieres 2007-10-30 23:07:44 UTC
Sorry, but the Tobias' patch is not enough. With the full test I get now:

Fatal Error: Reading module m_common_attrs at line 120 column 55: Expected left parenthesis

line 120 reads:

INTEGER ()) 0 '0')) (OP (LOGICAL 4 0 0 LOGICAL ()) 0 <= (VARIABLE (

with column 55 between at =

Reduced test case

module foo
contains
  function pop(n) result(item)
    integer :: n
    character(len=merge(0, 0, n <= 0)) :: item
  end function pop
end module foo

program test
  use foo
end program

I don't know the module syntax and what is expected, but clearly the parser needs some improvement(s).

Comment 16 Dominique d'Humieres 2007-10-30 23:10:55 UTC
If I replace <= by == or /=, the error becomes:

Fatal Error: Reading module foo at line 21 column 16: find_enum(): Enum not found

Comment 17 Francois-Xavier Coudert 2007-10-30 23:20:18 UTC
For other operators, ie .eqv., we output them as the simple EQV string (without the dots). What about outputting the troublesome operators into simple alphanumeric strings, like SIGN_EQ for ==, SIGN_NE for /=, SIGN_LE for <=, etc. That would mean changing the output routines, but not the parser (SIGN_* is a bit of a lousy naming, but I don't have any better idea).
Comment 18 Dominique d'Humieres 2007-10-30 23:25:24 UTC
> (SIGN_* is a bit of a lousy naming, but I don't have any better idea)

what about GFC_* ? However I wonder about possible conflicts with user defined operator. Is that so difficult to parse ==, /=, <=, and =>?

Comment 19 Dominique d'Humieres 2007-10-31 11:11:17 UTC
Minimal quick and dirty patch (including Tobias' one) that fixes the composite relational operators:

--- /opt/gcc/_gcc-clean/gcc/fortran/module.c    2007-10-28 21:01:20.000000000 +0100
+++ /opt/gcc/gcc-4.3-work/gcc/fortran/module.c  2007-10-31 11:27:08.000000000 +0100
@@ -1087,7 +1087,8 @@
   for (;;)
     {
       c = module_char ();
-      if (!ISALNUM (c) && c != '_' && c != '-')
+      /* Added '=' for intrinsic operators such as /= or >=.  */
+      if (!ISALNUM (c) && c != '_' && c != '-' && c != '=')
        break;
 
       *p++ = c;
@@ -1195,6 +1196,11 @@
     case 'X':
     case 'Y':
     case 'Z':
+    /* For intrinsic operators such as /= or >=.  */
+    case '=':
+    case '<':
+    case '>':
+    case '/':
       parse_name (c);
       return ATOM_NAME;

It assumes that parse_name is used to read a valid module. If this is not the case, it leads to:

Fatal Error: Reading module foo at line 30 column 17: find_enum(): Enum not found

(checked on a hacked module file, note that it would be more user friendly to replace 'Enum' by the actual atom_name).

Final note for Toby: making FOX-3.0 fails at m_dom_dom.f90 with:

m_dom_dom.f90:10630.37:

    character(len=getsystemId_len(np, associated(np))) :: c
                                    1
Error: Inquiry function 'associated' at (1) is not permitted in an initialization expression
...
m_dom_dom.f90:928.37:

    character(len=getnodeName_len(np, associated(np))) :: c
                                    1
Error: Inquiry function 'associated' at (1) is not permitted in an initialization expression
m_dom_dom.f90:3838.21:

          if (str_vs(np%elExtras%namespaceNodes%nodes(i)%this%elExtras%namespac
                    1
Error: Procedure argument at (1) is local to a PURE procedure and has the POINTER attribute
m_dom_dom.f90:3848.23:

            if (str_vs(np%elExtras%ownerElement%elExtras%namespaceNodes%nodes(i
                      1
Error: Procedure argument at (1) is local to a PURE procedure and has the POINTER attribute
Fatal Error: Error count reached limit of 25.

If you think that these errors are buggy, please open a new PR.
Comment 20 Tobias Burnus 2007-10-31 13:35:11 UTC
Alternative patch (bootstraps/regtests).

I'm not sure how soon I can submit it.

Index: gcc/fortran/module.c
===================================================================
--- gcc/fortran/module.c        (Revision 129791)
+++ gcc/fortran/module.c        (Arbeitskopie)
@@ -2627,17 +2627,17 @@
     minit ("OR", INTRINSIC_OR),
     minit ("EQV", INTRINSIC_EQV),
     minit ("NEQV", INTRINSIC_NEQV),
-    minit ("==", INTRINSIC_EQ),
+    minit ("EQ_SIGN", INTRINSIC_EQ),
     minit ("EQ", INTRINSIC_EQ_OS),
-    minit ("/=", INTRINSIC_NE),
+    minit ("NE_SIGN", INTRINSIC_NE),
     minit ("NE", INTRINSIC_NE_OS),
-    minit (">", INTRINSIC_GT),
+    minit ("GT_SIGN", INTRINSIC_GT),
     minit ("GT", INTRINSIC_GT_OS),
-    minit (">=", INTRINSIC_GE),
+    minit ("GE_SIGN", INTRINSIC_GE),
     minit ("GE", INTRINSIC_GE_OS),
-    minit ("<", INTRINSIC_LT),
+    minit ("LT_SIGN", INTRINSIC_LT),
     minit ("LT", INTRINSIC_LT_OS),
-    minit ("<=", INTRINSIC_LE),
+    minit ("LE_SIGN", INTRINSIC_LE),
     minit ("LE", INTRINSIC_LE_OS),
     minit ("NOT", INTRINSIC_NOT),
     minit ("PARENTHESES", INTRINSIC_PARENTHESES),
Comment 21 Francois-Xavier Coudert 2007-10-31 14:04:37 UTC
(In reply to comment #20)
> Alternative patch (bootstraps/regtests).

I think it's better to go that way: apparently, care has been taken until now to keep module files alphanumeric, let's keep it that way.

If your patch regetests fine, it's pre-approved.
Comment 22 Tobias Burnus 2007-10-31 15:10:43 UTC
Subject: Bug 33941

Author: burnus
Date: Wed Oct 31 15:10:12 2007
New Revision: 129801

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129801
Log:
2007-10-31  Tobias Burnus  <burnus@net-b.de>

        PR fortran/33941
        * modules.c (intrinsics): Use only alphabetic names for
        intrinsic operators.


2007-10-31  Dominique d'Humieres  <dominiq@lps.ens.fr>
            Tobias Burnus  <burnus@net-b.de>

        PR fortran/33941
        * gfortran.dg/module_read_1.f90: New.


Added:
    trunk/gcc/testsuite/gfortran.dg/module_read_1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/module.c
    trunk/gcc/testsuite/ChangeLog

Comment 23 Tobias Burnus 2007-10-31 15:11:46 UTC
> > Alternative patch (bootstraps/regtests).
> I think it's better to go that way: apparently, care has been taken until now
> to keep module files alphanumeric, let's keep it that way.
> If your patch regetests fine, it's pre-approved.

I checked in now in.

FIXED for the trunk (4.3.0).