[OpenJDK 2D-Dev] [REGRESSION?] Build warnings at jdhuff.c with GCC 6

[Erik Joelsson]

Hello,


On 2016-08-23 18:12, Phil Race wrote:
> On 08/23/2016 08:47 AM, Erik Joelsson wrote:
>> Hello,
>>
>> I do agree that maintaining the list of disabled warnings will be
>> impossible unless we have a structured way of tracking for which
>> compiler versions we disable what. Ideally we should be able to easily
>> add conditions for when certain warnings should be disabled. We are
>> unfortunately lacking that today and at least I don't have the
>> bandwidth to fix that anytime soon.
>>
>> The official compilers are only really official for Oracle. The
>> OpenJDK can (and should) be buildable with a much wider range of
>> compiler versions.
> I agree there. This is fortunately not an "unbuildable" situation.
> The only other option I can think of which may or may not be palatable
> is to explicitly
> check the compiler version and add that disabled warning only for that
> exact compiler version.
> There'd still be some maintenance as that compiler version became either
> official .. or obsolete ..
>
> Is there precedent (or any kind of support) for that ?
What I had in mind was a structured way of adding conditionals for some  
kind of ranges of compiler versions, or at least something like 6.*, or  
"greater than 4.9.3". It's pretty simple today to check for exact  
compiler versions but then we end up with a lot of changes when minor  
versions are bumped. I don't think that would be worth it.

In this particular case is shift-negative-value a new warning in GCC 6?  
If that's the case it doesn't actually hurt adding it since GCC is nice  
enough to not complain about unknown warning tags. If we do, just make  
sure to specify in a comment that it's specific to GCC version 6+.

/Erik
> -phil.
>
>> Luckily we have the workaround of setting --disable-warnings-as-errors
>> when this situation occurs.
>>
>> For reference, the compilers Oracle uses are listed in
>> README-builds.md in the top level directory in the forest.
>>
>> So for now at least, I think Phil is right.
>>
>> /Erik
>>
>> On 2016-08-23 17:11, Philip Race wrote:
>>> Erik .. please chime in if you disagree with the following
>>> The goal here is to have no warnings with the *official* compilers.
>>> If you are using something else and get warnings then either fix
>>> the *source* or else you need to use --disable-warning-as-errors.
>>>
>>> Otherwise we'll be suppressing the warnings for a whole range
>>> of compilers and no one will know what the set is and these
>>> bugs will become 'unfixable' and a continual chore of churn.
>>> It will be something we should address as we move *official* compilers.
>>>
>>> In fact the warning you want to suppress is one I just got rid of (the
>>> suppression)
>>> http://hg.openjdk.java.net/jdk9/client/jdk/rev/d4f7412a51d2
>>> I don't think we want to add it back unless the *official* compilers
>>> object
>>> .. I am sure that official list is documented somewhere.
>>>
>>> -phil.
>>>
>>>
>>> On 8/23/16, 6:10 AM, Yasumasa Suenaga wrote:
>>>> Hi all,
>>>>
>>>> I've fixed several warnings when we build OpenJDK with GCC 6 in
>>>> JDK-8160294.
>>>> However, I encountered shift-negative-value warnings at jdhuff.c on my
>>>> Fedora 24 (GCC 6.1.1):
>>>> --------------
>>>> /home/ysuenaga/OpenJDK/hs/jdk/src/java.desktop/share/native/libjavajpeg/jdhuff.c:458:13:
>>>>
>>>> warning: left shift of negative value [-Wshift-negative-value]
>>>> { 0, ((-1)<<1) + 1, ((-1)<<2) + 1, ((-1)<<3) + 1, ((-1)<<4) + 1,
>>>> ^~
>>>> /home/ysuenaga/OpenJDK/hs/jdk/src/java.desktop/share/native/libjavajpeg/jdhuff.c:458:28:
>>>>
>>>> warning: left shift of negative value [-Wshift-negative-value]
>>>> { 0, ((-1)<<1) + 1, ((-1)<<2) + 1, ((-1)<<3) + 1, ((-1)<<4) + 1,
>>>> ^~
>>>> /home/ysuenaga/OpenJDK/hs/jdk/src/java.desktop/share/native/libjavajpeg/jdhuff.c:458:43:
>>>>
>>>> warning: left shift of negative value [-Wshift-negative-value]
>>>> { 0, ((-1)<<1) + 1, ((-1)<<2) + 1, ((-1)<<3) + 1, ((-1)<<4) + 1,
>>>> :
>>>> --------------
>>>>
>>>> I think these warnings are available from JDK-8074827.
>>>> This issue enables warnings to fix in the source code.
>>>> However, in review of JDK-8160294, I heared that warnings in IJG JPEG
>>>> library should just be suppressed:
>>>>
>>>> http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-June/004497.html
>>>>
>>>>
>>>> So I think they should be suppressed and we should fix as below:
>>>> --------------
>>>> diff -r b548b8217d8c make/lib/Awt2dLibraries.gmk
>>>> --- a/make/lib/Awt2dLibraries.gmk Mon Aug 22 19:28:36 2016 -0700
>>>> +++ b/make/lib/Awt2dLibraries.gmk Tue Aug 23 22:08:44 2016 +0900
>>>> @@ -490,7 +490,7 @@
>>>> CFLAGS := $(CFLAGS_JDKLIB) $(BUILD_LIBJAVAJPEG_HEADERS) \
>>>> $(LIBJAVA_HEADER_FLAGS) \
>>>> -I$(SUPPORT_OUTPUTDIR)/headers/java.desktop, \
>>>> - DISABLED_WARNINGS_gcc := clobbered, \
>>>> + DISABLED_WARNINGS_gcc := clobbered shift-negative-value, \
>>>> MAPFILE := $(JDK_TOPDIR)/make/mapfiles/libjpeg/mapfile-vers, \
>>>> LDFLAGS := $(LDFLAGS_JDKLIB) \
>>>> $(call SET_SHARED_LIBRARY_ORIGIN), \
>>>> --------------
>>>>
>>>> If it is correct, I file it to JBS and upload webrev.
>>>> Do you think about it?
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>