Discussion:
stdnoreturn: fix for Cygwin
Bruno Haible
2017-08-17 00:01:31 UTC
Permalink
On Cygwin 1.7.30, I'm seeing this testdir build failure, when compiling
test-stdnoreturn.c:

/usr/include/stdlib.h:66:28: error: expected ‘,’ or ‘;’ before ‘)’ token
_VOID _EXFUN(abort,(_VOID) _ATTRIBUTE ((noreturn)));
^

Similarly on Cygwin 1.5.25:

In file included from ../gllib/stdlib.h:36,
from ../../gltests/test-stdnoreturn.c:23:
/usr/include/stdlib.h:61: error: parse error before "__attribute__"
/usr/include/stdlib.h:61: error: parse error before ')' token

However, the situation is not exactly the same:
* On Cygwin 1.5.25, there is no standard <stdnoreturn.h>, so gnulib provides
its own one.
* On Cygwin 1.7.30, GCC comes with a <stdnoreturn.h>, but it does the same
thing: "#define noreturn _Noreturn".
In both cases, the problem is the definition introduced by gnulib
(through config.h):
#define _Noreturn __attribute__ ((__noreturn__))

This patch fixes both cases.


2017-08-16 Bruno Haible <***@clisp.org>

stdnoreturn: Fix test compilation failure on Cygwin.
* m4/stdnoreturn.m4 (gl_STDNORETURN_H): On Cygwin, use gnulib's
<stdnoreturn.h> replacement.
* lib/stdnoreturn.in.h (noreturn): Treat Cygwin like MSVC.
* doc/posix-headers/stdnoreturn.texi: Mention the Cygwin problem.

diff --git a/doc/posix-headers/stdnoreturn.texi b/doc/posix-headers/stdnoreturn.texi
index dc20b61..6574d4a 100644
--- a/doc/posix-headers/stdnoreturn.texi
+++ b/doc/posix-headers/stdnoreturn.texi
@@ -27,13 +27,13 @@ When the macro @code{lint} is defined, standard headers define
expands to the empty token sequence on some platforms:
Cygwin 2.5.1, FreeBSD 10.3.
@item
-On MSVC 14, @code{noreturn} expands to the empty token sequence, to avoid
-problems with standard headers that use @code{__declspec (noreturn)}
-directly. Although the resulting code operates correctly, the
-compiler is not informed whether @code{noreturn} functions do not
-return, so it may generate incorrect warnings at compile-time, or code
-that is slightly less optimized. This problem does not occur with
-@code{_Noreturn}.
+On Cygwin 1.7.30 and MSVC 14, @code{noreturn} expands to the empty token
+sequence, to avoid problems with standard headers that use @code{noreturn}
+in combination with @code{__attribute__} or @code{__declspec}. Although
+the resulting code operates correctly, the compiler is not informed whether
+@code{noreturn} functions do not return, so it may generate incorrect
+warnings at compile-time, or code that is slightly less optimized. This
+problem does not occur with @code{_Noreturn}.
@item
Circa 2012 bleeding-edge GCC with @code{-Werror=old-style-declaration}
requires @code{_Noreturn} or @code{noreturn} before the returned type
diff --git a/lib/stdnoreturn.in.h b/lib/stdnoreturn.in.h
index 9b25a56..445c227 100644
--- a/lib/stdnoreturn.in.h
+++ b/lib/stdnoreturn.in.h
@@ -28,15 +28,25 @@

/* The definition of _Noreturn is copied here. */

-#if 1200 <= _MSC_VER
-/* Standard include files on this platform contain declarations like
- "__declspec (noreturn) void abort (void);". "#define noreturn
- _Noreturn" would cause this declaration to be rewritten to the
- invalid "__declspec (__declspec (noreturn)) void abort (void);".
- Instead, define noreturn to empty, so that such declarations are
- rewritten to "__declspec () void abort (void);", which is
- equivalent to "void abort (void);"; this gives up on noreturn's
- advice to the compiler but at least it is valid code. */
+#if 1200 <= _MSC_VER || defined __CYGWIN__
+/* On MSVC, standard include files contain declarations like
+ __declspec (noreturn) void abort (void);
+ "#define noreturn _Noreturn" would cause this declaration to be rewritten
+ to the invalid
+ __declspec (__declspec (noreturn)) void abort (void);
+
+ Similarly, on Cygwin, standard include files contain declarations like
+ void __cdecl abort (void) __attribute__ ((noreturn));
+ "#define noreturn _Noreturn" would cause this declaration to be rewritten
+ to the invalid
+ void __cdecl abort (void) __attribute__ ((__attribute__ ((__noreturn__))));
+
+ Instead, define noreturn to empty, so that such declarations are rewritten to
+ __declspec () void abort (void);
+ or
+ void __cdecl abort (void) __attribute__ (());
+ respectively. This gives up on noreturn's advice to the compiler but at
+ least it is valid code. */
# define noreturn /*empty*/
#else
# define noreturn _Noreturn
diff --git a/m4/stdnoreturn.m4 b/m4/stdnoreturn.m4
index 9d70b37..ea4d735 100644
--- a/m4/stdnoreturn.m4
+++ b/m4/stdnoreturn.m4
@@ -9,33 +9,43 @@ dnl with or without modifications, as long as this notice is preserved.

