[OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[David Holmes]

Hi Magnus,

On 25/01/2018 9:55 AM, Magnus Ihse Bursie wrote:
> I'm not sure I'm able to follow where this discussion is heading, but at 
> the very least I'd like to chime in on the patch below:
> 
> I don't like how the entire DISABLED_WARNINGS_gcc line is lifted out. A 
> better solution is something like this:
> 
> ifeq ($(CC_VERSION_NUMBER), 4.8.5)
>   #Turn off array-bounds warnings for libjava compilation because jchuff.c
>   #fails to compile on gcc 4.8.5 with a warning that an array is potentially
>   #out of bounds. This kills a warnings=errors build.
>   LIBJAVAJPEG_DISABLED_WARNINGS_gcc485 := array-bounds
> endif
> ...
>      DISABLED_WARNINGS_gcc := clobbered 
> $(LIBJAVAJPEG_DISABLED_WARNINGS_gcc485), \
> 
> I also can't see this going into anything else than the current 
> mainline, jdk/jdk i.e. JDK 11. Since the issue is trivially avoided by 
> using --disable-warnings-as-errors, I hardly see that it warrants a 
> backport.

It's pointless in JDK 11 as we're not going to claim support for gcc 4.8.5.

The issue is that out-of-the-box if you are building 8u or 9 and use gcc 
4.8.5, which is supposed to be a valid compiler to use based on the 
wiki, then it fails because of this gcc bug, unless you disable 
warnings-as-errors. So either we put in a workaround for the bug or we 
update to the build docs (and/or the wiki) to make it very clear that 
you may need to disable warnings-as-errors (something there is no 
mention of in the build doc in 9).

David
-----

> /Magnus
> 
> On 2018-01-23 14:44, Adam Farley8 wrote:
>> Hi All,
>>
>> All: I think I responded to everyone below. Please could a committer or
>> author raise a bug and, if people are happy with this change, line it up
>> for contribution to JDKs 8-11 (assuming 4.8.5 is still the recommended 
>> gcc
>> for JDK10 and 11 on SUSE sles)?
>>
>> Erik: One toolchain-specific change, as requested. This was tested on 
>> JDK9,
>> so please apply an offset of -1 to the line numbers before applying to 
>> JDK10.
>>
>> ----------------------- Start of Diff -----------------------
>>
>> diff --git a/make/lib/Awt2dLibraries.gmk b/make/lib/Awt2dLibraries.gmk
>> --- a/make/lib/Awt2dLibraries.gmk
>> +++ b/make/lib/Awt2dLibraries.gmk
>> @@ -482,6 +482,14 @@
>>    BUILD_LIBJAVAJPEG_HEADERS := $(addprefix -I, $(LIBJAVAJPEG_SRC))
>>  endif
>>
>> +LIBJAVAJPEG_DISABLED_WARNINGS_gcc := clobbered
>> +ifeq ($(CC_VERSION_NUMBER), 4.8.5)
>> +  #Turn off array-bounds warnings for libjava compilation because 
>> jchuff.c
>> +  #fails to compile on gcc 4.8.5 with a warning that an array is 
>> potentially
>> +  #out of bounds. This kills a warnings=errors build.
>> +  LIBJAVAJPEG_DISABLED_WARNINGS_gcc := clobbered array-bounds
>> +endif
>> +
>>  $(eval $(call SetupNativeCompilation,BUILD_LIBJAVAJPEG, \
>>      LIBRARY := javajpeg, \
>>      OUTPUT_DIR := $(INSTALL_LIBRARIES_HERE), \
>> @@ -491,7 +499,7 @@
>>      CFLAGS := $(CFLAGS_JDKLIB) $(BUILD_LIBJAVAJPEG_HEADERS) \
>>          $(LIBJAVA_HEADER_FLAGS) \
>>  -I$(SUPPORT_OUTPUTDIR)/headers/java.desktop, \
>> -    DISABLED_WARNINGS_gcc := clobbered, \
>> +    DISABLED_WARNINGS_gcc := $(LIBJAVAJPEG_DISABLED_WARNINGS_gcc), \
>>      MAPFILE := $(JDK_TOPDIR)/make/mapfiles/libjpeg/mapfile-vers, \
>>      LDFLAGS := $(LDFLAGS_JDKLIB) \
>>          $(call SET_SHARED_LIBRARY_ORIGIN), \
>>
>> ----------------------- End of Diff -----------------------
>>
>> Phil: I've changed the title as asked and supplied the diff above. 
>> However,
>> I can't upload files to cr.openjdk.java.net, nor can I create bugs 
>> myself.
>> I understand that authors and committers can, but I don't have those
>> privileges yet. Be nice if I did. :)
>>
>> John: I read your email, and I understand your position. I disagree 
>> with it,
>> but I understand it. 4.8.5 is an old version of gcc, but right now it 
>> is the
>> listed gcc version for SUSE sles on intel, ppc, ppcle, and zLinux. 
>> Even if
>> this is not the case for JDK 10 or 11, we should ensure this fix is fully
>> propagated to ensure consistent behaviour.
>>
>> That is my position.
>>
>> David: Thanks for the URL. I agree with your position on 4.8.5 gcc 
>> needing
>> to compile OpenJDK without errors or special options. :)
>>
>> Best Regards
>>
>> Adam Farley
>>
>>
>>
>> From: Phil Race <philip.race at oracle.com>
>> To: Adam Farley8 <adam.farley at uk.ibm.com>, 2d-dev at openjdk.java.net
>> Cc: build-dev <build-dev at openjdk.java.net>
>> Date: 18/01/2018 19:16
>> Subject: Re: [OpenJDK 2D-Dev] [PATCH] Build fails to compile jchuff.c
>> ------------------------------------------------------------------------
>>
>>
>>
>> Try again with build-dev cc'd ..
>>
>> -phil.
>>
>> On 01/18/2018 11:14 AM, Phil Race wrote:
>> I agree with what Erik said on build-dev that being specific about the 
>> tool chain
>> and the reason are worthwhile and important. We've done that in 
>> similar cases.
>>
>> Also these review threads usually should have a subject like
>> RFR: <BUG ID>: <Bug Synopsis>
>>
>> which means you first need a bug id .. the patch can't be pushed 
>> without one anyway.
>>
>> Then the patch should be an in-line diff or a webrev hosted on 
>> cr.openjdk.java.net.
>>
>> I think in-line would be OK for this small change.
>>
>> -phil.
>>
>> On 01/17/2018 09:30 AM, Adam Farley8 wrote:
>> Hi All,
>>
>> Under these circumstances, jchuff.c will not compile:
>>
>> Platform: zLinux (s390x)
>> Release: JDK9 (may affect other JDKs).
>> GCC Version: 4.8.5
>> Notes: --disable-warnings-as-errors suppresses this error.
>>
>> The error is:
>>
>> /home/adamfarl/hotspot/jdk9/jdk/src/java.desktop/share/native/libjavajpeg/jchuff.c:
>> In function 'jGenOptTbl':
>> /home/adamfarl/hotspot/jdk9/jdk/src/java.desktop/share/native/libjavajpeg/jchuff.c:808:18:
>> error: array subscript is below array bounds [-Werror=array-bounds]
>>       while (bits[j] == 0)
>>                  ^
>>
>> This is a continuation of a conversation in the build-dev mailing 
>> list, if anyone wants to
>> check the history.
>>
>> The short version is that, while you *can* suppress the problem by adding
>> --disable-warnings-as-errors to your configure step, I posit that a 
>> builder shouldn't
>> have to.
>>
>> Various solutions were debated. One involves changing Awt2dLibraries.gmk.
>>
>> Basically you change line 494 to this:
>>
>>     DISABLED_WARNINGS_gcc := clobbered array-bounds, \
>>
>> I'm running a build now to check that works, but basically we should 
>> end up with a
>> -Wno-array-bounds on the gcc compile command for jchuff.c, thereby 
>> ignoring the warning.
>>
>> A smarter variant involves checking for that specific version of the 
>> gcc, but that seems
>> wordy to me for this problem. Keeping it simple. :)
>>
>> Thoughts?
>>
>> Best Regards
>>
>> Adam Farley
>>
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with 
>> number 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
>> 3AU
>>
>>
>>
>>
>>
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with 
>> number 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>