[libvirt] fseeko broken by Fedora rawhide glibc / git master (ie future 2.28)
Fedora rawhide has just upgraded to the latest glibc git master snapshot, of what will become the 2.28 release, and this has exposed bugs in gnulib's fseeko.c implementation (and probably more macros) besides. The issue starts are line 50: http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/fseeko.c#n50 #if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ Historically the _IO_ftrylockfile symbol has been defined unconditionally in glibc, by /usr/include/libio.h, which is pulled in unconditionally from /usr/include/stdio.h The libio.h header was deprecated in 2.27 release and is removed in git master for future 2.28. Thus the _IO_ftrylockfile symbol has gone away. https://sourceware.org/ml/libc-announce/2018/msg00000.html "The nonstandard header files <libio.h> and <_G_config.h> are deprecated and will be removed in a future release. Software that is still using either header should be updated to use standard <stdio.h> interfaces instead. libio.h was originally the header for a set of supported GNU extensions, but they have not been maintained as such in many years, they are now standing in the way of improvements to stdio, and we don't think there are any remaining external users. _G_config.h was never intended for public use, but predates the bits convention." We then fail the __GNU_LIBRARY__ test too, because modern glibc defines that to a value of '6', not '1'. For added fun __GNU_LIBRARY__ is considered deprecated too, with recommendation to use other symbols like __GLIBC__ and __GLIBC_MINOR__ /* This macro indicates that the installed library is the GNU C Library. For historic reasons the value now is 6 and this will stay from now on. The use of this variable is deprecated. Use __GLIBC__ and __GLIBC_MINOR__ now (see below) when you want to test for a specific GNU C library version and use the values in <gnu/lib-names.h> to get the sonames of the shared libraries. * #define __GNU_LIBRARY__ 6 I hit failure on fseeko.c, but many other files in gnulib test on _IO_ftrylockfile so I presume they are all broken. I'm curious why we're trying to replace fseeko impl at all, since I would expect the modern glibc impl to be suitable to use as-is. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Mar 05, 2018 at 12:19:24PM +0000, Daniel P. Berrangé wrote:
Fedora rawhide has just upgraded to the latest glibc git master snapshot, of what will become the 2.28 release, and this has exposed bugs in gnulib's fseeko.c implementation (and probably more macros) besides.
The issue starts are line 50:
http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/fseeko.c#n50
#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */
Historically the _IO_ftrylockfile symbol has been defined unconditionally in glibc, by /usr/include/libio.h, which is pulled in unconditionally from /usr/include/stdio.h
The libio.h header was deprecated in 2.27 release and is removed in git master for future 2.28. Thus the _IO_ftrylockfile symbol has gone away.
https://sourceware.org/ml/libc-announce/2018/msg00000.html
"The nonstandard header files <libio.h> and <_G_config.h> are deprecated and will be removed in a future release. Software that is still using either header should be updated to use standard <stdio.h> interfaces instead.
libio.h was originally the header for a set of supported GNU extensions, but they have not been maintained as such in many years, they are now standing in the way of improvements to stdio, and we don't think there are any remaining external users. _G_config.h was never intended for public use, but predates the bits convention."
We then fail the __GNU_LIBRARY__ test too, because modern glibc defines that to a value of '6', not '1'.
For added fun __GNU_LIBRARY__ is considered deprecated too, with recommendation to use other symbols like __GLIBC__ and __GLIBC_MINOR__
/* This macro indicates that the installed library is the GNU C Library. For historic reasons the value now is 6 and this will stay from now on. The use of this variable is deprecated. Use __GLIBC__ and __GLIBC_MINOR__ now (see below) when you want to test for a specific GNU C library version and use the values in <gnu/lib-names.h> to get the sonames of the shared libraries. * #define __GNU_LIBRARY__ 6
I hit failure on fseeko.c, but many other files in gnulib test on _IO_ftrylockfile so I presume they are all broken.
I'm curious why we're trying to replace fseeko impl at all, since I would expect the modern glibc impl to be suitable to use as-is.
Changing the test for '__GNU_LIBRARY__ == 1' to just "__GNU_LIBRARY__" does appear to work at first, but fflush.c then fails fflush.c: In function 'clear_ungetc_buffer_preserving_position': fflush.c:42:20: error: '_IO_IN_BACKUP' undeclared (first use in this function) if (fp->_flags & _IO_IN_BACKUP) ^~~~~~~~~~~~~ This constant was also in libio.h and so no longer defined. Re-defining it in gnulib feels dangerous because glibc is free to change its value at any time now this is no longer a public API symbol. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 03/05/2018 04:19 AM, Daniel P. Berrangé wrote:
I'm curious why we're trying to replace fseeko impl at all, since I would expect the modern glibc impl to be suitable to use as-is.
It's to work around glibc bug#12799, a longstanding bug with ungetc and fflush. See the bug-gnulib thread that starts here: https://lists.gnu.org/r/bug-gnulib/2009-01/msg00067.html Any chance of getting that glibc bug fixed in glibc 2.28? That should avoid the problem with Gnulib, and would be a good thing in its own right. In the meantime, the attached Gnulib patch is intended to work around the bug from the Gnulib side; does it work for you? (I haven't tried it on rawhide.)
On Mon, Mar 05, 2018 at 11:00:40AM -0800, Paul Eggert wrote:
On 03/05/2018 04:19 AM, Daniel P. Berrangé wrote:
I'm curious why we're trying to replace fseeko impl at all, since I would expect the modern glibc impl to be suitable to use as-is.
It's to work around glibc bug#12799, a longstanding bug with ungetc and fflush. See the bug-gnulib thread that starts here:
https://lists.gnu.org/r/bug-gnulib/2009-01/msg00067.html
Any chance of getting that glibc bug fixed in glibc 2.28? That should avoid the problem with Gnulib, and would be a good thing in its own right. In the meantime, the attached Gnulib patch is intended to work around the bug from the Gnulib side; does it work for you? (I haven't tried it on rawhide.)
Yes, this worked on rawhide when I tested with libvirt. Though note that libvirt doesn't pull in all of the files you've changed here, so my test hasn't validated everything.
From 1831628c0630ae96a43586b2a25ca51cbdba3e53 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Mon, 5 Mar 2018 10:56:29 -0800 Subject: [PATCH] fflush: adjust to glibc 2.28 libio.h removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Problem reported by Daniel P. Berrangé in: https://lists.gnu.org/r/bug-gnulib/2018-03/msg00000.html * lib/fbufmode.c (fbufmode): * lib/fflush.c (clear_ungetc_buffer_preserving_position) (disable_seek_optimization, rpl_fflush): * lib/fpending.c (__fpending): * lib/fpurge.c (fpurge): * lib/freadable.c (freadable): * lib/freadahead.c (freadahead): * lib/freading.c (freading): * lib/freadptr.c (freadptr): * lib/freadseek.c (freadptrinc): * lib/fseeko.c (fseeko): * lib/fseterr.c (fseterr): * lib/fwritable.c (fwritable): * lib/fwriting.c (fwriting): Check _IO_EOF_SEEN instead of _IO_ftrylockfile. * lib/stdio-impl.h (_IO_IN_BACKUP) [_IO_EOF_SEEN]: Define if not already defined. --- ChangeLog | 23 +++++++++++++++++++++++ lib/fbufmode.c | 2 +- lib/fflush.c | 6 +++--- lib/fpending.c | 2 +- lib/fpurge.c | 2 +- lib/freadable.c | 2 +- lib/freadahead.c | 2 +- lib/freading.c | 2 +- lib/freadptr.c | 2 +- lib/freadseek.c | 2 +- lib/fseeko.c | 4 ++-- lib/fseterr.c | 2 +- lib/fwritable.c | 2 +- lib/fwriting.c | 2 +- lib/stdio-impl.h | 6 ++++++ 15 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/ChangeLog b/ChangeLog index 667f91663..beb835670 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2018-03-05 Paul Eggert <eggert@cs.ucla.edu> + + fflush: adjust to glibc 2.28 libio.h removal + Problem reported by Daniel P. Berrangé in: + https://lists.gnu.org/r/bug-gnulib/2018-03/msg00000.html + * lib/fbufmode.c (fbufmode): + * lib/fflush.c (clear_ungetc_buffer_preserving_position) + (disable_seek_optimization, rpl_fflush): + * lib/fpending.c (__fpending): + * lib/fpurge.c (fpurge): + * lib/freadable.c (freadable): + * lib/freadahead.c (freadahead): + * lib/freading.c (freading): + * lib/freadptr.c (freadptr): + * lib/freadseek.c (freadptrinc): + * lib/fseeko.c (fseeko): + * lib/fseterr.c (fseterr): + * lib/fwritable.c (fwritable): + * lib/fwriting.c (fwriting): + Check _IO_EOF_SEEN instead of _IO_ftrylockfile. + * lib/stdio-impl.h (_IO_IN_BACKUP) [_IO_EOF_SEEN]: + Define if not already defined. + 2018-02-27 Paul Eggert <eggert@cs.ucla.edu>
environ: fix link error on 32-bit Cygwin diff --git a/lib/fbufmode.c b/lib/fbufmode.c index 73aa5fc07..f711fdfb7 100644 --- a/lib/fbufmode.c +++ b/lib/fbufmode.c @@ -31,7 +31,7 @@ fbufmode (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ # if HAVE___FLBF /* glibc >= 2.2 */ if (__flbf (fp)) return _IOLBF; diff --git a/lib/fflush.c b/lib/fflush.c index 983ade0ff..a6edfa105 100644 --- a/lib/fflush.c +++ b/lib/fflush.c @@ -33,7 +33,7 @@ #undef fflush
-#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */
/* Clear the stream's ungetc buffer, preserving the value of ftello (fp). */ static void @@ -72,7 +72,7 @@ clear_ungetc_buffer (FILE *fp)
#endif
-#if ! (defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */) +#if ! (defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */)
# if (defined __sferror || defined __DragonFly__ || defined __ANDROID__) && defined __SNPT /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */ @@ -148,7 +148,7 @@ rpl_fflush (FILE *stream) if (stream == NULL || ! freading (stream)) return fflush (stream);
-#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */
clear_ungetc_buffer_preserving_position (stream);
diff --git a/lib/fpending.c b/lib/fpending.c index c84e3a5b4..789f50e4e 100644 --- a/lib/fpending.c +++ b/lib/fpending.c @@ -32,7 +32,7 @@ __fpending (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ return fp->_IO_write_ptr - fp->_IO_write_base; #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */ diff --git a/lib/fpurge.c b/lib/fpurge.c index b1d417c7a..3aedcc373 100644 --- a/lib/fpurge.c +++ b/lib/fpurge.c @@ -62,7 +62,7 @@ fpurge (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -# if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +# if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ fp->_IO_read_end = fp->_IO_read_ptr; fp->_IO_write_ptr = fp->_IO_write_base; /* Avoid memory leak when there is an active ungetc buffer. */ diff --git a/lib/freadable.c b/lib/freadable.c index 03b50e186..c4ca0b86e 100644 --- a/lib/freadable.c +++ b/lib/freadable.c @@ -31,7 +31,7 @@ freadable (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ return (fp->_flags & _IO_NO_READS) == 0; #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */ diff --git a/lib/freadahead.c b/lib/freadahead.c index c2ecb5b28..23ec76ee5 100644 --- a/lib/freadahead.c +++ b/lib/freadahead.c @@ -30,7 +30,7 @@ extern size_t __sreadahead (FILE *); size_t freadahead (FILE *fp) { -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ if (fp->_IO_write_ptr > fp->_IO_write_base) return 0; return (fp->_IO_read_end - fp->_IO_read_ptr) diff --git a/lib/freading.c b/lib/freading.c index 73c28acdd..c24d0c88a 100644 --- a/lib/freading.c +++ b/lib/freading.c @@ -31,7 +31,7 @@ freading (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -# if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +# if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ return ((fp->_flags & _IO_NO_WRITES) != 0 || ((fp->_flags & (_IO_NO_READS | _IO_CURRENTLY_PUTTING)) == 0 && fp->_IO_read_base != NULL)); diff --git a/lib/freadptr.c b/lib/freadptr.c index 5aeadf3da..ffb801039 100644 --- a/lib/freadptr.c +++ b/lib/freadptr.c @@ -29,7 +29,7 @@ freadptr (FILE *fp, size_t *sizep) size_t size;
/* Keep this code in sync with freadahead! */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ if (fp->_IO_write_ptr > fp->_IO_write_base) return NULL; size = fp->_IO_read_end - fp->_IO_read_ptr; diff --git a/lib/freadseek.c b/lib/freadseek.c index e7b0c7bdb..5fd2dd7ca 100644 --- a/lib/freadseek.c +++ b/lib/freadseek.c @@ -36,7 +36,7 @@ freadptrinc (FILE *fp, size_t increment) /* Keep this code in sync with freadptr! */ #if HAVE___FREADPTRINC /* musl libc */ __freadptrinc (fp, increment); -#elif defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#elif defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ fp->_IO_read_ptr += increment; #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */ diff --git a/lib/fseeko.c b/lib/fseeko.c index 0101ab55f..193f4e8ce 100644 --- a/lib/fseeko.c +++ b/lib/fseeko.c @@ -47,7 +47,7 @@ fseeko (FILE *fp, off_t offset, int whence) #endif
/* These tests are based on fpurge.c. */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ if (fp->_IO_read_end == fp->_IO_read_ptr && fp->_IO_write_ptr == fp->_IO_write_base && fp->_IO_save_base == NULL) @@ -123,7 +123,7 @@ fseeko (FILE *fp, off_t offset, int whence) return -1; }
-#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ fp->_flags &= ~_IO_EOF_SEEN; fp->_offset = pos; #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ diff --git a/lib/fseterr.c b/lib/fseterr.c index 82649c3ac..adb637256 100644 --- a/lib/fseterr.c +++ b/lib/fseterr.c @@ -29,7 +29,7 @@ fseterr (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ fp->_flags |= _IO_ERR_SEEN; #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */ diff --git a/lib/fwritable.c b/lib/fwritable.c index 981382163..3b1a380ec 100644 --- a/lib/fwritable.c +++ b/lib/fwritable.c @@ -31,7 +31,7 @@ fwritable (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ return (fp->_flags & _IO_NO_WRITES) == 0; #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */ diff --git a/lib/fwriting.c b/lib/fwriting.c index 461ab0f1f..4a3d9c8d4 100644 --- a/lib/fwriting.c +++ b/lib/fwriting.c @@ -27,7 +27,7 @@ fwriting (FILE *fp) /* Most systems provide FILE as a struct and the necessary bitmask in <stdio.h>, because they need it for implementing getc() and putc() as fast macros. */ -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, Linux libc5 */ return (fp->_flags & (_IO_NO_READS | _IO_CURRENTLY_PUTTING)) != 0; #elif defined __sferror || defined __DragonFly__ || defined __ANDROID__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */ diff --git a/lib/stdio-impl.h b/lib/stdio-impl.h index 78d896e9f..05c5752a2 100644 --- a/lib/stdio-impl.h +++ b/lib/stdio-impl.h @@ -18,6 +18,12 @@ the same implementation of stdio extension API, except that some fields have different naming conventions, or their access requires some casts. */
+/* Glibc 2.28 made _IO_IN_BACKUP private. For now, work around this + problem by defining it ourselves. FIXME: Do not rely on glibc + internals. */ +#if !defined _IO_IN_BACKUP && defined _IO_EOF_SEEN +# define _IO_IN_BACKUP 0x100 +#endif
/* BSD stdio derived implementations. */
-- 2.14.3
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Paul Eggert