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

Wed Jan 24 23:55:48 UTC 2018

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.

/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