Hi Matthias,
On 17/12/2018 11:12 pm, Baesken, Matthias wrote:
Hello, please review
http://cr.openjdk.java.net/~mbaesken/webrevs/8215296.0/
in my change just -xc99=%none is removed, so we do not forbid c99 coding.
The -Xa compile flag is kept, no special additional settings are needed to
compile png/awt .
It's not at all clear to me that C99-isms will be allowed if -Xa is set.
I don't think jdk-submit tests Solaris. I'm putting this through our
internal builds.
Thanks,
David
Thanks, Matthias
----------------------------------------------------------------------
Message: 1
Date: Fri, 14 Dec 2018 15:39:26 +0100
From: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>
To: Erik Joelsson <erik.joels...@oracle.com>, build-dev
<build-dev@openjdk.java.net>, "awt-...@openjdk.java.net"
<awt-...@openjdk.java.net>, 2d-dev <2d-...@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8215296 do not disable c99 on
Solaris
Message-ID: <5874d10e-db2d-8681-a54b-a1eeb6e45...@oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
On 2018-12-14 12:49, Magnus Ihse Bursie wrote:
13 dec. 2018 kl. 19:07 skrev Erik Joelsson <erik.joels...@oracle.com
<mailto:erik.joels...@oracle.com>>:
On 2018-12-13 02:11, Magnus Ihse Bursie wrote:
-D_XPG6
??
To be honest, I'm not completely sure about this. Without this
define, the build failed with the following error message:
Compiler or options invalid for pre-UNIX 03 X/Open applications and
pre-2001 POSIX applications
This was triggered by the following section in
/usr/include/sys/feature_tests.h:
/*
* It is invalid to compile an XPG3, XPG4, XPG4v2, or XPG5 application
* using c99. The same is true for POSIX.1-1990, POSIX.2-1992,
POSIX.1b,
* and POSIX.1c applications. Likewise, it is invalid to compile an
XPG6
* or a POSIX.1-2001 application with anything other than a c99 or
later
* compiler. Therefore, we force an error in both cases.
*/
#if defined(_STDC_C99) && (defined(__XOPEN_OR_POSIX) &&
!defined(_XPG6))
#error "Compiler or options invalid for pre-UNIX 03 X/Open
applications \
and pre-2001 POSIX applications"
#elif !defined(_STDC_C99) && \
(defined(__XOPEN_OR_POSIX) && defined(_XPG6))
#error "Compiler or options invalid; UNIX 03 and POSIX.1-2001
applications \
require the use of c99"
#endif
The solution, as also hinted to by searching for other resolutions
to this error online, was to provide the _XPG6 system define. But
exactly how we end up in feature_tests.h with __XOPEN_OR_POSIX set,
without _XPG6 set, and only when compiling this library and not
others, I don't know. I also don't understand what the XPG standard
refers to, nor what versions 2-5 means or what version 6 has that
differs from them.
By setting this flag, I am telling solaris include headers that we
want to compile using the XPG standard version 6, instead of an
older one. It solves the problem. I am happy enough with this. Are you?
It looks like this comes from libpng. It has this in
src/java.desktop//share/native/libsplashscreen/libpng/pngpriv.h:
/* Feature Test Macros. The following are defined here to ensure
that correctly
* implemented libraries reveal the APIs libpng needs to build and
hide those
* that are not needed and potentially damaging to the compilation.
*
* Feature Test Macros must be defined before any system header is
included (see
* POSIX 1003.1 2.8.2 "POSIX Symbols."
*
* These macros only have an effect if the operating system supports
either
* POSIX 1003.1 or C99, or both. On other operating systems
(particularly
* Windows/Visual Studio) there is no effect; the OS specific tests
below are
* still required (as of 2011-05-02.)
*/
#ifndef _POSIX_SOURCE
# define _POSIX_SOURCE 1 /* Just the POSIX 1003.1 and C89 APIs */
#endif
This in turn triggers _XOPEN_OR_POSIX to be defined in
/usr/include/sys/feature_tests.h and so triggers the error.
What I'm not clear about is if libpng is trying to declare that it
should not be compiled with any newer standards, and so by doing
that, we risk introducing problems. Reading in the system header, it
seems the _XPG6 macro is internal and should not be used by the
application. It's derived from _XOPEN_SOURCE=600 or
_POSIX_C_SOURCE=200112L which is what applications should use.
Interesting. We should probably define one, or both of these. Perhaps
globally for all native files and compilers. It might have been the
case that the solstudio compiler set _POSIX_C_SOURCE for us before,
prior to setting -std=c99. The following stack overflow article claims
that this is at least the behavior of gcc/clang:
https://stackoverflow.com/questions/21867897/c89-and-posix-at-the-
same-time
So we might have had an implicit _POSIX_C_SOURCE that we now miss.
That would explain why this starts to fail. I'll see if I can confirm
this the next time I log into a Solaris computer.
Of course it was not as simple. Setting:
ifeq ($(OPENJDK_TARGET_OS), solaris)
LIBSPLASHSCREEN_CFLAGS += -D_POSIX_C_SOURCE=200112L -
D_XOPEN_SOURCE=600
endif
instead made us fail with:
open/src/java.desktop/unix/native/libsplashscreen/splashscreen_sys.c",
line 143: error: incomplete struct/union/enum timezone: tz
I don't have more time to dig into this now. Overall, changes such as
these make it all feel a bit scary; I recommend that any change to this
be made in JDK 13 and not 12.
/Magnus
Otoh, the same article claims, and it sounds reasonable, that we
should set these variables ourself, to be well behaved and to minimize
surprises. And I think this applies to all unix platforms, regardless
of compiler being used. I'll see if I can kick off a test job with
this to see how/if it influences other platforms. But it sounds like
something we should do; the level of posix conformance should be
controlled by us, not left to chance. We also need to verify, of
course, that all platforms we want to support is capable of
supporting _POSIX_C_SOURCE=200112L. I doubt there's a problem
though.
Possibly on AIX...
/Magnus
So the the question is, is it ok to override the requirements of
libpng or should it receive special treatment? If we are fine with
overriding, then we should use one of the public APIs instead.
/Erik
/Magnus
Thanks,
David
On 13/12/2018 7:02 am, Magnus Ihse Bursie wrote:
On 2018-12-12 20:08, Magnus Ihse Bursie wrote:
On 2018-12-12 19:12, Magnus Ihse Bursie wrote:
From the bug report:
"Currently we disable C99 in the Solaris build by setting
-xc99=%none%.
This differs from a lot of other build environments like
gcc/Linux or VS2013/2017 on Windows where C99 features work.
We should remove this difference on Solaris and remove or
replace the setting.
Kim Barrett mentioned :
"I merely mentioned the C++14 work as evidence that removing
-xc99=%none% didn?t appear harmful."
However it will take more time until the C++14 change is in."
I am currently running a test build on our CI build system to
confirm that this does not break the Solaris build (but I'd be
highly surprised if it did). I will not push this until the
builds are cleared.
Of course it was not that simple... :-( Two AWT libraries (at
least) failed to build. I'm currently investigating if there's a
simple fix to that.
New attempt, that fixes the two AWT libraries:
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8215296-build-solstudio-with-
c99/webrev.01
<http://cr.openjdk.java.net/%7Eihse/JDK-8215296-build-solstudio-
with-c99/webrev.01>
Now this passes the CI build test.
/Magnus
/Magnus
/Magnus
Bug: https://bugs.openjdk.java.net/browse/JDK-8215296
Patch inline:
diff --git a/make/autoconf/flags-cflags.m4
b/make/autoconf/flags-cflags.m4
--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -559,7 +559,7 @@
TOOLCHAIN_CFLAGS="-errshort=tags"
TOOLCHAIN_CFLAGS_JDK="-mt $TOOLCHAIN_FLAGS"
- TOOLCHAIN_CFLAGS_JDK_CONLY="-xc99=%none -xCC -Xa -W0,-
noglobal
$TOOLCHAIN_CFLAGS" # C only
+ TOOLCHAIN_CFLAGS_JDK_CONLY="-std=c99 -xCC -W0,-noglobal
$TOOLCHAIN_CFLAGS" # C only
TOOLCHAIN_CFLAGS_JDK_CXXONLY="-features=no%except -
norunpath
-xnolib" # CXX only
TOOLCHAIN_CFLAGS_JVM="-template=no%extdef -
features=no%split_init \
-library=stlport4 -mt -features=no%except
$TOOLCHAIN_FLAGS"
