| Summary: | [4.2 only] silent data corruption in gfortran read statement | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Russell O'Connor <roconnor> |
| Component: | libfortran | Assignee: | Jerry DeLisle <jvdelisle2> |
| Status: | RESOLVED FIXED | ||
| Severity: | critical | CC: | fxcoudert, gcc-bugs, jvdelisle2, mark |
| Priority: | P3 | Keywords: | patch, wrong-code |
| Version: | 4.1.2 | ||
| Target Milestone: | --- | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | 4.1.3 4.2.0 4.3.0 | Last reconfirmed: | 2007-05-10 01:07:10 |
| Bug Depends on: | |||
| Bug Blocks: | 19292 | ||
Confirmed on i686-linux, for all active branches. (In reply to comment #1) > Confirmed on i686-linux, for all active branches. > I'm not sure how you can confirm this. The program is invalid. a,b,c, and e are undefined in the write statement, so gfortran can do whateve it wants. (In reply to comment #2) > (In reply to comment #1) > > Confirmed on i686-linux, for all active branches. > > > > I'm not sure how you can confirm this. The program is invalid. > a,b,c, and e are undefined in the write statement, so gfortran > can do whateve it wants. > This also exhibits the bug: program r3 integer*4 a(1025),b(1025),c(1025),d(2048),e(1022) a = 0 b = 0 c = 0 e = 0 do i=1,2048 d(i)=i end do open (3,file='a',form='unformatted') write (3) a,b,c,d,e rewind 3 read (3) a,b,c,d close (3) print *,d(1) end I am regression testing the patch. Subject: Bug 31880 Author: jvdelisle Date: Thu May 10 01:01:27 2007 New Revision: 124588 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124588 Log: 2007-05-09 Jerry DeLisle <jvdelisle@gcc.gnu.org> PR libfortran/31880 * io/unix.c (fd_alloc_r_at): Fix calculation of physical offset. Modified: trunk/libgfortran/ChangeLog trunk/libgfortran/io/unix.c Subject: Bug 31880 Author: jvdelisle Date: Thu May 10 01:09:57 2007 New Revision: 124590 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124590 Log: 2007-05-09 Jerry DeLisle <jvdelisle@gcc.gnu.org> PR libfortran/31880 * gfortran.dg/unf_read_corrupted_2.f90: New test. Added: trunk/gcc/testsuite/gfortran.dg/unf_read_corrupted_2.f90 Modified: trunk/gcc/testsuite/ChangeLog Russel, thanks for finding this and reporting it. I assume you do not have copyright assignment. I committed the patch because it is very small and simple. Thanks much! Subject: Bug 31880 Author: jvdelisle Date: Thu May 10 03:22:40 2007 New Revision: 124591 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124591 Log: 2007-05-09 Jerry DeLisle <jvdelisle@gcc.gnu.org> PR libfortran/31880 * gfortran.dg/unf_read_corrupted_2.f90: Fix test. Modified: trunk/gcc/testsuite/gfortran.dg/unf_read_corrupted_2.f90 (In reply to comment #7) > Russel, thanks for finding this and reporting it. I assume you do not have > copyright assignment. I committed the patch because it is very small and > simple. > You're welcome. Thank you for the prompt response. You're right about the copyright assignment. But even with the test case, it comes in under the FSF's 15-line threshold for legally significant. Subject: Bug number PR31880 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-05/msg00728.html Subject: Bug 31880 Author: jvdelisle Date: Fri May 11 00:34:41 2007 New Revision: 124609 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124609 Log: 2007-05-10 Jerry DeLisle <jvdelisle@gcc.gnu.org> PR libfortran/31880 * io/unix.c (fd_alloc_r_at): Fix calculation of physical offset. 2007-05-10 Jerry DeLisle <jvdelisle@gcc.gnu.org> PR libfortran/31880 * gfortran.dg/unf_read_corrupted_2.f90: New test. Added: branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/unf_read_corrupted_2.f90 Modified: branches/gcc-4_1-branch/gcc/testsuite/ChangeLog branches/gcc-4_1-branch/libgfortran/ChangeLog branches/gcc-4_1-branch/libgfortran/io/unix.c This is a regression WRT g77. Mark, this bug is critical - silent data corruption upon read is really, really bad. I second Jerry's DeLisle's request for inclusion in 4.2. This is a regression versus g77, the fix is simple and safe. Subject: Re: [4.2 only] silent data corruption in gfortran
read statement
tkoenig at gcc dot gnu dot org wrote:
> I second Jerry's DeLisle's request for inclusion in 4.2. This is a
> regression versus g77, the fix is simple and safe.
OK, please apply the patch to 4.2.
However, please note for future that a patch should never go on 4.1
before it has gone on 4.2: we want to be sure that new regressions are
not being introduced!
Subject: Bug 31880 Author: jvdelisle Date: Fri May 11 21:53:52 2007 New Revision: 124626 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124626 Log: 2007-05-11 Jerry DeLisle <jvdelisle@gcc.gnu.org> PR libfortran/31880 * io/unix.c (fd_alloc_r_at): Fix calculation of physical offset. Modified: branches/gcc-4_2-branch/libgfortran/ChangeLog branches/gcc-4_2-branch/libgfortran/io/unix.c Subject: Bug 31880 Author: jvdelisle Date: Fri May 11 21:57:05 2007 New Revision: 124627 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124627 Log: 2007-05-11 Jerry DeLisle <jvdelisle@gcc.gnu.org> PR libfortran/31880 * gfortran.dg/unf_read_corrupted_2.f90: New test. Added: branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/unf_read_corrupted_2.f90 Modified: branches/gcc-4_2-branch/gcc/testsuite/ChangeLog Fixed on 4.2, Closing. Sorry for the premature commit on 4.1. |
This program should print out "1". Instead, it prints out "1024": program r3 integer*4 a(1025),b(1025),c(1025),d(2048),e(1022) do i=1,2048 d(i)=i end do open (3,file='a',form='unformatted') write (3) a,b,c,d,e rewind 3 read (3) a,b,c,d close (3) print *,d(1) end output of /opt/bin/gfortran -v: Using built-in specs. Target: x86_64-unknown-linux-gnu Configured with: ../gcc-4.1.2/configure --prefix=/opt --disable-multilib Thread model: posix gcc version 4.1.2 It was compiled with: /opt/bin/gfortran -Wall -o r3 r3.f and run with: LD_LIBRARY_PATH=/opt/lib64 ./r3 This patch fixes it: --- gcc-4.1-4.1.2/gcc-4.1.2/libgfortran/io/unix.c 2006-05-29 22:51:26.000000000 -0400 +++ gcc-4.1-4.1.2_patched/gcc-4.1.2/libgfortran/io/unix.c 2007-05-08 19:22:09.000000000 -0400 @@ -465,7 +465,7 @@ if (n < 0) return NULL; - s->physical_offset = where + n; + s->physical_offset = m + n; s->active += n; } else @@ -476,7 +476,7 @@ if (do_read (s, s->buffer + s->active, &n) != 0) return NULL; - s->physical_offset = where + n; + s->physical_offset = m + n; s->active += n; } The problem is that s->physical_offset is being miscalculated as where+n, which is the requested read start address plus the bytes read from the disk to fill the buffer. It should be where+s->active+n (=m+n), the requested read start address plus the as-yet-unread bytes still in the buffer plus the bytes read from the disk to fill the buffer. The s->logical_offset is correctly set to where+*len. What happens is that if an unbuffered (<8192 bytes) read of more than half the buffer (array b in the example) exceeds the data remaining in the buffer, the remaining data will get moved to the beginning of the buffer, and some amount of data will be read from disk to fill the buffer. If the next read (array c) is of the same length, it will also exceed the data remaining in the buffer, the remaining data will get moved to the beginning of the buffer, and this time the length of the data read from the disk (n) will be the same as the requested read length (*len). Since the physical_offset is being set to where+n and s->logical_offset is set to where+*len, they are equal. If the next read is >=8192 bytes (array d), it will be unbuffered, which would normally cause the data remaining in the buffer to be ignored and cause a seek to re-read that data along with the rest of the request, from the disk. But the code will decide that it doesn't have to seek because physical_offset=logical_offset. So the remaining data in the buffer will be lost.