Review Request: Zero and Shark fixes

Gary Benson gbenson at redhat.com
Thu Mar 31 02:21:26 PDT 2011


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/


More information about the hotspot-compiler-dev mailing list