[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c
From: Adam Farley8 <adam.farley () uk ! ibm ! com>
Date: 2018-02-13 12:12:16
Message-ID: OF473E9C12.3749B960-ON80258233.00411F65-80258233.00430B2E () notes ! na ! collabserv ! com
[Download RAW message or body]
This is a multipart message in MIME format.
--=_alternative 00430A3D80258233_=
Content-Type: text/plain; charset="US-ASCII"
-- Summary --
I ask for a committer to go into jdk/jdk and add one word to
make/lib/Awt2dLibraries.gmk
We need to go to line 495 and add array-bounds into the list of disabled
warnings.
So this:
DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
shift-negative-value, \
becomes this:
DISABLED_WARNINGS_gcc := clobbered implicit-fallthrough
shift-negative-value array-bounds, \
This fixes a build-breaking problem which occurs if you don't disable
errors-as-warnings on zLinux or Linux for ppcle.
P.S. Backporting this to jdk8 would be awesome, but the priority is
jdk/jdk.
-- Conversation --
> Am I understanding this correctly that it's really not tied to a gcc
version
> but a cpu architecture, so it's only really affecting s390x?
I'm saying it is tied to a combination of CPU architecture and gcc
version.
Any combination of the affected gcc versions (4.8.5, 5.4.0) and affected
platforms (zLinux, ppcle Linux) see this error.
x86 Linux is not affected, not are gcc versions equal to (or, I assume,
later
than) 7.2.1.
> Are you also saying that gcc 7.2.1 is also affected but with a different
> message? I'm fine with disabling this warning conditional on s390x, no
need
> for specific gcc versions.
I was unclear. 7.2.1 fails my unit test with a different warning, but a
build
I ran proves that this warning doesn't fail the build.
That being said, the addition of this option to a 7.2.1 test didn't seem
to
break anything, so we should be fine to just stick
"DISABLED_WARNINGS_gcc := clobbered array-bounds" into Awt2dLibraries.gmk.
> This discussion has already taken more time than it really warrants. :)
Agreed. :)
> Regarding warning chasing. I agree that we it's not feasible to chase
down every
> warning in every version of GCC, or any other toolchain, but I also
think that
> for platforms/configurations where people are actively developing
changes for
> OpenJDK, it makes sense to try to keep it clean. This helps prevent new
code from
> introducing warnings. For the configurations Oracle use, we keep a
strict -Werror
> policy because we want the code to be clean. I'm fine with other users
trying to
> keep the same standards on their configurations, but knowing that it
will be their
> responsibility to keep up to date. I also think we need to be reasonably
fine grained
> in when we disable warnings. Specifying every affected version of a
toolchain is too
> much, but if there are specific well defined limits to where the
disabling relevant,
> then I think we should use them, within reason. This also helps with
keeping track
> of why a particular warning is disabled in a future attempt to fix them.
I agree with all of this. Well put. :)
> On the other hand, if you are just building OpenJDK to produce binaries,
without
> producing and up streaming new code changes, there really isn't much
need for
> making the effort of trying to keep things clean, and trying to do so
will likely
> just end up being more work than it's worth.
> /Erik
I'm building OpenJDK to test fixes and new features, which I will
eventually contribute
to OpenJDK. I consider this to be one of those fixes. One fix at a time.
:)
Given all of this, I ask for a volunteer to raise a bug so we can
integrate this change
into JDK8 (as it's still very popular), and JDK.
10 would be great too, though I understand it's locked against all but the
worst bugs.
9 is optional, as it's soon to be replaced by 10.
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
--=_alternative 00430A3D80258233_=
Content-Type: text/html; charset="US-ASCII"
<span style=" font-size:10pt;font-family:sans-serif">-- Summary --</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">I ask for a committer
to go into jdk/jdk and add one word to make/lib/Awt2dLibraries.gmk</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">We need to go
to line 495 and add array-bounds into the list of disabled warnings.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">So this:</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">DISABLED_WARNINGS_gcc
:= clobbered implicit-fallthrough shift-negative-value, \ </span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">becomes this:</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">DISABLED_WARNINGS_gcc
:= clobbered implicit-fallthrough shift-negative-value array-bounds, \</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">This fixes a build-breaking
problem which occurs if you don't disable</span>
<br><span style=" font-size:10pt;font-family:sans-serif">errors-as-warnings
on zLinux or Linux for ppcle.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">P.S. Backporting
this to jdk8 would be awesome, but the priority is jdk/jdk.</span>
<br>
<br>
<br>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">-- Conversation
--</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">> Am I understanding
this correctly that it's really not tied to a gcc version</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> but a cpu
architecture, so it's only really affecting s390x? </span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">I'm saying it
is tied to a combination of CPU architecture and gcc version.</span>
<br><span style=" font-size:10pt;font-family:sans-serif">Any combination
of the affected gcc versions (4.8.5, 5.4.0) and affected</span>
<br><span style=" font-size:10pt;font-family:sans-serif">platforms (zLinux,
ppcle Linux) see this error.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">x86 Linux is not
affected, not are gcc versions equal to (or, I assume, later </span>
<br><span style=" font-size:10pt;font-family:sans-serif">than) 7.2.1.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">> Are you also
saying that gcc 7.2.1 is also affected but with a different </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> message?
I'm fine with disabling this warning conditional on s390x, no need </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> for specific
gcc versions.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">I was unclear.
7.2.1 fails my unit test with a different warning, but a build </span>
<br><span style=" font-size:10pt;font-family:sans-serif">I ran proves that
this warning doesn't fail the build.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">That being said,
the addition of this option to a 7.2.1 test didn't seem to </span>
<br><span style=" font-size:10pt;font-family:sans-serif">break anything,
so we should be fine to just stick </span>
<br><span style=" font-size:10pt;font-family:sans-serif">"DISABLED_WARNINGS_gcc
:= clobbered array-bounds" into Awt2dLibraries.gmk. </span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">> This discussion
has already taken more time than it really warrants. :)</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">Agreed. :)</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">> Regarding
warning chasing. I agree that we it's not feasible to chase down every
</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> warning in
every version of GCC, or any other toolchain, but I also think that </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> for platforms/configurations
where people are actively developing changes for </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> OpenJDK,
it makes sense to try to keep it clean. This helps prevent new code from
</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> introducing
warnings. For the configurations Oracle use, we keep a strict -Werror </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> policy because
we want the code to be clean. I'm fine with other users trying to </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> keep the
same standards on their configurations, but knowing that it will be their
</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> responsibility
to keep up to date. I also think we need to be reasonably fine grained
</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> in when we
disable warnings. Specifying every affected version of a toolchain is too
</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> much, but
if there are specific well defined limits to where the disabling relevant,
</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> then I think
we should use them, within reason. This also helps with keeping track </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> of why a
particular warning is disabled in a future attempt to fix them.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">I agree with all
of this. Well put. :)</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">> On the other
hand, if you are just building OpenJDK to produce binaries, without </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> producing
and up streaming new code changes, there really isn't much need for </span>
<br><span style=" font-size:10pt;font-family:sans-serif">> making the
effort of trying to keep things clean, and trying to do so will likely
</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> just end
up being more work than it's worth.</span>
<br><span style=" font-size:10pt;font-family:sans-serif">> /Erik</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">I'm building OpenJDK
to test fixes and new features, which I will eventually contribute</span>
<br><span style=" font-size:10pt;font-family:sans-serif">to OpenJDK. I
consider this to be one of those fixes. One fix at a time. :)</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">Given all of this,
I ask for a volunteer to raise a bug so we can integrate this change </span>
<br><span style=" font-size:10pt;font-family:sans-serif">into JDK8 (as
it's still very popular), and JDK. </span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">10 would be great
too, though I understand it's locked against all but the worst bugs.</span>
<br><span style=" font-size:10pt;font-family:sans-serif">9 is optional,
as it's soon to be replaced by 10.</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">Best Regards</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif">Adam Farley</span>
<br>
<br><span style=" font-size:10pt;font-family:sans-serif"><br>
Unless stated otherwise above:<br>
IBM United Kingdom Limited - Registered in England and Wales with number
741598. <br>
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6
3AU<br>
</span>
--=_alternative 00430A3D80258233_=--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic