<AWT Dev> RFR: [OpenJDK 2D-Dev] JDK-8215296 do not disable c99 on Solaris

David Holmes david.holmes at oracle.com
Wed Dec 19 21:13:58 UTC 2018 

Correction: jdk-submit does test Solaris.

David

On 18/12/2018 8:02 am, David Holmes wrote:
> 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.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:
>>>>>>
>>>>>>> -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"
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>
>>

More information about the awt-dev mailing list