RFR: JDK-8074859 Turn on warnings as error

[David Holmes]

Hi Volker,

On 20/05/2015 7:50 PM, Volker Simonis wrote:
> Hi,
>
> I'm a little confused about this change. I finally found some time to
> look at it, because it makes most of our nightly jdk9/dev builds fail.
> Now I've looked at the 'simplest possible failure' which happens on
> linux/amd64. The failure is the following:
>
> /net/usr.work/openjdk/nb/linuxx86_64/nightly/jdk9/jdk/src/java.base/share/native/libjava/jdk_util.c:79:5:
> error: comparison of unsigned expression >= 0 is always true
> [-Werror=type-limits]
>
> First I thought this is because we are using a different version of
> GCC but I've just verified that this warning which is now turned into
> an error happens with all version of GCC starting from at least 4.3 up
> to 4.8.
>
> So do you do your regular builds on linux/amd64 by default with
> "--disable-warnings-as-errors" now?

In jdk/make/lib/CoreLibraries.gmk there should be this fragment:

$(eval $(call SetupNativeCompilation,BUILD_LIBJAVA, \
     LIBRARY := java, \
     OUTPUT_DIR := $(INSTALL_LIBRARIES_HERE), \
     SRC := $(LIBJAVA_SRC_DIRS), \
     OPTIMIZATION := HIGH, \
     CFLAGS := $(CFLAGS_JDKLIB) \
         $(LIBJAVA_CFLAGS), \
     DISABLED_WARNINGS_gcc := type-limits format-nonliteral, \

which disables the warning you are seeing.

David
-----

> Reading you first mail, the code should compile fine on the build
> systems used internally at Oracle and according to
> https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms
> they should use gcc 4.8.2.
>
> So what's the problem here or am I doing something wrong?
>
> Thank you and best regards,
> Volker
>
>
> On Sat, May 9, 2015 at 6:30 AM, Tim Bell <tim.bell at oracle.com> wrote:
>> Magnus:
>>
>> Looks good to me as well.
>>
>> Tim
>>
>>
>> On 05/08/15 01:32, Erik Joelsson wrote:
>>>
>>> Looks good.
>>>
>>> /Erik
>>>
>>> On 2015-05-08 09:57, Magnus Ihse Bursie wrote:
>>>>
>>>> On 2015-04-20 09:02, Erik Joelsson wrote:
>>>>>
>>>>> Looks good to me.
>>>>
>>>> Thanks Erik.
>>>>
>>>> Unfortunately, I never got round to pushing this. In the meantime, the
>>>> codebase evolved, and I had to add a couple of more disabled warnings. I
>>>> also modified the help text on a failed build slightly.
>>>>
>>>> Here's the updated webrev:
>>>> http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors/webrev.01
>>>>
>>>> /Magnus
>>>>
>>>>
>>>>>
>>>>> /Erik
>>>>>
>>>>> On 2015-04-17 14:52, Magnus Ihse Bursie wrote:
>>>>>>
>>>>>> With JDK-8074096, the number of warnings in the product was reduced to
>>>>>> a minimum. This enables the next step, which is turning on the respective
>>>>>> compiler flags that turns warnings into errors. In the long run, this is the
>>>>>> only way to keep the warnings from creeping back.
>>>>>>
>>>>>> Even with JDK-8074096, the product does not build 100% warning free.
>>>>>> This is due to some warnings that cannot be disabled, or (in one case) where
>>>>>> C and C++ code is mixed, and warnings for both languages cannot be used. A
>>>>>> system similar to the one introduced in JDK-8074096 is therefore needed, in
>>>>>> which individual libraries can be exempted from this flag, until such
>>>>>> warnings are fixed. A library can thus disable warnings as errors with
>>>>>> WARNINGS_AS_ERRORS := false, or (better) use a toolchain-specific version,
>>>>>> e.g WARNINGS_AS_ERRORS_gcc := false. This is intended as a temporary
>>>>>> measure, though. The long-term solution is reasonably to fix the warnings
>>>>>> and remove that argument.
>>>>>>
>>>>>> Also, different versions of compilers can generate a different set of
>>>>>> warnings. It is therefore necessary to be able to turn off this globally.
>>>>>> Therefor a new flag for configure is introduced:
>>>>>> --disable-warnings-as-errors.
>>>>>>
>>>>>> While the code compiles without errors on the build systems used
>>>>>> internally at Oracle, this might not be the case on other combinations of
>>>>>> operating system versions and toolchain versions. To facilitate for
>>>>>> unexpecting developers, a help message is added if the build fails, that
>>>>>> suggests using --disable-warnings-as-errors. This solution was chosen as a
>>>>>> compromise between the "hard core" solution of turning on warnings as errors
>>>>>> by default for anyone, and the cowar... erm, conservative solution of
>>>>>> checking if the compiler versions exactly match what's used inside Oracle
>>>>>> (and therefore regularly tested), and only turn it on in that case.
>>>>>>
>>>>>> Similarly to JDK-8074096, I intend to file follow-up bugs for each
>>>>>> individual library that got a WARNINGS_AS_ERRORS_* := false.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8074859
>>>>>> WebRev for top:
>>>>>> http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-top/webrev.01
>>>>>> WebRev for jdk:
>>>>>> http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-jdk/webrev.01
>>>>>>
>>>>>> Some comments:
>>>>>>
>>>>>> * I needed to add a few more DISABLED_WARNINGS. For windows, this is
>>>>>> most likely due to the recent compiler change. For other libraries, I'm not
>>>>>> sure, but it might well be the result of recent changes that has introduced
>>>>>> new warnings. If so, it highlights the need of this patch to keep the build
>>>>>> warning free.
>>>>>>
>>>>>> * For a few libraries and toolchains, there is *both*
>>>>>> WARNINGS_AS_ERRORS := false and a DISABLED_WARNINGS list. This is the case
>>>>>> if not all warnings are possible to disable.
>>>>>>
>>>>>> * I have removed some incorrect uses of SHARED_LIBRARY_FLAGS. This is
>>>>>> included in our JDK LDFLAGS, so it should not be set separately, and
>>>>>> definitely not as CFLAGS. (This caused compiler warnings, which now turned
>>>>>> into errors.) However, a more suitable long-term solution is probably to
>>>>>> move the knowledge of how to create shared libraries more specifically into
>>>>>> SetupNativeCompilation, and not set it as part of the JDK flags.
>>>>>>
>>>>>> /Magnus
>>>>>
>>>>>
>>>>
>>>
>>