Re: Solaris ftell bug

[Bruno Haible]

Eric Blake wrote:
> As proof that the bug exists independently of gnulib:
> 
> $ cat foo.c
> #include <stdio.h>
> #include <stdio_ext.h>
> int main() {
>   FILE *fp = fopen ("file", "r+");
>   fseek (fp, -1, SEEK_END);
>   if (getc (fp) != 'h') return 1;
>   if (getc (fp) != EOF) return 2;
>   __fpurge (fp);
>   if (putc ('!', fp) != '!') return 3;
>   printf ("%d\n", (int) ftell (fp));
>   if (fclose (fp) != 0) return 4;
>   return 0;
> }
> $ printf foogarsh > file
> $ ./foo
> -8183
> 
> I guess we need to beef up the unit tests to expose the Solaris bug,
> then work on replacing broken __fpurge with a version that works
> correctly on read-write streams.

The bug in Solaris is not in fpurge, bug actually in getc + putc + ftell.

Here's a sample code without fpurge:

=============================== foo.c ================================
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define ASSERT(expr) \
  do                                                                         \
    {                                                                        \
      if (!(expr))                                                           \
        {                                                                    \
          fprintf (stderr, "%s:%d: assertion failed\n",                      \
                   __FILE__, __LINE__);                                      \
          fflush (stderr);                                                   \
          abort ();                                                          \
        }                                                                    \
    }                                                                        \
  while (0)

#define TESTFILE "t-fpurge2.tmp"

int
main (void)
{
  FILE *fp;

  /* Create a file with some contents.  */
  fp = fopen (TESTFILE, "w");
  if (fp == NULL)
    goto skip;
  if (fwrite ("foogarsh", 1, 8, fp) < 8)
    goto skip;
  if (fclose (fp))
    goto skip;

  /* The file's contents is now "foogarsh".  */

  /* Ensure that purging a read does not corrupt subsequent writes.  */
  fp = fopen (TESTFILE, "r+");
  if (fp == NULL)
    goto skip;
  if (fseek (fp, -1, SEEK_END))
    goto skip;
  ASSERT (getc (fp) == 'h');
  ASSERT (getc (fp) == EOF);
  ASSERT (ftell (fp) == 8);
  ASSERT (putc ('!', fp) == '!');
  ASSERT (ftell (fp) == 9);
  ASSERT (fclose (fp) == 0);
  fp = fopen (TESTFILE, "r");
  if (fp == NULL)
    goto skip;
  {
    char buf[10];
    ASSERT (fread (buf, 1, 10, fp) == 9);
    ASSERT (memcmp (buf, "foogarsh!", 9) == 0);
  }
  ASSERT (fclose (fp) == 0);

  /* The file's contents is now "foogarsh!".  */

  remove (TESTFILE);
  return 0;

 skip:
  fprintf (stderr, "Skipping test: prerequisite file operations failed.\n");
  remove (TESTFILE);
  return 77;
}
==========================================================================
$ ./foo
foo.c:46: assertion failed
Abort (core dumped)

POSIX <http://www.opengroup.org/onlinepubs/9699919799/functions/fopen.html>
imposes the restriction that
  "When a file is opened with update mode ('+' as the second or third
   character in the mode argument), both input and output may be performed
   on the associated stream. However, the application shall ensure that
   ... input is not directly followed by output without an intervening
   call to a file positioning function (fseek(), fsetpos(), or rewind()),
   unless the input operation encounters end-of-file."

Here the input operation did encounter end-of-file, and nevertheless the
putc call disturb's ftell's notion of current file position.

Changing the test program by inserting an fflush operation
  ASSERT (getc (fp) == EOF);
  ASSERT (ftell (fp) == 8);
  fflush (fp);
  ASSERT (ftell (fp) == 8);
  ASSERT (putc ('!', fp) == '!');
makes it succeed. Before the fflush, the stream looks like this:
  (gdb) print *fp
  $1 = {_cnt = 0, _ptr = 0x8061654 "hoogarsh", _base = 0x8061654 "hoogarsh", 
    _flag = 153 '\231', _file = 6 '\006', __orientation = 0, __ionolock = 0, 
    __seekable = 1, __extendedfd = 0, __xf_nocheck = 0, __filler = 0}
