Bug 35627 - [4.3 regression] namelist read error
Summary: [4.3 regression] namelist read error
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.3.0
: P4 normal
Target Milestone: 4.3.1
Assignee: Jerry DeLisle
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2008-03-18 16:01 UTC by Alexander Pletzer
Modified: 2008-03-21 03:13 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.2.2 4.1.3 4.4.0
Known to fail: 4.3.0
Last reconfirmed: 2008-03-18 23:57:49


Attachments
A final patch (588 bytes, patch)
2008-03-20 01:17 UTC, Jerry DeLisle
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pletzer 2008-03-18 16:01:48 UTC
Hi: The following program fails to read the namelist file below.

test program:
program test
  implicit none
    LOGICAL :: nlco(200)  ! (1:nbeam)
    REAL*8 :: xlbtna(200)  ! (1:nbeam)
  NAMELIST/nbdrive_naml/ nlco,xlbtna
    INTEGER :: nbshapa(200)  ! (1:nbeam)
  NAMELIST/nbdrive_naml/ nbshapa
  open(10, file='t.nml')
  read(10, nbdrive_naml)
  write(*,nbdrive_naml)
  close(10)
end program test

file t.nml:
&nbdrive_naml
nlco = 4*.TRUE.
xlbtna = 802.8, 802.8, 802.8, 802.8
nbshapa = 4*1
/

error message:
[pletzer@quartic namelist2]$ /contrib/gcc-4.3/bin/gfortran t.f90
[pletzer@quartic namelist2]$ ./a.out
At line 9 of file t.f90 (unit = 10, file = 't.nml')
Fortran runtime error: Bad data for namelist object xlbtna

Thanks for your help.
Comment 1 Tobias Burnus 2008-03-18 21:50:59 UTC
Confirmed. Again a GCC 4.3.0/4.4.0 regression.

Seemingly caused between 2007-12-07-r130671 and 2007-12-10-r130736. Probably caused by my NaN/INF patch (PR fortran/34319 - or, less likely, fortran/34404).
Comment 2 Tobias Burnus 2008-03-18 22:02:13 UTC
The problem is that we don't properly rewind properly for "nbshapa"; the parser sees 'n'; it then try whether "NaN" matches. It checks the next character 'b' and fails. The problem is that only 'b' and not 'nb' are put back:

    } /* Match NaN. Up to here, c contains either 'N' or 'n'. */
  else if (((c = next_char (dtp)) == 'a' || c == 'A')
           && ((c = next_char (dtp)) == 'n' || c == 'N')
           && (c = next_char (dtp)))
    { ... }
 bad:
  if (nml_bad_return (dtp, c))
    return 0;
