Hi Dan,
On 2/21/18 10:02, Daniel D. Daugherty wrote:
On 2/21/18 12:55 PM, Karen Kinnear wrote:
Dan,
Thank you for all the background digging. This is
really helpful.
Serguei - do you know what tests exist for this
behavior?
I mentioned a test that I found in the email that I attached to
my
previous reply:
Re: adding/deleting methods in JVM/TI
RedefineClasses()/RetransformClasses()
Looks like I added a JLI/JPLIS test that exercises this
"feature"
by calling the newly added method via reflection:
$ ls
/work/shared/bug_hunt/jdk8/exp/test/java/lang/instrument/RedefineMethodAdd*
RedefineMethodAddInvoke.sh
RedefineMethodAddInvokeAgent.java
RedefineMethodAddInvokeApp.java
RedefineMethodAddInvokeTarget.java
RedefineMethodAddInvokeTarget_1.java
RedefineMethodAddInvokeTarget_2.java
This test was added for the following bug:
6667089 3/3 multiple redefinitions of a class break
reflection
All these little pieces of information keep coming back at
me...
I've added a comment to the JDK-8192936 with the most important
fragments
from this email exchange in a hope, it will stop this bad loop. :)
Dan
I haven't checked to see if that test is still there or if any
additional tests have been added.
Thank you for pointing to this!
Also, there is a test for DELETE method in the
open/test/jdk/java/lang/instrument:
RedefineMethodDelInvokeAgent.java
RedefineMethodDelInvokeApp.java
RedefineMethodDelInvoke.sh
RedefineMethodDelInvokeTarget_1.java
RedefineMethodDelInvokeTarget_2.java
RedefineMethodDelInvokeTarget.java
Thanks,
Serguei
Dan
The way I read the source code - we currently
allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read
that correctly?
With the current implementation, I am not sure if
deletion works for private methods - do we
have a test for that? Or could you add one as part
of this exercise?
Today we create a vtable entry for private methods
(my misunderstanding ~ 2006ish). After discussions
with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an
assertion
assert(!old_method->is_deleted(), “vtable
methods may not be deleted”)
I may have read the code incorrectly - but I would
expect to hit this assertion if you had a private
method you were deleting that was not final and
not static.
option 1) we could explicitly tighten the
restrictions to match what we have implemented
option 2) we could make this work by changing
klassVtable.cpp::update_inherited_vtable
handling of private fields to be done the way it
handles final fields.
option 3) I read the code incorrectly?
thanks,
Karen
On 2/21/18 2:45 AM, serguei.spit...@oracle.com
wrote:
On 2/20/18 23:01, David Holmes wrote:
On 21/02/2018 4:50
PM, serguei.spit...@oracle.com
wrote:
Hi Karen and
David,
On 2/20/18 19:52, David Holmes wrote:
Hi Karen,
On 21/02/2018 1:54 AM, Karen Kinnear wrote:
Folks,
As part of the Valhalla EG discussions for
JVMTI changes for nestmates (thank you
Serguei and David),
IBM brought up a request that we update the
JVMTI documentation to reflect that we allow
addition
of private methods.
Is there a reason we do not document this?
I’m inviting those who were involved at the
time - please include
others if needed.
I support documenting this in the JVMTI spec and
had a plan to fix it in 11.
However, it is not clear to me yet if we have a
consensus on it.
I would like to see a detailed analysis of the
implications of allowing this. I _think_ it is
safe but ...
Valid concern.
Also, I'd love to collect more details on the
initial motivation to relax the JVMTI spec.
Most likely we had no CCC/CSR filed on this change.
This issue is
tracked by:
https://bugs.openjdk.java.net/browse/JDK-8192936
"RI does not follow the JVMTI RedefineClasses
spec that is too strict in the definition"
Yes, this is the one.
Thank you, David, for posting the link.
As I wrote
there ... It is not at all clear how
JDK-6404550 morphed into "Permit the adding or
deleting of private final/static methods with
redefine" - nor why those changes failed to
make any change to the spec itself. It is also
unclear whether the add/delete is restricted
to final/static methods or any private method?
I can see that the intent was to only allow
something that would not perturb the vtable
for existing instances.
I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a
different bug than the one in the Bugtraq
system?
The JDK-6404550 in JIRA has a different
synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
Cannot implement late attach in NetBeans
Profiler due to missing functionality in JVMTI
Digging deeper ... to fix the problem described in
that bug they augmented JVM TI to allow private
method redefinition as an alternate to the "native
rebinding" technique that had been used
previously. See the final comment in:
https://bugs.openjdk.java.net/browse/JDK-6341303
"JVMTI Spec: Need a way how to rebind Object.wait
and Thread.sleep with late attach"
which was closed as a duplicate.
Thank you for the point.
This explains it.
It seems, the bug synopsis was changed at some
moment.
The synopsis for 6404550 has never changed. Here's
the subject line when
it was created on 2006.03.27:
> CR 6404550 *HOT* Created P1 hotspot/jvmti
Cannot implement late attach in NetBeans Profiler
due to missing functionality in JVMTI
I think the confusion arises over comments like this
in 6341303:
BT2:EVALUATION
This can now be accomplished with Java
programming language instrumentation, via:
6404550: missing late attach (JVM TI
redefine) functionality
Permit the adding or deleting of
private final/static methods with redefine
Closing this bug as a duplicate.
That's just Robert's style for an sccs delta
comment:
D 1.65.2.3 06/04/25 23:36:35 rfield 140 139
00023/00013/03263
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine)
functionality
Add/delete private methods, continued:
changes per review
Back in the ancient past we tried to include some
brief
info about the change in the delta comment. This was
one of many
deltas associated with 6404550.
Please see the attached email that I sent on
2012.12.17 about the
history behind this issue... (sent to Karen, Mikael
V, and Serguei)
It seems I forwarded that same email to Coleen,
Markus G and Serguei
back on 2014.05.20. Since Markus is on that thread,
it must have had
something to do with research about JFR...
I need to do a detailed read thru my e-mail archive
for 6404550 to
see if I can spot some clues about why we didn't do
a spec update.
Dan
Thanks,
Serguei
David
-----
Thanks,
Serguei
--
David
thanks,
Karen
<Attached
Message.eml>
|