After the fflush, the stream looks like this:
  (gdb) print *fp
  $3 = {_cnt = 0, _ptr = 0x8061654 "hoogarsh", _base = 0x8061654 "hoogarsh", 
    _flag = 152 '\230', _file = 6 '\006', __orientation = 0, __ionolock = 0, 
    __seekable = 1, __extendedfd = 0, __xf_nocheck = 0, __filler = 0}
As you can see, the _flag field has changed from
  _IORW | _IOEOF | _IOMYBUF | _IOREAD
to
  _IORW | _IOEOF | _IOMYBUF

Changing the test program by inserting an fseek operation
  ASSERT (getc (fp) == EOF);
  ASSERT (ftell (fp) == 8);
  fseek (fp, 0, SEEK_CUR);
  ASSERT (ftell (fp) == 8);
  ASSERT (putc ('!', fp) == '!');
makes it succeed as well. Before the fseek, the stream looks like this:
  (gdb) print *fp
  $2 = {_cnt = 0, _ptr = 0x8061664 "hoogarsh", _base = 0x8061664 "hoogarsh", 
    _flag = 153 '\231', _file = 6 '\006', __orientation = 0, __ionolock = 0, 
    __seekable = 1, __extendedfd = 0, __xf_nocheck = 0, __filler = 0}
After the fseek, it looks like this
  $3 = {_cnt = 0, _ptr = 0x8061664 "hoogarsh", _base = 0x8061664 "hoogarsh", 
    _flag = 136 '\210', _file = 6 '\006', __orientation = 0, __ionolock = 0, 
    __seekable = 1, __extendedfd = 0, __xf_nocheck = 0, __filler = 0}
As you can see, the _flag field has changed from
  _IORW | _IOEOF | _IOMYBUF | _IOREAD
to
  _IORW | _IOMYBUF

The crucial difference is the _IOREAD flag. Indeed, the OpenSolaris
ftell.c code looks at this flag.

For comparison, here's the same program, with a dump of the FILE's flags:

=================================== foo.c ===============================
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define ASSERT(expr) \
  do                                                                         \
    {                                                                        \
      if (!(expr))                                                           \
        {                                                                    \
          fprintf (stderr, "%s:%d: assertion failed\n",                      \
                   __FILE__, __LINE__);                                      \
          fflush (stderr);                                                   \
          abort ();                                                          \
        }                                                                    \
    }                                                                        \
  while (0)

static void dumpFILE (FILE *fp)
{
  fprintf (stderr, "_ptr = %p, _base = %p, _cnt = %d, _flag = 0x%x 
=%s%s%s%s%s\n",
           fp->_ptr, fp->_base, fp->_cnt, fp->_flag,
           fp->_flag & _IORW ? " _IORW" : "",
           fp->_flag & _IOEOF ? " _IOEOF" : "",
           fp->_flag & _IOMYBUF ? " _IOMYBUF" : "",
           fp->_flag & _IOWRT ? " _IOWRT" : "",
           fp->_flag & _IOREAD ? " _IOREAD" : "");
  fflush (stderr);
}

#define TESTFILE "t-fpurge2.tmp"

int
main (void)
{
  FILE *fp;

  /* Create a file with some contents.  */
  fp = fopen (TESTFILE, "w");
  if (fp == NULL)
    goto skip;
  if (fwrite ("foogarsh", 1, 8, fp) < 8)
    goto skip;
  if (fclose (fp))
    goto skip;

  /* The file's contents is now "foogarsh".  */

  /* Ensure that purging a read does not corrupt subsequent writes.  */
  fp = fopen (TESTFILE, "r+");
  if (fp == NULL)
    goto skip;
  if (fseek (fp, -1, SEEK_END))
    goto skip;
  ASSERT (getc (fp) == 'h');
  ASSERT (getc (fp) == EOF);
  dumpFILE(fp);
  ASSERT (ftell (fp) == 8);
  dumpFILE(fp);
#if 0
  fseek (fp, 0, SEEK_CUR);
  dumpFILE(fp);
#endif
  ASSERT (ftell (fp) == 8);
  dumpFILE(fp);
  ASSERT (putc ('!', fp) == '!');
  dumpFILE(fp);
  ASSERT (ftell (fp) == 9);
  ASSERT (fclose (fp) == 0);
  fp = fopen (TESTFILE, "r");
  if (fp == NULL)
    goto skip;
  {
    char buf[10];
    ASSERT (fread (buf, 1, 10, fp) == 9);
    ASSERT (memcmp (buf, "foogarsh!", 9) == 0);
  }
  ASSERT (fclose (fp) == 0);

  /* The file's contents is now "foogarsh!".  */

  remove (TESTFILE);
  return 0;

 skip:
  fprintf (stderr, "Skipping test: prerequisite file operations failed.\n");
  remove (TESTFILE);
  return 77;
}
=========================================================================

