Request for approval to sync Feb 2013 CPU fixes into jdk6-open
Andrew Hughes
gnu.andrew at redhat.com
Thu Mar 7 07:12:34 PST 2013
----- Original Message -----
> Hello,
>
> please review a fix for the build failure:
>
> http://cr.openjdk.java.net/~bae/8009641/webrev.00/
>
Looks good to me and better than what I had as the synchronized blocks were still outside the try
(so the variables could end up being null).
Ok to commit.
> Thanks,
> Andrew
>
> On 3/7/2013 5:55 PM, Seán Coffey wrote:
> > Even better if you want to push the change ? I haven't heard from
> > Andrew B. yet.
> >
> > I've logged 8009641 to track this. You could use that ID if you
> > want.
> >
> > 8009641: OpenJDK 6 build broken via 8007675 fix
> >
> > regards,
> > Sean.
> >
> > On 07/03/2013 13:51, Andrew Hughes wrote:
> >>
> >> ----- Original Message -----
> >>> Yes - you're right. That does look like an issue. Andrew Brygin
> >>> ran
> >>> pre
> >>> integration tests before pushing the changes internally and they
> >>> were
> >>> successful. However - I've traced back over the sources and what
> >>> was
> >>> run
> >>> in his test build and what he pushed to internal repo differs. (
> >>> in 2
> >>> areas) - No doubt, it's the curse of the last minute change.
> >>>
> >>> Andrew Brygin, can you log a bug and get this corrected asap ?
> >>>
> >>> Thanks for the notice Andrew!
> >>>
> >> I have a patch I can post if that would help. With that, this
> >> builds.
> >> I just need to clean up my workspace and post a webrev.
> >>
> >>> Thanks,
> >>> Sean.
> >>>
> >>>
> >>>> public short[] colorConvert(short[] src, short[] dst) {
> >>>>
> >>>> if (dst == null) {
> >>>> dst = new short
> >>>> [(src.length/getNumInComponents())*getNumOutComponents()];
> >>>> }
> >>>>
> >>>> try {
> >>>> LCMSImageLayout srcIL = new LCMSImageLayout(
> >>>> src, src.length/getNumInComponents(),
> >>>> LCMSImageLayout.CHANNELS_SH(getNumInComponents()) |
> >>>> LCMSImageLayout.BYTES_SH(2),
> >>>> getNumInComponents()*2);
> >>>>
> >>>> LCMSImageLayout dstIL = new LCMSImageLayout(
> >>>> dst, dst.length/getNumOutComponents(),
> >>>> LCMSImageLayout.CHANNELS_SH(getNumOutComponents())
> >>>> |
> >>>> LCMSImageLayout.BYTES_SH(2),
> >>>> getNumOutComponents()*2);
> >>>>
> >>>> synchronized(this) {
> >>>> LCMS.colorConvert(this, srcIL, dstIL);
> >>>> }
> >>>> } catch (ImageLayoutException e) {
> >>>> throw new CMMException("Unable to convert data");
> >>>> }
> >>>>
> >>>> return dst;
> >>>> }
> >>> On 07/03/2013 12:39, Andrew Hughes wrote:
> >>>> ----- Original Message -----
> >>>>> ----- Original Message -----
> >>>>>> I'm only the proxy here but I created the webrev from the
> >>>>>> changesets
> >>>>>> that I pushed. I don't see any difference.
> >>>>>>
> >>>>>> t4 $diff LCMSTransform.java.webrev
> >>>>>> jdk/src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java
> >>>>>> t4 $
> >>>>>>
> >>>>> Indeed. I'm still seeing the same failure using the changesets
> >>>>> pushed
> >>>>> to OpenJDK6, yet the repository itself builds. Very odd.
> >>>>>
> >>>> The copy of OpenJDK6 I was looking at wasn't up-to-date, hence
> >>>> the
> >>>> diff.
> >>>> The patched version and upstream repo are identical when
> >>>> compared
> >>>> correctly.
> >>>>
> >>>> What's baffling is how this code compiles with the javac
> >>>> compiler
> >>>> as it looks
> >>>> clearly wrong to me, which is why ecj falls over on it.
> >>>>
> >>>> try {
> >>>> LCMSImageLayout srcIL = new LCMSImageLayout(
> >>>> src, src.length/getNumInComponents(),
> >>>> LCMSImageLayout.CHANNELS_SH(getNumInComponents())
> >>>> |
> >>>> LCMSImageLayout.BYTES_SH(2),
> >>>> getNumInComponents()*2);
> >>>>
> >>>> LCMSImageLayout dstIL = new LCMSImageLayout(
> >>>> dst, dst.length/getNumOutComponents(),
> >>>> LCMSImageLayout.CHANNELS_SH(getNumOutComponents())
> >>>> |
> >>>> LCMSImageLayout.BYTES_SH(2),
> >>>> getNumOutComponents()*2);
> >>>> } catch (ImageLayoutException e) {
> >>>> throw new CMMException("Unable to convert data");
> >>>> }
> >>>>
> >>>> synchronized(this) {
> >>>> LCMS.colorConvert(this, srcIL, dstIL);
> >>>> }
> >>>>
> >>>> srcIL and dstIL are declared inside the try block so aren't
> >>>> visible
> >>>> outside it.
> >>>>
> >>>> In the other colorConvert methods, these are declared at the top
> >>>> of
> >>>> the method.
> >>>>
> >>>>>> regards,
> >>>>>> Sean.
> >>>>>>
> >>>>>> On 07/03/2013 10:59, Andrew Hughes wrote:
> >>>>>>> ----- Original Message -----
> >>>>>>>> February CPU pushes completed as reviewed in last round of
> >>>>>>>> webrevs.
> >>>>>>>>
> >>>>>>>> I'd like to push 2 extra fixes now for issues addressed in
> >>>>>>>> yesterday's
> >>>>>>>> JDK releases.
> >>>>>>>>
> >>>>>>>> webrev :
> >>>>>>>> http://cr.openjdk.java.net/~coffeys/webrev.6open.mar5/
> >>>>>>>>
> >>>>>>>> Good to push ?
> >>>>>>>>
> >>>>>>>> regards,
> >>>>>>>> Sean.
> >>>>>>>>
> >>>>>>>> On 05/03/2013 18:44, Omair Majid wrote:
> >>>>>>>>> On 03/05/2013 10:52 AM, Edvard Wendelin wrote:
> >>>>>>>>>> Hi,
> >>>>>>>>>>
> >>>>>>>>>> The change in JAXP can be viewed here:
> >>>>>>>>>> http://cr.openjdk.java.net/~joehw/jdk8/8001235/webrev/
> >>>>>>>>>> While
> >>>>>>>>>> it's
> >>>>>>>>>> generated against JDK 8, the change in 6 is identical.
> >>>>>>>>> The JAXP changes look identical to what was pushed to
> >>>>>>>>> jdk7u.
> >>>>>>>>> Looks
> >>>>>>>>> all
> >>>>>>>>> good to me!
> >>>>>>>>>
> >>>>>>>>>> I plan to push the changes today.
> >>>>>>>>> Thank you. It will be great to have all the security fixes
> >>>>>>>>> in!
> >>>>>>>>>
> >>>>>>>>> Cheers,
> >>>>>>>>> Omair
> >>>>>>>>>
> >>>>>>> I notice that what was finally committed differs from this
> >>>>>>> webrev
> >>>>>>> (in a positive
> >>>>>>> way). The version posted from the webrev failed to compile:
> >>>>>>>
> >>>>>>> 1. ERROR in
> >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java
> >>>>>>> (at line 580)
> >>>>>>> LCMS.colorConvert(this, srcIL, dstIL);
> >>>>>>> ^^^^^
> >>>>>>> srcIL cannot be resolved to a variable
> >>>>>>> ----------
> >>>>>>> 2. ERROR in
> >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java
> >>>>>>> (at line 580)
> >>>>>>> LCMS.colorConvert(this, srcIL, dstIL);
> >>>>>>> ^^^^^
> >>>>>>> dstIL cannot be resolved to a variable
> >>>>>>> ----------
> >>>>>>> 3. ERROR in
> >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java
> >>>>>>> (at line 606)
> >>>>>>> LCMS.colorConvert(this, srcIL, dstIL);
> >>>>>>> ^^^^^
> >>>>>>> srcIL cannot be resolved to a variable
> >>>>>>> ----------
> >>>>>>> 4. ERROR in
> >>>>>>> ../../../src/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java
> >>>>>>> (at line 606)
> >>>>>>> LCMS.colorConvert(this, srcIL, dstIL);
> >>>>>>> ^^^^^
> >>>>>>> dstIL cannot be resolved to a variable
> >>>>>>>
> >>>>>>> but this seems to have been fixed in the committed version,
> >>>>>>> presumably due to:
> >>>>>>>
> >>>>>>> - try {
> >>>>>>> - LCMSImageLayout srcIL = new LCMSImageLayout(
> >>>>>>> - src, src.length/getNumInComponents(),
> >>>>>>> -
> >>>>>>> LCMSImageLayout.CHANNELS_SH(getNumInComponents())
> >>>>>>> |
> >>>>>>> - LCMSImageLayout.BYTES_SH(1),
> >>>>>>> getNumInComponents());
> >>>>>>> -
> >>>>>>> - LCMSImageLayout dstIL = new LCMSImageLayout(
> >>>>>>> - dst, dst.length/getNumOutComponents(),
> >>>>>>> -
> >>>>>>> LCMSImageLayout.CHANNELS_SH(getNumOutComponents())
> >>>>>>> |
> >>>>>>> - LCMSImageLayout.BYTES_SH(1),
> >>>>>>> getNumOutComponents());
> >>>>>>> - } catch (ImageLayoutException e) {
> >>>>>>> - throw new CMMException("Unable to convert
> >>>>>>> data");
> >>>>>>> - }
> >>>>>>> + LCMSImageLayout srcIL = new LCMSImageLayout(
> >>>>>>> + src, src.length/getNumInComponents(),
> >>>>>>> + LCMSImageLayout.CHANNELS_SH(getNumInComponents()) |
> >>>>>>> + LCMSImageLayout.BYTES_SH(1),
> >>>>>>> getNumInComponents());
> >>>>>>> +
> >>>>>>> + LCMSImageLayout dstIL = new LCMSImageLayout(
> >>>>>>> + dst, dst.length/getNumOutComponents(),
> >>>>>>> + LCMSImageLayout.CHANNELS_SH(getNumOutComponents())
> >>>>>>> |
> >>>>>>> + LCMSImageLayout.BYTES_SH(1),
> >>>>>>> getNumOutComponents());
> >>>>>>>
> >>>>>>> so srcIL and dstIL are now declared at a wider scope. I'll
> >>>>>>> use
> >>>>>>> the
> >>>>>>> committed versions
> >>>>>>> in IcedTea6 HEAD.
> >>>>>>>
> >>>>>>> Cheers,
> >>>>> --
> >>>>> Andrew :)
> >>>>>
> >>>>> Free Java Software Engineer
> >>>>> Red Hat, Inc. (http://www.redhat.com)
> >>>>>
> >>>>> PGP Key: 248BDC07 (https://keys.indymedia.org/)
> >>>>> Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B
> >>>>> DC07
> >>>>>
> >>>>>
> >>>
> >
>
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
More information about the jdk6-dev
mailing list