Bug 26136 - List directed input with underfilled (logicals) array read incorrectly
Summary: List directed input with underfilled (logicals) array read incorrectly
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libfortran (show other bugs)
Version: 4.2.0
: P3 normal
Target Milestone: 4.1.1
Assignee: Jerry DeLisle
URL:
Keywords: wrong-code
Depends on: 957
Blocks: 15502 19292
  Show dependency treegraph
 
Reported: 2006-02-06 19:35 UTC by H.J. Lu
Modified: 2006-03-03 06:04 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-02-06 20:27:42


Attachments
A testcase (794 bytes, application/octet-stream)
2006-02-06 19:36 UTC, H.J. Lu
Details
Updated patch. (1.99 KB, patch)
2006-02-18 07:22 UTC, Jerry DeLisle
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2006-02-06 19:35:19 UTC
namelist doesn't work correctly
Comment 1 H.J. Lu 2006-02-06 19:36:16 UTC
Created attachment 10790 [details]
A testcase

I got

/export/build/gnu/gcc/build-x86_64-linux/gcc/gfortran -B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -O2 -o foo foo.f90
./foo
make: *** [all] Aborted
[hjl@gnu-16 namelist]$
Comment 2 Andrew Pinski 2006-02-06 20:27:42 UTC
Confirmed.
Simple example:
!{ dg-do run }
! Tests filling variables from a namelist read when object list is
! not complete.
program pr

   implicit none
   integer, parameter :: max_domains = 4
   integer ier
   logical, dimension(max_domains) :: non_hydrostatic
   integer, dimension(max_domains) :: time_step_sound
   namelist /dynamics/ non_hydrostatic
   namelist /dynamics/ time_step_sound

   non_hydrostatic = .true.

   read (5, nml=dynamics, iostat=ier, err = 1000)
   print *, non_hydrostatic
   print *, time_step_sound

   stop
1000 call abort()
end program pr
----
cat a
 &dynamics
 non_hydrostatic                     = .true., .true., .true
 time_step_sound = 4,      4,      4,
 /
--------
./a.out < a
namelist read: missplaced = sign
Cannot match namelist object name 4,
Cannot match namelist object name 4,
Cannot match namelist object name 4,

 T T T T
       19036 -1881089148 -1073742864  1140850722

So what is happening is that we don't recognize the start of the differrent namelist object.
Comment 3 H.J. Lu 2006-02-06 21:39:15 UTC
The problem is we have

   logical, dimension(max_domains) :: non_hydrostatic
   integer, dimension(max_domains) :: time_step_sound
   namelist /dynamics/ non_hydrostatic
   namelist /dynamics/ time_step_sound

The input is

 &dynamics
 non_hydrostatic                     = .true.,
 time_step_sound                     = 4,      4,      4,
 /

