RFR: [OpenJDK 2D-Dev] JDK-8215296 do not disable c99 on Solaris (original) (raw)

Baesken, Matthias matthias.baesken at sap.com
Mon Dec 17 13:12:12 UTC 2018

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 .

Thanks, Matthias

----------------------------------------------------------------------

Message: 1 Date: Fri, 14 Dec 2018 15:39:26 +0100 From: Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> To: Erik Joelsson <erik.joelsson at oracle.com>, build-dev <build-dev at openjdk.java.net>, "awt-dev at openjdk.java.net" <awt-dev at openjdk.java.net>, 2d-dev <2d-dev at openjdk.java.net> Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8215296 do not disable c99 on Solaris Message-ID: <5874d10e-db2d-8681-a54b-a1eeb6e45994 at 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.joelsson at oracle.com_ _> <mailto:erik.joelsson at oracle.com>>: > >> >> On 2018-12-13 02:11, Magnus Ihse Bursie wrote: >>> >>>> -DXPG6 >>>> >>>> ?? >>> 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/featuretests.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(STDCC99) && (defined(XOPENORPOSIX) && >>> !defined(XPG6)) >>> #error "Compiler or options invalid for pre-UNIX 03 X/Open _>>> applications _ >>> and pre-2001 POSIX applications" _>>> #elif !defined(STDCC99) && _ _>>> (defined(XOPENORPOSIX) && 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 featuretests.h with XOPENORPOSIX 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 POSIXSOURCE >> # define POSIXSOURCE 1 /* Just the POSIX 1003.1 and C89 APIs */ >> #endif >> >> This in turn triggers XOPENORPOSIX to be defined in >> /usr/include/sys/featuretests.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 XOPENSOURCE=600 or >> POSIXCSOURCE=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 POSIXCSOURCE 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 POSIXCSOURCE 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 ($(OPENJDKTARGETOS), solaris) LIBSPLASHSCREENCFLAGS += -DPOSIXCSOURCE=200112L - DXOPENSOURCE=600 endif instead made us fail with: open/src/java.desktop/unix/native/libsplashscreen/splashscreensys.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 POSIXCSOURCE=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 @@ >>>>>>> TOOLCHAINCFLAGS="-errshort=tags" >>>>>>> >>>>>>> TOOLCHAINCFLAGSJDK="-mt $TOOLCHAINFLAGS" >>>>>>> - TOOLCHAINCFLAGSJDKCONLY="-xc99=%none -xCC -Xa -W0,- noglobal >>>>>>> $TOOLCHAINCFLAGS" # C only >>>>>>> + TOOLCHAINCFLAGSJDKCONLY="-std=c99 -xCC -W0,-noglobal >>>>>>> $TOOLCHAINCFLAGS" # C only >>>>>>> TOOLCHAINCFLAGSJDKCXXONLY="-features=no%except - norunpath >>>>>>> -xnolib" # CXX only >>>>>>> TOOLCHAINCFLAGSJVM="-template=no%extdef - _features=no%splitinit _ >>>>>>> -library=stlport4 -mt -features=no%except >>>>>>> $TOOLCHAINFLAGS" >>>>>> >>>>> >>>

More information about the build-dev mailing list