Result on various systems:

AIX
_ptr = 20000e30, _base = 20000e30, _cnt = 0, _flag = 0x98 = _IORW _IOEOF 
_IOMYBUF
_ptr = 20000e30, _base = 20000e30, _cnt = 0, _flag = 0x98 = _IORW _IOEOF 
_IOMYBUF
_ptr = 20000e30, _base = 20000e30, _cnt = 0, _flag = 0x98 = _IORW _IOEOF 
_IOMYBUF
_ptr = 20000e31, _base = 20000e30, _cnt = 4095, _flag = 0x8a = _IORW _IOMYBUF 
_IOWRT

HP-UX
_ptr = 40003228, _base = 40003228, _cnt = 0, _flag = 0x118 = _IORW _IOEOF 
_IOMYBUF
_ptr = 40003228, _base = 40003228, _cnt = 0, _flag = 0x118 = _IORW _IOEOF 
_IOMYBUF
_ptr = 40003228, _base = 40003228, _cnt = 0, _flag = 0x118 = _IORW _IOEOF 
_IOMYBUF
_ptr = 40003229, _base = 40003228, _cnt = 8191, _flag = 0x10a = _IORW _IOMYBUF 
_IOWRT

OSF/1
_ptr = 140006008, _base = 140006008, _cnt = 0, _flag = 0x118 = _IORW _IOEOF 
_IOMYBUF
_ptr = 140006008, _base = 140006008, _cnt = 0, _flag = 0x118 = _IORW _IOEOF 
_IOMYBUF
_ptr = 140006008, _base = 140006008, _cnt = 0, _flag = 0x118 = _IORW _IOEOF 
_IOMYBUF
_ptr = 140006009, _base = 140006008, _cnt = 65535, _flag = 0x10a = _IORW 
_IOMYBUF _IOWRT

Solaris
_ptr = 2162c, _base = 2162c, _cnt = 0, _flag = 0x99 = _IORW _IOEOF _IOMYBUF 
_IOREAD
_ptr = 2162c, _base = 2162c, _cnt = 0, _flag = 0x99 = _IORW _IOEOF _IOMYBUF 
_IOREAD
_ptr = 2162c, _base = 2162c, _cnt = 0, _flag = 0x99 = _IORW _IOEOF _IOMYBUF 
_IOREAD
_ptr = 2162d, _base = 2162c, _cnt = 8191, _flag = 0x8b = _IORW _IOMYBUF _IOWRT 
_IOREAD
foo.c:67: assertion failed
Abort

mingw
_ptr = 003E3B28, _base = 003E3B28, _cnt = 0, _flag = 0x99 = _IORW _IOEOF 
_IOMYBUF _IOREAD
_ptr = 003E3B28, _base = 003E3B28, _cnt = 0, _flag = 0x99 = _IORW _IOEOF 
_IOMYBUF _IOREAD
_ptr = 003E3B28, _base = 003E3B28, _cnt = 0, _flag = 0x99 = _IORW _IOEOF 
_IOMYBUF _IOREAD
_ptr = 003E3B29, _base = 003E3B28, _cnt = 4095, _flag = 0x8a = _IORW _IOMYBUF 
_IOWRT

As you can see, the _IOREAD flag was cleared during the putc call (when it set
the _IOWRT flag) on all platforms, but not on Solaris.

I don't think gnulib should hook into getc (to clear the _IOREAD flag when EOF
is reached) or into putc (to clear the _IOREAD flag when switching to write
mode), because that would cause major inefficiencies. Instead, the ftell 
function
should be taught to ignore the _IOREAD flag when _IOWRT is set.

I'm applying these two fixes.


2010-05-01  Bruno Haible  <address@hidden>

        freading: Adapt to special meaning of _IOREAD flag on Solaris.
        * lib/freading.c (freading): On Solaris, ignore the _IOREAD flag if
        the _IOWRT flag is also set.

