bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Issues with posix functions on modern macOS/Xcode


From: Bruno Haible
Subject: Re: Issues with posix functions on modern macOS/Xcode
Date: Sun, 21 Mar 2021 03:57:39 +0100
User-agent: KMail/5.1.3 (Linux/4.4.0-203-generic; KDE/5.18.0; x86_64; ; )

> > > FAIL: test-fflush2.sh
> > > FAIL: test-fpurge
> > > FAIL: test-ftell.sh
> > > FAIL: test-ftell2.sh
> > > FAIL: test-ftello.sh
> > > FAIL: test-ftello2.sh
> 
> These would need debugging on a macOS 10.15 machine. Anyone willing to take
> this challenge?

The compilefarm now has a macOS 11.2 machine. These tests fail there as well.
I could debug it, and found that it is a regression introduced in macOS 10.15.0:

--- Libc-1272.250.1/stdio/FreeBSD/fflush.c      2015-02-04 23:57:51.000000000 
+0100
+++ Libc-1353.11.2/stdio/FreeBSD/fflush.c       2019-09-21 00:18:36.000000000 
+0200
...
@@ -110,12 +90,52 @@
        int n, t;
 
        t = fp->_flags;
-       if ((t & __SWR) == 0)
-               return (0);
 
        if ((p = fp->_bf._base) == NULL)
                return (0);
 
