<AWT Dev> RFR: JDK-8215296 do not disable c99 on Solaris
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Fri Dec 14 14:39:26 UTC 2018
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