AC_DEFUN([gl_STDNORETURN_H],
[
- AC_CACHE_CHECK([for working stdnoreturn.h],
- [gl_cv_header_working_stdnoreturn_h],
- [AC_COMPILE_IFELSE(
- [AC_LANG_PROGRAM(
- [[#include <stdlib.h>
- #include <stdnoreturn.h>
- /* Do not check for 'noreturn' after the return type.
- C11 allows it, but it's rarely done that way
- and circa-2012 bleeding-edge GCC rejects it when given
- -Werror=old-style-declaration. */
- noreturn void foo1 (void) { exit (0); }
- _Noreturn void foo2 (void) { exit (0); }
- int testit (int argc, char **argv) {
- if (argc & 1)
- return 0;
- (argv[0][0] ? foo1 : foo2) ();
- }
- ]])],
- [gl_cv_header_working_stdnoreturn_h=yes],
- [gl_cv_header_working_stdnoreturn_h=no])])
-
- if test $gl_cv_header_working_stdnoreturn_h = yes; then
- STDNORETURN_H=''
- else
- STDNORETURN_H='stdnoreturn.h'
- fi
-
+ AC_REQUIRE([AC_CANONICAL_HOST])
+ case "$host_os" in
+ cygwin*)
+ dnl Regardless whether a working <stdnoreturn.h> exists or not,
+ dnl we need our own <stdnoreturn.h>, because of the definition
+ dnl of _Noreturn done by gnulib-common.m4.
+ STDNORETURN_H='stdnoreturn.h'
+ ;;
+ *)
+ AC_CACHE_CHECK([for working stdnoreturn.h],
+ [gl_cv_header_working_stdnoreturn_h],
+ [AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[#include <stdlib.h>
+ #include <stdnoreturn.h>
+ /* Do not check for 'noreturn' after the return type.
+ C11 allows it, but it's rarely done that way
+ and circa-2012 bleeding-edge GCC rejects it when given
+ -Werror=old-style-declaration. */
+ noreturn void foo1 (void) { exit (0); }
+ _Noreturn void foo2 (void) { exit (0); }
+ int testit (int argc, char **argv)
+ {
+ if (argc & 1)
+ return 0;
+ (argv[0][0] ? foo1 : foo2) ();
+ }
+ ]])],
+ [gl_cv_header_working_stdnoreturn_h=yes],
+ [gl_cv_header_working_stdnoreturn_h=no])])
+ if test $gl_cv_header_working_stdnoreturn_h = yes; then
+ STDNORETURN_H=''
+ else
+ STDNORETURN_H='stdnoreturn.h'
+ fi
+ ;;
+ esac
AC_SUBST([STDNORETURN_H])
AM_CONDITIONAL([GL_GENERATE_STDNORETURN_H], [test -n "$STDNORETURN_H"])
])
Eric Blake
2017-08-17 00:59:55 UTC
Permalink
Adding cygwin list...
Post by Bruno Haible
On Cygwin 1.7.30, I'm seeing this testdir build failure, when compiling
/usr/include/stdlib.h:66:28: error: expected ‘,’ or ‘;’ before ‘)’ token
_VOID _EXFUN(abort,(_VOID) _ATTRIBUTE ((noreturn)));
+
+ Similarly, on Cygwin, standard include files contain declarations like
+ void __cdecl abort (void) __attribute__ ((noreturn));
+ "#define noreturn _Noreturn" would cause this declaration to be rewritten
+ to the invalid
+ void __cdecl abort (void) __attribute__ ((__attribute__ ((__noreturn__))));
Hmm. It's evil for any system .h file to ever use
__attribute__((barename)), since barename is in the user's namespace and
can therefore be defined to anything else, possibly breaking the header
(as you just proved). Hopefully, the problem goes away if cygwin
patches its headers to use __attribute__((__noreturn__)), so that gnulib
can then define noreturn at will.

I'll look into patching Cygwin to fix all barename attributes I can find
that should be __barename__ instead.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Eric Blake
2017-08-17 01:54:58 UTC
Permalink
Post by Eric Blake
Adding cygwin list...
Post by Bruno Haible
On Cygwin 1.7.30, I'm seeing this testdir build failure, when compiling
/usr/include/stdlib.h:66:28: error: expected ‘,’ or ‘;’ before ‘)’ token
_VOID _EXFUN(abort,(_VOID) _ATTRIBUTE ((noreturn)));
+
+ Similarly, on Cygwin, standard include files contain declarations like
+ void __cdecl abort (void) __attribute__ ((noreturn));
+ "#define noreturn _Noreturn" would cause this declaration to be rewritten
+ to the invalid
+ void __cdecl abort (void) __attribute__ ((__attribute__ ((__noreturn__))));
Hmm. It's evil for any system .h file to ever use
__attribute__((barename)), since barename is in the user's namespace and
can therefore be defined to anything else, possibly breaking the header
(as you just proved). Hopefully, the problem goes away if cygwin
patches its headers to use __attribute__((__noreturn__)), so that gnulib
can then define noreturn at will.
I'll look into patching Cygwin to fix all barename attributes I can find
that should be __barename__ instead.
On a closer look, I've already done this, several years ago (although a
few more have crept in since then):
https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=commitdiff;h=ada456dcf

Bruno, your cygwin installation of 1.7.30 is old, compared to current 2.8.2
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Loading...