+       /*
+        * SUSv3 requires that fflush() on a seekable input stream updates the 
file
+        * position indicator with the underlying seek function.  Use a dumb 
fseek
+        * for this (don't attempt to preserve the buffers).
+        */
+       if ((t & __SRD) != 0) {
+               if (fp->_seek == NULL) {
+                       /*
+                        * No way to seek this file -- just return "success."
+                        */
+                       return (0);
+               }
+
+               n = fp->_r;
+
+               if (n > 0) {
+                       /*
+                        * See _fseeko's dumb path.
+                        */
+                       if (_sseek(fp, (fpos_t)-n, SEEK_CUR) == -1) {
+                               if (errno == ESPIPE) {
+                                       /*
+                                        * Ignore ESPIPE errors, since there's 
no way to put the bytes
+                                        * back into the pipe.
+                                        */
+                                       return (0);
+                               }
+                               return (EOF);
+                       }
+
+                       if (HASUB(fp)) {
+                               FREEUB(fp);
+                       }
+                       fp->_p = fp->_bf._base;
+                       fp->_r = 0;
+                       fp->_flags &= ~__SEOF;
+                       memset(&fp->_mbstate, 0, sizeof(mbstate_t));
+               }
+               return (0);
+       }
+
+       if ((t & __SWR) != 0) {
        n = fp->_p - p;         /* write this much */
 
        /*
@@ -142,6 +162,7 @@
                        return (EOF);
                }
        }
+       }
        return (0);
 }
 

They added new code to the function __sflush, which makes sense (in order to
make fflush() POSIX compliant). But __sflush is being called by ftello(),
and the new code in __sflush severely disturbs the functioning of ftello:
  - it causes the ungetc buffer to be discarded,
  - it causes a flush of buffered data (which no one has asked for),
  - it returns a broken file position.

The bug was already spotted in the wild, in some non-GNU program:
<https://github.com/espeak-ng/espeak-ng/issues/674>.

This patch provides a workaround.


2021-03-20  Bruno Haible  <bruno@clisp.org>

        ftello: Work around bug in macOS >= 10.15.
        Reported by Martin Storsjö <martin@martin.st> in
        <https://lists.gnu.org/archive/html/bug-gnulib/2020-12/msg00002.html>.
        * m4/ungetc.m4 (gl_FUNC_UNGETC_WORKS): On macOS, don't define
        FUNC_UNGETC_BROKEN. Instead, set gl_ftello_broken_after_ungetc to yes.
        * m4/ftello.m4 (gl_FUNC_FTELLO): Invoke gl_FUNC_UNGETC_WORKS, and
        arrange to provide the workaround if ftello is broken after ungetc.
        * lib/ftello.c: Include <errno.h>, intprops.h.
        (ftello) [FTELLO_BROKEN_AFTER_UNGETC]: Implement from scratch.
        * modules/ftello (Files): Add m4/ungetc.m4.
        (Depends-on): Add intprops.
        * doc/posix-functions/ftello.texi: Mention the macOS bug.

diff --git a/doc/posix-functions/ftello.texi b/doc/posix-functions/ftello.texi
index 2561c5d..eab591f 100644
--- a/doc/posix-functions/ftello.texi
+++ b/doc/posix-functions/ftello.texi
@@ -20,6 +20,11 @@ This function produces incorrect results after @code{putc} 
that followed a
 @code{getc} call that reached EOF on some platforms:
 Solaris 11 2010-11.
 @item
+This function, when invoked after @code{ungetc}, throws away the @code{ungetc}
+buffer, changes the stream's file position, and returns the wrong position on
+some platforms:
+macOS 10.15 and newer.
+@item
 This function fails on seekable stdin, stdout, and stderr: cygwin <= 1.5.24.
 @item
 On platforms where @code{off_t} is a 32-bit type, @code{ftello} does not work
diff --git a/lib/ftello.c b/lib/ftello.c
index c5a4e0c..da13694 100644
--- a/lib/ftello.c
+++ b/lib/ftello.c
@@ -19,6 +19,9 @@
 /* Specification.  */
 #include <stdio.h>
 
+#include <errno.h>
+#include "intprops.h"
+
 /* Get lseek.  */
 #include <unistd.h>
 
@@ -40,13 +43,79 @@ ftello (FILE *fp)
 # endif
 #endif
 {
-#if LSEEK_PIPE_BROKEN
+#if FTELLO_BROKEN_AFTER_UNGETC /* macOS >= 10.15 */
+  /* The system's ftello() is completely broken, because it calls __sflush,
+     which makes side effects on the stream.  */
+
+  /* Handle non-seekable files first.  */
+  if (fp->_file < 0 || fp->_seek == NULL)
+    {
+      errno = ESPIPE;
+      return -1;
+    }
+
+  /* Determine the current offset, ignoring buffered and pushed-back bytes.  */
+  off_t pos;
+
+  if (fp->_flags & __SOFF)
+    pos = fp->_offset;
+  else
+    {
+      pos = fp->_seek (fp->_cookie, 0, SEEK_CUR);
+      if (pos < 0)
+        return -1;
+      if (fp->_flags & __SOPT)
+        {
+          fp->_offset = pos;
+          fp->_flags |= __SOFF;
+        }
+    }
+
+  if (fp->_flags & __SRD)
+    {
+      /* Now consider buffered and pushed-back bytes from ungetc.  */
+      if (fp->_ub._base != NULL)
+        /* Considering the buffered bytes, we are at position
+             pos - fp->_ur.
+           Considering also the pushed-back bytes, we are at position
+             pos - fp->_ur - fp->_r.  */
+        pos = pos - fp->_ur - fp->_r;
+      else
+        /* Considering the buffered bytes, we are at position
+             pos - fp->_r.  */
+        pos = pos - fp->_r;
+      if (pos < 0)
+        {
+          errno = EIO;
+          return -1;
+        }
+    }
+  else if ((fp->_flags & __SWR) && fp->_p != NULL)
+    {
+      /* Consider the buffered bytes.  */
+      off_t buffered = fp->_p - fp->_bf._base;
+
+      /* Compute pos + buffered, with overflow check.  */
+      off_t sum;
+      if (! INT_ADD_OK (pos, buffered, &sum))
+        {
+          errno = EOVERFLOW;
+          return -1;
+        }
+      pos = sum;
+    }
+
+  return pos;
+
+#else
+
+# if LSEEK_PIPE_BROKEN
   /* mingw gives bogus answers rather than failure on non-seekable files.  */
   if (lseek (fileno (fp), 0, SEEK_CUR) == -1)
     return -1;
-#endif
+# endif
 
-#if FTELLO_BROKEN_AFTER_SWITCHING_FROM_READ_TO_WRITE /* Solaris */
+# 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.  */
@@ -66,9 +135,9 @@ ftello (FILE *fp)
         }
       return pos;
     }
