Review Request: Zero and Shark fixes

[Dr Andrew John Hughes]

On 31/03/2011, Gary Benson <gbenson at redhat.com> wrote:
> Dr Andrew John Hughes wrote:
>> On 30 March 2011 15:49, Christian Thalinger
>> <christian.thalinger at oracle.com> wrote:
>> > On Mar 30, 2011, at 4:38 PM, Dr Andrew John Hughes wrote:
>> > > 1. Makefile changes which make sure the Shark files are once
>> > > again compiled (and which I believe would need to be reviewed
>> > > by Kelly anyway)
>> > > 2. The trivial #ifndef addition which fixes a regression
>> > > introduced by the recent HotSpot security fix.
>> > > 3. The fix for the bug Gary mentioned which appears to mean
>> > > API changes (I don't completely understand this bit).
>> > >
>> > > The first two have been tested in IcedTea6 and correspond to
>> > > regressions introduced by identifiable changesets.
>> > > I'm not sure of the impact of the third.
> ...
>> > Alright, let's split the work :-)
>> >
>> > Andrew, can you take care of 1. and send it to build-dev?  I take
>> > the rest of the patch (2. and 3.) and push it as the one CR I
>> > created.  I don't think it makes much sense to have an extra CR
>> > for 2.
>>
>> I'm happy to handle 1 (as I say, I was going to get around to it
>> anyway).  I would prefer 2 was as without it, Shark is currently
>> broken in OpenJDK6.  1 won't be an issue until hs20 is imported into
>> OpenJDK6.  I don't know enough about 3, which is why I'm sceptical
>> about including that in a fix for a current Shark build failure in
>> OpenJDK6.  So if you do combine them, I'll probably split them for
>> 6 anyway, to be on the safe side.
>
> Ok, I looked a little further into 3.  The root cause was this:
>
>   changeset: 1611:136b78722a08
>   user:      jrose
>   date:      Wed Jun 09 18:50:45 2010 -0700
>   summary:   6939203: JSR 292 needs method handle constants
>
> It two new internal instructions, _fast_aldc and _fast_aldc_w, but
> it added them *before* _return_register_finalizer, which broke the
> instructions table in bytecodeInterpreter.cpp.  It meant that when
> the C++ interpreter saw _return_register_finalizer it would execute
> opc_default, which should have thrown an error but for the piece of
> the ARM interpreter which hid the error by rewriting the instruction
> to a plain _return.  I didn't see it before because I was doing
> debug builds, and the C++ interpreter only uses the instructions
> table for product builds.
>
> The bottom line is, if you see _fast_aldc in bytecodes.hpp then Zero
> won't work correctly without this patch.
>
> Cheers,
> Gary
>
> --
> http://gbenson.net/
>

Ok, that change is in hs19 and thus OpenJDK6 already:

$ hg log -R ../../jdk6/hotspot -k 6939203
changeset:   1805:136b78722a08
parent:      1795:49fac4acd688
user:        jrose
date:        Wed Jun 09 18:50:45 2010 -0700
summary:     6939203: JSR 292 needs method handle constants

I'm still wary that the fix is untested, but my main worry (that it
fixed a newer change that wasn't in hs20) is alleviated.

If Zero is broken without this, it needs to go in the 1.9 (hs19
non-default), 1.10 (hs19 default, hs20 non-default) and HEAD (hs20
default, hs19 non-default) branches of IcedTea6.
-- 
Andrew :-)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net

PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8