Comment 3 Jerry DeLisle 2008-03-18 23:57:49 UTC
Namelist bugs are just a tradition. :) Assigning myself.
Comment 4 Toon Moene 2008-03-19 08:37:25 UTC
I'm hit by this, too - don't know when it started (it's in a namelist I've been using for years).
Comment 5 Jerry DeLisle 2008-03-19 12:35:17 UTC
I believe the problem is actual;y in read_logical and occurs on a short read.  By short, I mean the case where we provide fewer data items in the namelist file then there are in the logical array.  I am curious what has triggered this.
Comment 6 Tobias Burnus 2008-03-19 15:54:31 UTC
(In reply to comment #5)
> I believe the problem is actual;y in read_logical and occurs on a short read. 
> By short, I mean the case where we provide fewer data items in the namelist
> file then there are in the logical array.  I am curious what has triggered
> this.

I think it has something to do with read_real, though different from what I thought. In any case, we end up in read_real, trying to match "NaN" and go to unwind. However, afterwards it shows an error for "xlbtna" (for which read_real is called) and not for the item that comes next and is not NaN, namely "nbshapa".

Just before the return, line_buffer contains in "unwind:" of read_real:
  line_buffer[0] == '\0'
  line_buffer[1] == 'n'
  line_buffer[2] == 'b'

The problem is that the first element is '\0' and thus the rewinding does not happen. If one changes "nbshapa" into "nnnbshapa" it works. Thus the question is : Where does the "\0" come from?
Comment 7 Jerry DeLisle 2008-03-19 17:55:21 UTC
What I observed is that if you change the namelist file to nlco = 200*.TRUE. the subsequent real reads work fine.  Likewise if you move the logical to the end after the real reads, it works fine. So this is why I suspect the read_logical may have an off by one leftover in line_buffer or we are not cleaning up correctly on that.  This may be where the spurious \0 is coming from.  I will have more time to look into it in the next few days. 
Comment 8 Tobias Burnus 2008-03-19 18:29:48 UTC
> So this is why I suspect the read_logical may have an off by one leftover
I think you are right. In that sense it is an only bug which was only exposed by the new NaN support.

Not very elegent, but the following works. One should probably do an audit of all l_push_char. The following works here, but I do not understand why element [0] was '\0' and not something else.

Index: libgfortran/io/list_read.c
===================================================================
--- libgfortran/io/list_read.c  (Revision 133342)
+++ libgfortran/io/list_read.c  (Arbeitskopie)
@@ -680,6 +680,7 @@ read_logical (st_parameter_dt *dtp, int
       break;

     CASE_SEPARATORS:
+      dtp->u.p.item_count--;
       unget_char (dtp, c);
       eat_separator (dtp);
       return;                  /* Null value.  */
@@ -741,6 +742,7 @@ read_logical (st_parameter_dt *dtp, int

  bad_logical:

+  dtp->u.p.item_count--;
   free_line (dtp);

   if (nml_bad_return (dtp, c))
Comment 9 Jerry DeLisle 2008-03-19 19:37:01 UTC
I agree and plan to look at l_push_char.  We just should never read a "\0" character.  We may be ungetting it because we use "\0" to indicate that the line_buffer is empty and to not read from it, but maybe unget_char is not quite right.
Comment 10 Jerry DeLisle 2008-03-20 01:17:16 UTC
Created attachment 15348 [details]
A final patch

l_push_char assumes the index which is item_count has been set to zero and after saving a character it bumps the index.  This bug has been latent for a long time, Forgot to set item_count back to zero when when freeing the line_buffer.
 
The simplest fix is this:

@@ -741,6 +742,7 @@ read_logical (st_parameter_dt *dtp, int

  bad_logical:

+  dtp->u.p.item_count = 0;
   free_line (dtp);

   if (nml_bad_return (dtp, c))

However, I am going to move this reseting into free_line where it ought to be.  The attachment is the final fix with some cleanup.  Regression tested on x86-64.  I will commit as obvious with the new test case.
Comment 11 Jerry DeLisle 2008-03-20 02:05:51 UTC
Subject: Bug 35627

Author: jvdelisle
Date: Thu Mar 20 02:05:05 2008
New Revision: 133360

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133360
Log:
2008-03-19  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/35627
	* io/list_read.c (free_line): Clear the line buffer enable flag and
	reset the index into line_buffer, aka item_count.
	(next_char): Cleanup whitespace.
	(read_logical): Use unget_char to assure that the first character of the
	bad logical is saved in case it is part of an object name. Remove the
	clearing of index and flag that is now in free_line.
	(read_real): Likewise.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/list_read.c

Comment 12 Jerry DeLisle 2008-03-20 02:08:22 UTC
Subject: Bug 35627

Author: jvdelisle
Date: Thu Mar 20 02:07:38 2008
New Revision: 133361

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133361
Log:
2008-03-19  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/35627
	* gfortran.dg/namelist_46.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/namelist_46.f90
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 13 Jerry DeLisle 2008-03-20 02:09:29 UTC
Fixed on trunk.  Will backport to 4.3 in a day or so.
Comment 14 Jerry DeLisle 2008-03-21 01:30:17 UTC
Subject: Bug 35627

Author: jvdelisle
Date: Fri Mar 21 01:29:30 2008
New Revision: 133411

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133411
Log:
2008-03-20  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/35627
	Backport from trunk.
	* io/list_read.c (free_line): Clear the line buffer enable flag and
	reset the index into line_buffer, aka item_count.
	(next_char): Cleanup whitespace.
	(read_logical): Use unget_char to assure that the first character of the
	bad logical is saved in case it is part of an object name. Remove the
	clearing of index and flag that is now in free_line.
	(read_real): Likewise.

	PR libfortran/35617
	Backport from trunk.
	* io/list_read.c (eat_separator): If next character after eatline is '!'
	then eatline again. 

Modified:
    branches/gcc-4_3-branch/libgfortran/ChangeLog
    branches/gcc-4_3-branch/libgfortran/io/list_read.c

Comment 15 Jerry DeLisle 2008-03-21 01:38:24 UTC
Subject: Bug 35627

Author: jvdelisle
Date: Fri Mar 21 01:37:38 2008
New Revision: 133413

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133413
Log:
2008-03-20  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	Backport from trunk:
	PR libfortran/35627
	* gfortran.dg/namelist_46.f90: New test.

	PR libfortran/35617
	* gfortran.dg/namelist_45.f90: New test.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/namelist_45.f90
Modified:
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog

Comment 16 Jerry DeLisle 2008-03-21 03:13:42 UTC
I typoed the file name on the commit of namelist_46.f90, but it is there.

Fixed on 4.3.