-#endif
+# endif
 
-#if defined __SL64 && defined __SCLE /* Cygwin */
+# if defined __SL64 && defined __SCLE /* Cygwin */
   if ((fp->_flags & __SL64) == 0)
     {
       /* Cygwin 1.5.0 through 1.5.24 failed to open stdin in 64-bit
@@ -80,6 +149,9 @@ ftello (FILE *fp)
       fp->_seek64 = tmp->_seek64;
       fclose (tmp);
     }
-#endif
+# endif
+
   return ftello (fp);
+
+#endif
 }
diff --git a/m4/ftello.m4 b/m4/ftello.m4
index 1a0f7bc..65bec39 100644
--- a/m4/ftello.m4
+++ b/m4/ftello.m4
@@ -1,4 +1,4 @@
-# ftello.m4 serial 13
+# ftello.m4 serial 14
 dnl Copyright (C) 2007-2021 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -130,6 +130,15 @@ main (void)
           ;;
       esac
     fi
+    if test $REPLACE_FTELLO = 0; then
+      dnl Detect bug on macOS >= 10.15.
+      gl_FUNC_UNGETC_WORKS
+      if test $gl_ftello_broken_after_ungetc = yes; then
+        REPLACE_FTELLO=1
+        AC_DEFINE([FTELLO_BROKEN_AFTER_UNGETC], [1],
+          [Define to 1 if the system's ftello function has the macOS bug.])
+      fi
+    fi
   fi
 ])
 
diff --git a/m4/ungetc.m4 b/m4/ungetc.m4
index b648fea..dd5d1dd 100644
--- a/m4/ungetc.m4
+++ b/m4/ungetc.m4
@@ -1,4 +1,4 @@
-# ungetc.m4 serial 9
+# ungetc.m4 serial 10
 dnl Copyright (C) 2009-2021 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -55,11 +55,19 @@ AC_DEFUN_ONCE([gl_FUNC_UNGETC_WORKS],
          esac
         ])
     ])
+  gl_ftello_broken_after_ungetc=no
   case "$gl_cv_func_ungetc_works" in
     *yes) ;;
     *)
-      AC_DEFINE([FUNC_UNGETC_BROKEN], [1],
-        [Define to 1 if ungetc is broken when used on arbitrary bytes.])
+      dnl On macOS >= 10.15, where the above program fails with exit code 6,
+      dnl we fix it through an ftello override.
+      case "$host_os" in
+        darwin*) gl_ftello_broken_after_ungetc=yes ;;
+        *)
+          AC_DEFINE([FUNC_UNGETC_BROKEN], [1],
+            [Define to 1 if ungetc is broken when used on arbitrary bytes.])
+          ;;
+      esac
       ;;
   esac
 ])
diff --git a/modules/ftello b/modules/ftello
index 6de0859..fc6bafb 100644
--- a/modules/ftello
+++ b/modules/ftello
@@ -6,6 +6,7 @@ lib/ftello.c
 lib/stdio-impl.h
 m4/fseeko.m4
 m4/ftello.m4
+m4/ungetc.m4
 
 Depends-on:
 stdio
@@ -13,6 +14,7 @@ extensions
 largefile
 sys_types
 lseek           [test $HAVE_FTELLO = 0 || test $REPLACE_FTELLO = 1]
+intprops        [test $HAVE_FTELLO = 0 || test $REPLACE_FTELLO = 1]
 # Just to guarantee consistency between ftell() and ftello().
 ftell
 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]