read_logical () thinks the first 't' in time_step_sound is logical TRUE.
Comment 4 Andrew Pinski 2006-02-06 21:44:48 UTC
The bug is in read_logical.  We eat the 't' and try to find the next seperator.
Comment 5 Andrew Pinski 2006-02-06 22:25:12 UTC
g77 had the same bug until PR 957 was fixed.
Comment 6 Andrew Pinski 2006-02-06 22:34:08 UTC
This was the patch to libi77 which fixed the problem for g77:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libf2c/libI77/Attic/lread.c.diff?r1=1.4&r2=1.5&only_with_tag=MAIN&f=h
Comment 7 Andrew Pinski 2006-02-06 22:43:45 UTC
Does this code show up in SPEC CPU 2006?
Comment 8 H.J. Lu 2006-02-06 23:47:20 UTC
This is in SPEC CPU 2006. But it needs a patch to compile with gcc 4.2. Since
I can't verify if gcc 4.2 compiles this benchmark correctly, I am reluctant
to sumbit my patch.
Comment 9 Jerry DeLisle 2006-02-07 02:42:48 UTC
This is pr 24459, see discussion there.  
Comment 10 Andrew Pinski 2006-02-07 02:50:14 UTC
(In reply to comment #9)
> This is pr 24459, see discussion there.  

Jerry, I would like to disagree based on the the old bug g77 bug I found which shows really what is going on here and why this is not really a dup of bug 24459. 
Comment 11 Jerry DeLisle 2006-02-07 04:15:06 UTC
(In reply to comment #10)
Yes, After looking at this more closely I agree this is not a dup. First letter f or t of the next name object is being interpreted as the next logical value in the incomplete array.
Comment 12 H.J. Lu 2006-02-12 01:31:07 UTC
It looks like that unget_char needs to modified to increase the supported number
of unget. The current number is 1. We can't do

  unget_char (dtp, c);
  unget_char (dtp, c);
  unget_char (dtp, c);

We can have an unget buffer in st_parameter_dt and change last_char to a pointer
which indexes to the unget buffer. The unget buffer only needs big enough to
hold the longest logical string we want to support.
Comment 13 Jerry DeLisle 2006-02-12 06:36:07 UTC
Yes, I am working on a scheme that will provide deeper ungets when needed.  Half way there now.
Comment 14 Jerry DeLisle 2006-02-17 00:07:53 UTC
A patch is testing and I am working up some additional test cases.  The test case in this PR is working.
Comment 15 patchapp@dberlin.org 2006-02-17 04:33:20 UTC
Subject: Bug number PR26136

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/2006-02/msg01387.html
Comment 16 H.J. Lu 2006-02-17 18:42:28 UTC
The patch

http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01387.html

is incomplete:

[hjl@gnu-16 namelist-3]$ cat namelist.input.1
 &dynamics
 non_hydrostatic                     = .true.,
 time_step_sound                     = 4,      4,      4,
 /
[hjl@gnu-16 namelist-3]$ ./foo < namelist.input.1
Cannot match namelist object name 4,
Cannot match namelist object name 4,

Aborted
[hjl@gnu-16 namelist-3]$ cat namelist.input.2
 &dynamics
 non_hydrostatic                     = .true., .true.,
 time_step_sound                     = 4,      4,      4,
 /
[hjl@gnu-16 namelist-3]$ ./foo < namelist.input.2
Cannot match namelist object name 4,
Cannot match namelist object name 4,
Cannot match namelist object name 4,

 T T T T
 -1073743496         127 -1073743512         127
[hjl@gnu-16 namelist-3]$ cat namelist.input.3
 &dynamics
 non_hydrostatic                     = .true.,.true., .true.,
 time_step_sound                     = 4,      4,      4,
 /
[hjl@gnu-16 namelist-3]$ ./foo < namelist.input.3
 T T T T
           4           4           4         127
[hjl@gnu-16 namelist-3]$
Comment 17 Jerry DeLisle 2006-02-18 05:15:34 UTC
Indeed you have found something I missed.  I am on it.  BTW Your namelist.input.3 is correct, keeping in mind that gfortran does not initialize variables and the result of an incomplete array is null value, meaning it does nothing.

Thanks for testing, it helps.
Comment 18 Jerry DeLisle 2006-02-18 07:22:41 UTC
Created attachment 10871 [details]
Updated patch.

Updated patch to fix problem found in Comment #16
Comment 19 Jerry DeLisle 2006-02-18 07:54:31 UTC
Just so everyone understands.  I am working one more modification to this patch to allow 63 character long names in the tests for logical true or false in namelist reads.  The patch in Comment #18 works for all cases found so far, but is limited to 31 character variable name tests.  Testing and expanded test cases in the queue.
Comment 20 Jerry DeLisle 2006-02-19 20:31:16 UTC
Final patch waiting for review/approval:

http://gcc.gnu.org/ml/fortran/2006-02/msg00421.html
Comment 21 Jerry DeLisle 2006-03-01 06:04:49 UTC
Subject: Bug 26136

Author: jvdelisle
Date: Wed Mar  1 06:04:45 2006
New Revision: 111597

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=111597
Log:
2006-02-28  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/26136
	* io/io.h: Add flag for reading from line_buffer.
	* io/list_read.c (l_push_char): New function to save namelist
	input when reading logicals.
	(free_line): New function to free line_buffer memory.
	(next_char): Added feature to read from line_buffer.
	(read_logical): Use new functions to test for '=' after reading a
	logical value, checking for possible variable name.
	(namelist_read): Use free_line when all done.

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

Comment 22 Jerry DeLisle 2006-03-01 06:14:35 UTC
Subject: Bug 26136

Author: jvdelisle
Date: Wed Mar  1 06:14:32 2006
New Revision: 111598

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=111598
Log:
2006-02-28  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/26136
	* gfortran.dg/namelist_23.f90: New test.

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

Comment 23 Jerry DeLisle 2006-03-01 06:50:16 UTC
Fixed on 4.2. Pending 4.1.1
Comment 24 Jerry DeLisle 2006-03-03 06:00:11 UTC
Subject: Bug 26136

Author: jvdelisle
Date: Fri Mar  3 06:00:08 2006
New Revision: 111672

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

	PR libgfortran/26136
	* io/io.h: Add flag for reading from line_buffer.
	* io/list_read.c (l_push_char): New function to save namelist
	input when reading logicals.
	(free_line): New function to free line_buffer memory.
	(next_char): Added feature to read from line_buffer.
	(read_logical): Use new functions to test for '=' after reading a
	logical value, checking for possible variable name.
	(namelist_read): Use free_line when all done.

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

Comment 25 Jerry DeLisle 2006-03-03 06:03:05 UTC
Subject: Bug 26136

Author: jvdelisle
Date: Fri Mar  3 06:03:01 2006
New Revision: 111673

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

	PR libgfortran/26136
	* gfortran.dg/namelist_23.f90: New test.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/namelist_23.f90
Modified:
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Comment 26 Jerry DeLisle 2006-03-03 06:04:47 UTC
Fixed on 4.1.1