Approved.
regards,
Sean.
On 14/01/2015 22:32, Kevin Walls wrote:
Hi,
This is a backport approval request for 8042235 into 7u. The 7u change
is a little different from 8 onwards, some discussion below that led
to the final webrev.
bug
https://bugs.openjdk.java.net/browse/JDK-8042235
webrev
http://cr.openjdk.java.net/~kevinw/8042235/webrev.01/
Thanks
Kevin
-------- Forwarded Message --------
Subject: Re: RFR: 8042235: redefining method used by multiple
MethodHandles crashes VM
Date: Wed, 14 Jan 2015 14:06:35 -0800
From: serguei.spit...@oracle.com <serguei.spit...@oracle.com>
To: Kevin Walls <kevin.wa...@oracle.com>, Coleen Phillimore
<coleen.phillim...@oracle.com>, hotspot-runtime-...@openjdk.java.net
Looks good.
Thanks,
Serguei
On 1/14/15 2:01 PM, Kevin Walls wrote:
Thanks Coleen for confirming that Handle/Oop role in jvm.cpp. That
update I just did should contain all the updates then:
http://cr.openjdk.java.net/~kevinw/8042235/webrev.01/
Thanks!
Kevin
On 14/01/2015 21:02, Coleen Phillimore wrote:
Kevin,
I have reviewed the code and I don't have any additional comments to
Serguei's comments.
On 1/14/15, 3:09 PM, serguei.spit...@oracle.com wrote:
Hi Kevin,
Thank you a lot for back porting this!
src/share/vm/classfile/javaClasses.cpp
2555 tty->print_cr("adjust_vmtarget: target = %x, new_method = %x",
target, new_method); // KJW
Is the line above a left over from your tracing and needs to be
removed?
src/share/vm/oops/cpCacheOop.cpp
- assert(m != NULL && m->is_method(), "sanity check");
+ // Secondary entry can have vfinal flag and a NULL _f2, giving
m==NULL here:
+ assert((m != NULL && !is_secondary_entry()) && m->is_method(),
"sanity check");
The assert condition seems to become stronger.
Did you really want something like this:
+ assert(is_secondary_entry() || (m != NULL && m->is_method()),
"sanity check");
src/share/vm/prims/jvm.cpp
637
instanceKlass::cast(m->method_holder())->add_member_name(new_obj);
Do we need to replace new_obj with new_obj() ?
add_member_name takes a Handle so this is ok. new_obj was changed
into a Handle with this patch.
Thanks,
Coleen
Thanks,
Serguei
On 1/14/15 10:27 AM, Kevin Walls wrote:
Hi all,
So an updated webrev in the review for 8042235:
http://cr.openjdk.java.net/~kevinw/8042235/webrev.01
On the assert I was hitting: I don't think that was really part of
this change: the constant pool cache in 7 can have these
"secondary" entries, and they can get created with the vfinal flag
set, but the _f2 field for a reference left null, which the
existing assert in cpCacheOop.cpp:619 would detect. I let it assert
now only if !is_secondary_entry().
Thanks
Kevin
On 12/01/2015 22:22, KEVIN WALLS wrote:
Hi Serguei -
Thanks oops yes I seem to have mishandled that part, I'll change it.
Unfortunately I think I still have the wierd constantpoolcache
crash I mentioned in the email just now, gotta keep looking at
that and then I'll update the webrev including this finalizer
setup call.
Thanks!
Kevin
On 12/01/2015 19:50, serguei.spit...@oracle.com wrote:
Hi Kevin,
src/share/vm/prims/jvm.cpp
645 new_obj =
instanceKlass::register_finalizer(instanceOop(new_obj_oop),
CHECK_NULL);
The above looks incorrect.
The new_obj() must be used in stead of the new_obj_oop.
Thanks,
Serguei
On 12/17/14 7:48 AM, KEVIN WALLS wrote:
Hi,
This is a request for review of a backport to 7u of 8042235.
There are a few changes from the original, hence the request here.
On JDK7 this is not a crash, but we run the wrong method, i.e.
invocation through a MethodHandle invokes the old version of the
method, if it has been redefined.
The test is different also: in jdk8 we have the ASM library, and
can visit methods and bytecodes. Here in 7, I wrote a
non-bytecode aware byte replacer method, and replaced some
literal bytecode sequence with another. As we're crafting a
method that we will rewrite, we can do something that avoids use
of the constant pool (which we haven't actually understood in
this trivial rewriter), so we rewrite some simple arithmetic,
and from the result of the method it's obvious whether we are
running the correct code.
Coleen: thanks for your earlier hints on oop / obj_field vs.
address_field.
bug
https://bugs.openjdk.java.net/browse/JDK-8042235
webrev
http://cr.openjdk.java.net/~kevinw/8042235/webrev.00/
Thanks
Kevin
