[Bug fortran/47296] New: I/O Segfault when running out of file descriptors

[Bug fortran/47296] New: I/O Segfault when running out of file descriptors

[burnus at gcc dot gnu.org]

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

           Summary: I/O Segfault when running out of file descriptors
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: burnus@gcc.gnu.org
                CC: jvdelisle@gcc.gnu.org


Found at
http://groups.google.com/group/comp.lang.fortran/browse_thread/thread/8843f2317cb565cd#

On Linux, I need to use either much higher numbers (> 1024) or set the number
of file descriptors down using, e.g.   ulimit -S -n 20

Valgrind then shows:

 Begin assignment unit
  ** try to connect chosen unit =           27
==26968== Warning: invalid file descriptor 25 in syscall open()
==26968== Use of uninitialised value of size 8
==26968==    at 0x4EFC6BF: _gfortrani_new_unit (open.c:509)
==26968==    by 0x4EFC994: _gfortran_st_open (open.c:702)
==26968==    by 0x400BF5: assignunit.1532 (hj4.f90:38)

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[burnus at gcc dot gnu.org]

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

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> 2011-01-14 19:48:26 UTC ---
The problems seems to be that opp->file is not set for scratch files
(STATUS_SCRATCH) when tempfile fails. Thus, open_external just returns.

Then, in new_unit in the error handling block, opp->file_len is used and
unpack_filename tries to read the string. However, accessing opp->file crashes
in 
fstrlen.

It might work if one sets file_len to 0 in tempfile. But maybe something
cleverer can be done.

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2011.01.14 19:52:52
         AssignedTo|unassigned at gcc dot       |jvdelisle at gcc dot
                   |gnu.org                     |gnu.org
     Ever Confirmed|0                           |1

--- Comment #2 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> 2011-01-14 19:52:52 UTC ---
I  will take this one.

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at gcc dot gnu.org]

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

--- Comment #3 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> 2011-01-15 06:33:15 UTC ---
This is easy enough and if you study the code path, it makes sense.  The error
flag returned here is passed up the call chain where an "os" error is
generated. When attempting to open a temporary file, if the file open fails,
the file name in the opp structure is not set, so it is NULL. 

Index: unix.c
===================================================================
--- unix.c    (revision 168773)
+++ unix.c    (working copy)
@@ -1000,6 +1000,8 @@
 int
 unpack_filename (char *cstring, const char *fstring, int len)
 {
+  if (fstring == NULL)
+    return 1;
   len = fstrlen (fstring, len);
   if (len >= PATH_MAX)
     return 1;

The result:

At line 39 of file pr47296.f90 (unit = 27, file = '')
Fortran runtime error: Too many open files

I will commit this as obvious shortly if regression testing passes.  I can not
imagine it causing a problem.

In the meantime, I will see if i can test on a Windows based system, to see if
it works or if there is another issue.

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at gcc dot gnu.org]

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

--- Comment #4 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> 2011-01-15 06:42:33 UTC ---
Author: jvdelisle
Date: Sat Jan 15 06:42:30 2011
New Revision: 168832

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168832
Log:
2011-01-14  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

    PR libgfortran/47296
    * io/unix.c (unpack_filename): Return non-zero if the filename passed
    in is NULL.

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

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jb at gcc dot gnu.org]

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

Janne Blomqvist <jb at gcc dot gnu.org> changed:

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

--- Comment #5 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-01-15 09:18:18 UTC ---
Nice catch.

Looking at the code, when creating a SCRATCH file fails, unix.c:tempfile()
returns -1, then as a result open_external() returns NULL, but then in
open.c:new_unit() even though we have checked that open_external() returns
NULL, we still call unpack_filename(), as well as accessing opp->file_len.

Just to be sure, I suspect it would be prudent to set opp->file to NULL and
opp->file_len to 0 in tempfile() in case fd < 0. Or are we sure that the entire
opp struct is set to 0 sometime before?

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at frontier dot com]

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

--- Comment #6 from jvdelisle at frontier dot com 2011-01-15 14:51:00 UTC ---
> Just to be sure, I suspect it would be prudent to set opp->file to NULL and
> opp->file_len to 0 in tempfile() in case fd<  0. Or are we sure that the entire
> opp struct is set to 0 sometime before?
>

I had the same thought. I believe it is "zeroed" when the opp structure is 
initialized...

at line 449 or so of open.c"

   switch (flags->status)
     {
     case STATUS_SCRATCH:
       if ((opp->common.flags & IOPARM_OPEN_HAS_FILE) == 0)
    {
      opp->file = NULL;
      break;
    }

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at gcc dot gnu.org]

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

--- Comment #7 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> 2011-01-15 18:16:33 UTC ---
Unpatched on Cygwin on Windows 7, I do not get the segfault. I also do not get
the OS error.  The assign error mechanism of the test case picks up that there
is a problem.  The windows version does not give an error and sets the tempfile
name string even though no file is opened.  I am doing a rebuild to latest
trunk on the Win7 box, so when that is done, I will see what mechanism we need
to error out on this.

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jb at gcc dot gnu.org]

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

--- Comment #8 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-01-15 19:31:17 UTC ---
I suppose one thing that could be done is to set opp->file and opp->file_len
regardless of whether mkstemp() succeeds or not, that is lines 1987:1093 in
unix.c, and then in open.c:new_unit() one can free opp->file after generating a
suitable error message. 

Even if the file name is automatically generated, maybe it would be helpful to
provide the user with info exactly what failed, e.g. so that the user knows
what failed was opening a file under /tmp which might otherwise not be obvious
to the user.

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at gcc dot gnu.org]

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

--- Comment #9 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> 2011-01-16 08:29:55 UTC ---
After getting Win7-cygwin build of gcc latest trunk built, I was able to do
some more testing.  I could then reproduce another segfault.  As Janne
suggested and I certainly agree, the following simple patch clears this second
segfault.  It is interesting that the Windows/cygwin execution behaves
differently and the opp->file somewhere is getting touched or not initialized.

Index: unix.c
===================================================================
--- unix.c    (revision 168844)
+++ unix.c    (working copy)
@@ -1085,7 +1085,11 @@
 #endif /* HAVE_MKSTEMP */

   if (fd < 0)
-    free (template);
+    {
+      free (template);
+      opp->file = NULL;
+      opp->file_len = 0;
+    }
   else
     {
       opp->file = template;

I will commit this sometime tomorrow.

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jb at gcc dot gnu.org]

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

--- Comment #10 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-01-16 20:12:55 UTC ---
Well, I was thinking more something like the patch I just posted at

http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01086.html

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at gcc dot gnu.org]

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

--- Comment #11 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> 2011-01-17 05:59:15 UTC ---
No more segfault on Cygwin, it should be good. Thanks.

[Bug fortran/47296] I/O Segfault when running out of file descriptors

[jvdelisle at gcc dot gnu.org]

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

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

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

--- Comment #12 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> 2011-01-22 14:23:45 UTC ---
Patch committed:

Sending        libgfortran/ChangeLog
Sending        libgfortran/io/unix.c
Transmitting file data ..
Committed revision 168888.

Thanks.

Closing