--- lib/freading.c.orig Sat May  1 20:21:32 2010
+++ lib/freading.c      Sat May  1 19:20:54 2010
@@ -40,7 +40,11 @@
 #elif defined __EMX__               /* emx+gcc */
   return (fp->_flags & _IOREAD) != 0;
 #elif defined _IOERR                /* AIX, HP-UX, IRIX, OSF/1, Solaris, 
OpenServer, mingw */
+# if defined __sun                  /* Solaris */
+  return (fp->_flag & _IOREAD) != 0 && (fp->_flag & _IOWRT) == 0;
+# else
   return (fp->_flag & _IOREAD) != 0;
+# endif
 #elif defined __UCLIBC__            /* uClibc */
   return (fp->__modeflags & (__FLAG_READONLY | __FLAG_READING)) != 0;
 #elif defined __QNX__               /* QNX */


2010-05-01  Bruno Haible  <address@hidden>

        ftell, ftello: Work around Solaris bug.
        * m4/ftello.m4 (gl_FUNC_FTELLO): Detect Solaris bug.
        * lib/ftello.c: Include stdio-impl.h.
        (ftello): On Solaris, when _IOWRT is set, compute the result without
        looking at _IOREAD.
        * modules/ftello (Files): Add lib/stdio-impl.h.
        * doc/posix-functions/ftell.texi: Mention Solaris bug.
        * doc/posix-functions/ftello.texi: Likewise.
        Reported by Eric Blake.

--- doc/posix-functions/ftell.texi.orig Sat May  1 20:31:42 2010
+++ doc/posix-functions/ftell.texi      Sat May  1 19:21:26 2010
@@ -10,6 +10,10 @@
 @itemize
 @item
 This function mistakenly succeeds on pipes on some platforms: mingw.
address@hidden
+This function produces incorrect results after @code{putc} that followed a
address@hidden call that reached EOF on some platforms:
+Solaris 10.
 @end itemize
 
 Portability problems not fixed by Gnulib:
--- doc/posix-functions/ftello.texi.orig        Sat May  1 20:31:42 2010
+++ doc/posix-functions/ftello.texi     Sat May  1 19:21:19 2010
@@ -15,6 +15,10 @@
 The declaration of @code{ftello} in @code{<stdio.h>} is not enabled by default
 on some platforms: glibc 2.3.6.
 @item
+This function produces incorrect results after @code{putc} that followed a
address@hidden call that reached EOF on some platforms:
+Solaris 10.
address@hidden
 This function fails on seekable stdin, stdout, and stderr: cygwin <= 1.5.24.
 @end itemize
 
--- lib/ftello.c.orig   Sat May  1 20:31:42 2010
+++ lib/ftello.c        Sat May  1 20:17:25 2010
@@ -22,6 +22,8 @@
 /* Get lseek.  */
 #include <unistd.h>
 
+#include "stdio-impl.h"
+
 off_t
 ftello (FILE *fp)
 #undef ftello
@@ -36,6 +38,28 @@
     return -1;
 #endif
 
