Message ID | eb5c03da-e195-78fd-6cac-01cb625f9092@charter.net |
---|---|
State | New |
Headers | show |
On Mon, May 15, 2017 at 01:10:43PM -0700, Jerry DeLisle wrote: > Hi all, > > Crash is a misnomer on this PR [aside: People see the backtrace and assume] > > This patch fixes the problem by correctly detecting the EOR condition for > internal units. The previous check in read_sf_internal was wrong, relying > probably on uninitialized memory as can be seen by the still open PR78881. > Removing the bad hunk fixes the regression here and the new code lets > dtio_26.f90 pass as expected. > > Regression tested on x86_64. New test case will be added. > > OK for trunk? Will back port in a few days to 7. > No. There are a number of other failures with your patch applied. Running /home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/dg.exp ... FAIL: gfortran.dg/read_3.f90 -O0 (test for excess errors) FAIL: gfortran.dg/read_3.f90 -O1 (test for excess errors) FAIL: gfortran.dg/read_3.f90 -O2 (test for excess errors) FAIL: gfortran.dg/read_3.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) FAIL: gfortran.dg/read_3.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/read_3.f90 -Os (test for excess errors) === gfortran Summary === # of expected passes 6 # of unexpected failures 6 /mnt/sgk/objx/gcc/gfortran version 8.0.0 20170515 (experimental) (GCC)
On Mon, May 15, 2017 at 01:33:55PM -0700, Steve Kargl wrote: > On Mon, May 15, 2017 at 01:10:43PM -0700, Jerry DeLisle wrote: > > Hi all, > > > > Crash is a misnomer on this PR [aside: People see the backtrace and assume] > > > > This patch fixes the problem by correctly detecting the EOR condition for > > internal units. The previous check in read_sf_internal was wrong, relying > > probably on uninitialized memory as can be seen by the still open PR78881. > > Removing the bad hunk fixes the regression here and the new code lets > > dtio_26.f90 pass as expected. > > > > Regression tested on x86_64. New test case will be added. > > > > OK for trunk? Will back port in a few days to 7. > > > > No. There are a number of other failures with your patch applied. > > Running /home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/dg.exp ... > FAIL: gfortran.dg/read_3.f90 -O0 (test for excess errors) > FAIL: gfortran.dg/read_3.f90 -O1 (test for excess errors) > FAIL: gfortran.dg/read_3.f90 -O2 (test for excess errors) > FAIL: gfortran.dg/read_3.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) > FAIL: gfortran.dg/read_3.f90 -O3 -g (test for excess errors) > FAIL: gfortran.dg/read_3.f90 -Os (test for excess errors) > The failures are from an unexpected warning. Executing on host: /mnt/sgk/objx/gcc/testsuite/gfortran2/../../gfortran -B/mnt/sgk/objx/gcc/testsuite/gfortran2/../../ -B/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/ /home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/read_3.f90 -fno-diagnostics-show-caret -fdiagnostics-color=never -O0 -pedantic-errors -B/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libatomic/.libs -B/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libquadmath/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libquadmath/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libquadmath/.libs -lm -o ./read_3.exe (timeout = 300) spawn -ignore SIGHUP /mnt/sgk/objx/gcc/testsuite/gfortran2/../../gfortran -B/mnt/sgk/objx/gcc/testsuite/gfortran2/../../ -B/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/ /home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/read_3.f90 -fno-diagnostics-show-caret -fdiagnostics-color=never -O0 -pedantic-errors -B/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libgfortran/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libatomic/.libs -B/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libquadmath/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libquadmath/.libs -L/mnt/sgk/objx/x86_64-unknown-freebsd12.0/./libquadmath/.libs -lm -o ./read_3.exe /home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/read_3.f90:6:11: Warning: GNU Extension: Nonstandard type declaration INTEGER*4 at (1) FAIL: gfortran.dg/read_3.f90 -O0 (test for excess errors) Excess errors: /home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/read_3.f90:6:11: Warning: GNU Extension: Nonstandard type declaration INTEGER*4 at (1)
On Mon, May 15, 2017 at 01:10:43PM -0700, Jerry DeLisle wrote: > > 2017-05-15 Jerry DeLisle <jvdelisle@gcc.gnu.org> > > PR libgfortran/80727 > * transfer.c (read_sf_internal): Remove bogus code to detect EOR. > (read_block_form): For internal units, generate EOR if no more > bytes left in unit and we are trying to read with ADVANCE='NO'. OK with change below. > > ! { dg-do run } > ! PR80727 Crash of runtime gfortran library during integer transformation > ! Note: before the patch this was giving an incorrect EOR error on READ. > program gfortran_710_io_bug > character str*4 > integer*4 i4 integer(4) > str ='' > i = 256 > write(str,fmt='(a)') i > i = 0 > read ( unit=str(1:4), fmt='(a)' ) i4 > if (i4.ne.256) call abort > end program gfortran_710_io_bug
On 05/15/2017 03:51 PM, Steve Kargl wrote: > On Mon, May 15, 2017 at 01:10:43PM -0700, Jerry DeLisle wrote: >> >> 2017-05-15 Jerry DeLisle <jvdelisle@gcc.gnu.org> >> >> PR libgfortran/80727 >> * transfer.c (read_sf_internal): Remove bogus code to detect EOR. >> (read_block_form): For internal units, generate EOR if no more >> bytes left in unit and we are trying to read with ADVANCE='NO'. > > OK with change below. > >> >> ! { dg-do run } >> ! PR80727 Crash of runtime gfortran library during integer transformation >> ! Note: before the patch this was giving an incorrect EOR error on READ. >> program gfortran_710_io_bug >> character str*4 >> integer*4 i4 > > integer(4) > >> str ='' >> i = 256 >> write(str,fmt='(a)') i >> i = 0 >> read ( unit=str(1:4), fmt='(a)' ) i4 >> if (i4.ne.256) call abort >> end program gfortran_710_io_bug > Oh, thanks Steve. I forgot to run the new test with the machinery. Will fix, check it one more time, then commit. Jerry
diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c index f16d8c55..928a448f 100644 --- a/libgfortran/io/transfer.c +++ b/libgfortran/io/transfer.c @@ -272,12 +272,6 @@ read_sf_internal (st_parameter_dt *dtp, int *length) return NULL; } - if (base && *base == 0) - { - generate_error (&dtp->common, LIBERROR_EOR, NULL); - return NULL; - } - dtp->u.p.current_unit->bytes_left -= *length; if (((dtp->common.flags & IOPARM_DT_HAS_SIZE) != 0) || @@ -470,11 +464,24 @@ read_block_form (st_parameter_dt *dtp, int *nbytes) } } - if (unlikely (dtp->u.p.current_unit->bytes_left == 0 - && !is_internal_unit(dtp))) + if (is_internal_unit(dtp)) { - hit_eof (dtp); - return NULL; + if (*nbytes > 0 && dtp->u.p.current_unit->bytes_left == 0) + { + if (dtp->u.p.advance_status == ADVANCE_NO) + { + generate_error (&dtp->common, LIBERROR_EOR, NULL); + return NULL; + } + } + } + else + { + if (unlikely (dtp->u.p.current_unit->bytes_left == 0)) + { + hit_eof (dtp); + return NULL; + } } *nbytes = dtp->u.p.current_unit->bytes_left;