[Bug libfortran/59419] New: [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[Bug libfortran/59419] New: [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

            Bug ID: 59419
           Summary: [4.9 Regression] Failing OPEN with FILE='xxx' and
                    IOSTAT creates the file 'xxx' after revision 196783
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libfortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dominiq at lps dot ens.fr
                CC: jb at gcc dot gnu.org, tilo@tilo-schwarz.de

While the test 

open(unit=-10, file='xxx')
end

gives an error at run time without creating the file 'xxx'. The test

integer :: i
open(unit=-10, file='xxx', iostat=i)
print *, i
end

prints 5002 at run time and create an empty file 'xxx'. This does not happens
with r196783, but does with r196819.

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Technically, one has:
          u = find_unit (opp->common.unit);
          if (u == NULL) /* Negative unit and no NEWUNIT-created unit found. 
*/
            generate_error (&opp->common, LIBERROR_BAD_OPTION,
                            "Bad unit number in OPEN statement");
without err=/stat= it prints an error and doesn't return. But with, it
continues after that line. Hence, a "return" is needed. It probably also
affects other code in the I/O library.

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-12-07
                 CC|                            |jvdelisle at gcc dot gnu.org
   Target Milestone|---                         |4.9.0
     Ever confirmed|0                           |1

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Dominique, probably rightly, suspects r196805.

(In reply to Tobias Burnus from comment #1)
>             generate_error (&opp->common, LIBERROR_BAD_OPTION,
>                             "Bad unit number in OPEN statement");
> without err=/stat= it prints an error and doesn't return. But with, it
> continues after that line. Hence, a "return" is needed.

... but not a simple return but one which ends with a library_end().

Other code in that file uses:
  && (opp->common.flags & IOPARM_LIBRETURN_MASK) == IOPARM_LIBRETURN_OK)
as condition. Hence, that would be another possibility.

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[jvdelisle at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

--- Comment #3 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
I will take this one if you like.

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

--- Comment #4 from Janne Blomqvist <jb at gcc dot gnu.org> ---
Indeed I suspect we have many cases in the IO library where stuff fails, leaks
memory, or leaves stuff in an inconsistent state when IOSTAT= is present. 

I think that we should modify generate_error() to not terminate the program in
case IOSTAT= is not present, but instead we return and unwind the IO call stack
(manually with return statements since this is plain old C), and then only at
the end just before we return to the user Fortran program we check
IOPARM_HAS_IOSTAT and IOPARM_LIBRETURN_MASK and exit() if appropriate. That way
error handling works the same whether IOSTAT= is present or not. But such a
change is probably 4.10 material.

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[jvdelisle at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

--- Comment #6 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Patch submitted for review.

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[jvdelisle at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

--- Comment #7 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Author: jvdelisle
Date: Tue Dec 17 03:06:04 2013
New Revision: 206039

URL: http://gcc.gnu.org/viewcvs?rev=206039&root=gcc&view=rev
Log:
2013-12-16  Jerry DeLisle  <jvdelisle@gcc.gnu>

    PR libfortran/59419
    * io/file_pos.c (st_rewind): Do proper return after
    generate_error.
    * io/open.c (edit_modes): Move action code inside block that
    checks for library ok. (new_unit): Do cleanup after error.
    (st_open): Do proper return after error.
    * io/transfer.c (data_transfer_init): Likewise.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/file_pos.c
    trunk/libgfortran/io/open.c
    trunk/libgfortran/io/transfer.c

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

--- Comment #8 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
The modified test gfortran.dg/open_negative_unit_1.f90 in the patch submitted
at http://gcc.gnu.org/ml/fortran/2013-12/msg00124.html checks that this PR is
fixed.

[Bug libfortran/59419] [4.9 Regression] Failing OPEN with FILE='xxx' and IOSTAT creates the file 'xxx' after revision 196783

[jvdelisle at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59419

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Fixed on 4.9.