+#if FTELLO_BROKEN_AFTER_SWITCHING_FROM_READ_TO_WRITE /* Solaris */
+  /* The Solaris stdio leaves the _IOREAD flag set after reading from a file
+     reaches EOF and the program then starts writing to the file.  ftello
+     gets confused by this.  */
+  if (fp_->_flag & _IOWRT)
+    {
+      off_t pos;
+
+      /* Call ftello nevertheless, for the side effects that it does on fp.  */
+      ftello (fp);
+
+      /* Compute the file position ourselves.  */
+      pos = llseek (fileno (fp), (off_t) 0, SEEK_CUR);
+      if (pos >= 0)
+        {
+          if ((fp_->_flag & _IONBF) == 0 && fp_->_base != NULL)
+            pos += fp_->_ptr - fp_->_base;
+        }
+      return pos;
+    }
+#endif
+
 #if defined __SL64 && defined __SCLE /* Cygwin */
   if ((fp->_flags & __SL64) == 0)
     {
--- m4/ftello.m4.orig   Sat May  1 20:31:42 2010
+++ m4/ftello.m4        Sat May  1 19:49:09 2010
@@ -1,5 +1,5 @@
-# ftello.m4 serial 6
-dnl Copyright (C) 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+# ftello.m4 serial 7
+dnl Copyright (C) 2007-2010 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
 dnl with or without modifications, as long as this notice is preserved.
@@ -10,7 +10,7 @@
   AC_REQUIRE([AC_PROG_CC])
   AC_REQUIRE([gl_STDIN_LARGE_OFFSET])
 
-  dnl Persuade glibc <stdio.h> to declare fseeko().
+  dnl Persuade glibc <stdio.h> to declare ftello().
   AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
 
   AC_CACHE_CHECK([for ftello], [gl_cv_func_ftello],
@@ -23,6 +23,93 @@
   else
     if test $gl_cv_var_stdin_large_offset = no; then
       REPLACE_FTELLO=1
+    else
+      dnl Detect bug on Solaris.
+      dnl ftell and ftello produce incorrect results after putc that followed a
+      dnl getc call that reached EOF on Solaris. This is because the _IOREAD
+      dnl flag does not get cleared in this case, even though _IOWRT gets set,
+      dnl and ftell and ftello look whether the _IOREAD flag is set.
+      AC_REQUIRE([AC_CANONICAL_HOST])
+      AC_CACHE_CHECK([whether ftello works],
+        [gl_cv_func_ftello_works],
+        [
+          dnl Initial guess, used when cross-compiling or when /dev/tty cannot
+          dnl be opened.
+changequote(,)dnl
+          case "$host_os" in
+                      # Guess no on Solaris.
+            solaris*) gl_cv_func_ftello_works="guessing no" ;;
+                      # Guess yes otherwise.
+            *)        gl_cv_func_ftello_works="guessing yes" ;;
+          esac
+changequote([,])dnl
+          AC_TRY_RUN([
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#define TESTFILE "conftest.tmp"
+int
+main (void)
+{
+  FILE *fp;
+
+  /* Create a file with some contents.  */
+  fp = fopen (TESTFILE, "w");
+  if (fp == NULL)
+    return 70;
+  if (fwrite ("foogarsh", 1, 8, fp) < 8)
+    return 71;
+  if (fclose (fp))
+    return 72;
+
+  /* The file's contents is now "foogarsh".  */
+
+  /* Try writing after reading to EOF.  */
+  fp = fopen (TESTFILE, "r+");
+  if (fp == NULL)
+    return 73;
+  if (fseek (fp, -1, SEEK_END))
+    return 74;
+  if (!(getc (fp) == 'h'))
+    return 1;
+  if (!(getc (fp) == EOF))
+    return 2;
+  if (!(ftell (fp) == 8))
+    return 3;
+  if (!(ftell (fp) == 8))
+    return 4;
+  if (!(putc ('!', fp) == '!'))
+    return 5;
+  if (!(ftell (fp) == 9))
+    return 6;
+  if (!(fclose (fp) == 0))
+    return 7;
+  fp = fopen (TESTFILE, "r");
+  if (fp == NULL)
+    return 75;
+  {
+    char buf[10];
+    if (!(fread (buf, 1, 10, fp) == 9))
+      return 10;
+    if (!(memcmp (buf, "foogarsh!", 9) == 0))
+      return 11;
+  }
+  if (!(fclose (fp) == 0))
+    return 12;
+
+  /* The file's contents is now "foogarsh!".  */
+
+  return 0;
+}], [gl_cv_func_ftello_works=yes], [gl_cv_func_ftello_works=no], [:])
+        ])
+      case "$gl_cv_func_ftello_works" in
+        *yes) ;;
+        *)
+          REPLACE_FTELLO=1
+          AC_DEFINE([FTELLO_BROKEN_AFTER_SWITCHING_FROM_READ_TO_WRITE], [1],
+            [Define to 1 if the system's ftello function has the Solaris bug.])
+          ;;
+      esac
     fi
   fi
   if test $HAVE_FTELLO = 0 || test $REPLACE_FTELLO = 1; then
--- modules/ftello.orig Sat May  1 20:31:42 2010
+++ modules/ftello      Sat May  1 19:50:45 2010
@@ -3,6 +3,7 @@
 
 Files:
 lib/ftello.c
+lib/stdio-impl.h
 m4/ftello.m4
 
 Depends-on: