fix compilation failure on Solaris 11
ClosedPublic

Authored by kgardas on Mar 28 2016, 5:08 PM.

Details

Summary

Solaris is quite picky about C and POSIX version combination.
For recent change to C99 we need to switch _XPG6 on which means
_XOPEN_SOURCE should be defined to 600

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kgardas updated this revision to Diff 7090.Mar 28 2016, 5:08 PM
kgardas retitled this revision from to fix compilation failure on Solaris 11.
kgardas updated this object.
kgardas edited the test plan for this revision. (Show Details)
kgardas added reviewers: bgamari, austin.
bgamari resigned from this revision.Mar 28 2016, 6:07 PM
bgamari added a reviewer: hvr.
bgamari removed a reviewer: bgamari.

Looks okay to me.

austin accepted this revision.Mar 28 2016, 6:10 PM
austin edited edge metadata.

I'll buy it.

This revision is now accepted and ready to land.Mar 28 2016, 6:10 PM
hvr accepted this revision.Mar 29 2016, 12:56 AM
hvr edited edge metadata.

I have been wondering about these feature test macros already...

@kgardas do you know which parts of http://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html don't apply to Solaris?

rts/PosixSource.h
19–20

Why don't we just set _XOPEN_SOURCE to 600 (or even 700) on all archs?

25–26

Is _ISOC99_SOURCE now implied by setting the compiler into C99-mode?

hvr added a comment.Mar 29 2016, 1:27 AM

PS: http://linux.die.net/man/7/feature_test_macros is more detailed than the glibc info manual

@hvr it's not IMHO the question about what macros are not implemented on Solaris but the question about Solaris strickness. It simply claims

346 /*
347  * It is invalid to compile an XPG3, XPG4, XPG4v2, or XPG5 application
348  * using c99.  The same is true for POSIX.1-1990, POSIX.2-1992, POSIX.1b,
349  * and POSIX.1c applications. Likewise, it is invalid to compile an XPG6
350  * or a POSIX.1-2001 application with anything other than a c99 or later
351  * compiler.  Therefore, we force an error in both cases.
352  */
353 #if defined(_STDC_C99) && (defined(__XOPEN_OR_POSIX) && !defined(_XPG6))
354 #error "Compiler or options invalid for pre-UNIX 03 X/Open applications \
355         and pre-2001 POSIX applications"
356 #elif !defined(_STDC_C99) && \
357         (defined(__XOPEN_OR_POSIX) && defined(_XPG6))
358 #error "Compiler or options invalid; UNIX 03 and POSIX.1-2001 applications \
359         require the use of c99"
360 #endif

and the error on line 354 is what I hit here. By defining _XOPEN_SOURCE to 600 this is corrected. Apparently Linux is not that strict and allows you to compile non XPG6 code with C99 compiler. Basically if you look here: https://github.com/joyent/illumos-joyent/blob/master/usr/src/uts/common/sys/feature_tests.h -- this is nearly the same like Solaris 11's own file except that Illumos modernize it to also support XPG7. Anyway, the logic of test above is still the same.

rts/PosixSource.h
19–20

Would be ok for Solaris, but I would need to check that on OpenBSD

25–26

I don't see it defined by the GNU C for empty file and neither defined not used in /usr/include headers. So I guess this is completely unnoticed by Solaris 11. Not sure about 10.

hvr requested changes to this revision.Mar 29 2016, 3:20 AM
hvr edited edge metadata.
hvr added inline comments.
rts/PosixSource.h
19–20

Fwiw, I just installed myself a Solaris11 in KVM (it was almost painless to my surprise)... so now I can at least experiment myself :-)

anyway, it seems to me that we should try to define consistently

#define _POSIX_C_SOURCE 200112L
#define _XOPEN_SOURCE   600

on all platforms, given we're aiming for a C99 base-line compatibility and the man-page says about these values:

(Since glibc 2.3.3) The value 200112L or greater additionally exposes definitions corresponding to the POSIX.1-2001 base specification (excluding the XSI extension) and also causes C95 (since glibc 2.12) and C99 (since glibc 2.10) features to be exposed.

(Since glibc 2.2) The value 600 or greater additionally exposes definitions for SUSv3
(UNIX 03; i.e., the POSIX.1-2001 base specification plus the XSI extension) and C99
definitions.

25–26

Any way to find out? :-)

This revision now requires changes to proceed.Mar 29 2016, 3:20 AM
hvr added inline comments.Mar 29 2016, 3:23 AM
rts/PosixSource.h
18

PS: this one's considered obsolete according to the man-page:

Defining this obsolete macro with any value is equivalent to defining _POSIX_C_SOURCE with the value 1.

hvr edited edge metadata.Mar 29 2016, 3:25 AM
hvr updated the Trac tickets for this revision.
This revision was automatically updated to reflect the committed changes.

@hvr uff, sorry about commit, I've noted your changed of mind too late.

hvr added a comment.Mar 29 2016, 4:21 AM

@hvr uff, sorry about commit, I've noted your changed of mind too late.

it can happen :-)

do you plan to follow-up with another commit or shall I take it from here?

@hvr agree with you on consistency C99 versus POSIX/XOPEN. Let me test following diff

diff --git a/rts/PosixSource.h b/rts/PosixSource.h
index 6246e3e..8246fda 100644
--- a/rts/PosixSource.h
+++ b/rts/PosixSource.h
@@ -11,25 +11,13 @@
 
 #include <ghcplatform.h>
 
-#if defined(freebsd_HOST_OS) || defined(dragonfly_HOST_OS) || defined(solaris2_HOST_OS)
+/* we aim for C99 so we need to define following two defines to be consistent
+   with what POSIX/XOPEN provide for C99. Some OSes are particularly picky
+   about the right versions defined here, e.g. Solaris */
 #define _POSIX_C_SOURCE 200112L
 #define _XOPEN_SOURCE   600
-#else
-#define _POSIX_SOURCE   1
-#define _POSIX_C_SOURCE 199506L
-#define _XOPEN_SOURCE   500
-// FreeBSD takes a different approach to _ISOC99_SOURCE: on FreeBSD it
-// means "I want *just* C99 things", whereas on GNU libc and Solaris
-// it means "I also want C99 things".
-//
-// On both GNU libc and FreeBSD, _ISOC99_SOURCE is implied by
-// _XOPEN_SOURCE==600, but on Solaris it is an error to omit it.
-#define _ISOC99_SOURCE
-// Defining __USE_MINGW_ANSI_STDIO is the most portable way to tell
-// mingw that we want to use the standard %lld style format specifiers,
-// rather than the Windows %I64d style
+
 #define __USE_MINGW_ANSI_STDIO 1
-#endif
hvr added a reviewer: Phyx.Mar 29 2016, 5:18 AM
hvr added inline comments.
rts/PosixSource.h
31

@kgardas This one seems redundant as well, as I stumbled over

/* We are activating __USE_MINGW_ANSI_STDIO for various define indicators.
   Note that we enable it also for _GNU_SOURCE in C++, but not for C case. */
#if (defined (_POSIX) || defined (_POSIX_SOURCE) || defined (_POSIX_C_SOURCE) \
     || defined (_ISOC99_SOURCE) \
     || defined (_XOPEN_SOURCE) || defined (_XOPEN_SOURCE_EXTENDED) \
     || (defined (_GNU_SOURCE) && defined (__cplusplus)) \
     || defined (_SVID_SOURCE)) \
    && !defined(__USE_MINGW_ANSI_STDIO)
/* Enable __USE_MINGW_ANSI_STDIO if _POSIX defined
 * and If user did _not_ specify it explicitly... */
#  define __USE_MINGW_ANSI_STDIO			1
#endif

in mingw's header files

@hvr: please review D2056

rts/PosixSource.h
31

Good to know, I've pushed D2056 but would like to have it solely focused on POSIX/XOPEN cleanup. Let's remove this in another Dxxxx