[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-awt-dev
Subject:    Re: <AWT Dev> [OpenJDK 2D-Dev] RFR: JDK-8215296 do not disable c99 on Solaris
From:       Phil Race <philip.race () oracle ! com>
Date:       2018-12-13 19:34:06
Message-ID: 4e1513f7-5425-b7a3-7470-a805bc05c7b4 () oracle ! com
[Download RAW message or body]



On 12/13/18 10:07 AM, Erik Joelsson wrote:
>   * 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,

That is my reading of what it is saying .. ie in case the compiler 
supports POSIX 1003.1 or C99,
this tells it what version of the standard it would like. Since 
POSIX_SOURCE is apparently obsolete,
it presumably would be interpreted by compilers as telling it that it is 
reading "old" code written
to the earlier version of the standard. The use of the word "Just" is 
key to how I read their intent ..

-phil.

>
> 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.
>
> 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 
>>>>
>>>>
>>>> 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"
>>>>>
>>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]