> new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
src/share/vm/runtime/sharedRuntime.hpp
No comments.
src/share/vm/runtime/sharedRuntime.cpp
No comments.
src/cpu/sparc/vm/sharedRuntime_sparc.cpp
No comments.
src/cpu/x86/vm/sharedRuntime_x86_32.cpp
No comments.
src/cpu/x86/vm/sharedRuntime_x86_64.cpp
No comments.
On the switch from call_VM_leaf() -> call_VM(), I looked through all
the mentions of the string call_VM in the three src/cpu files. Your
change adds the first call_VM() use in all three files and the other
places that mention the string "call_VM" seem to have gone through
a great deal of trouble not to use call_VM() directly. I have no
specific thing I think is wrong, but I find this data worrisome...
Thumbs up!
Dan
On 5/13/14 3:20 AM, Staffan Larsen wrote:
On 9 maj 2014, at 20:18, serguei.spit...@oracle.com
<mailto:serguei.spit...@oracle.com> wrote:
Staffan,
This is important discovery, thanks!
The fix looks good to me.
One question below.
Thanks,
Serguei
On 5/9/14 3:47 AM, Staffan Larsen wrote:
On 8 maj 2014, at 19:05, Daniel D. Daugherty<daniel.daughe...@oracle.com>
wrote:
webrev:http://cr.openjdk.java.net/~sla/8041934/webrev.00/
src/share/vm/runtime/sharedRuntime.hpp
No comments.
src/share/vm/runtime/sharedRuntime.cpp
line 994: JRT_LEAF(int, SharedRuntime::jvmti_method_exit(
I'm not sure that JRT_LEAF is right. I would think that
JvmtiExport::post_method_exit() would have to grab one or
more locks with all the state checks it has to make...
For reference, InterpreterRuntime::post_method_exit()
is a wrapper around JvmtiExport::post_method_exit()
and it is IRT_ENTRY instead of IRT_LEAF.
Good catch!
Q: Is correct to use call_VM_leaf (vs call_VM ) in the
sharedRuntime_<arch>.cpp ?
+ __ call_VM_leaf(
+ CAST_FROM_FN_PTR(address, SharedRuntime::jvmti_method_exit),
+ thread, rax);
That is another good catch! It should probably be call_VM as you note.
The reason for all these leaf references is because we used the dtrace
probe as a template - obviously without fully understanding what we
did :-/
I have changed to code to use call_VM instead. This also required a
change from jccb to jcc for the jump (which is now longer than an
8-bit offset).
new webrev is here: http://cr.openjdk.java.net/~sla/8041934/webrev.02/
<http://cr.openjdk.java.net/%7Esla/8041934/webrev.02/>
Thanks,
/Staffan
src/cpu/sparc/vm/sharedRuntime_sparc.cpp
No comments.
src/cpu/x86/vm/sharedRuntime_x86_32.cpp
No comments.
src/cpu/x86/vm/sharedRuntime_x86_64.cpp
No comments.
I'm guessing that PPC has the same issue, but I'm presuming
that someone else (Vladimir?) will handle that…
Yes, I was hoping that I could file a follow-up bug for the platforms I didn’t
know how to fix.
Updated review:http://cr.openjdk.java.net/~sla/8041934/webrev.01/
Thanks,
/Staffan
Dan
On 5/8/14 12:06 AM, Staffan Larsen wrote:
All,
This is a fix for an assert in JVMTI that verifies that JVMTI’s internal notion
of the number of frames on the stack is correct.
When running in -Xcomp mode and enable single-stepping (or method_entry/exit)
we will revert all frames on the stack to be run by the interpreter. Only the
interpreter can send single-step and method_entry/exit. However, if one of the
frames is a native wrapper, that frame will not be reverted (presumably because
we don't know how to do that). This will cause us to miss a method_exit event
when that native frame is popped. This in turn will mess up the logic in JVMTI
that keeps track of the number of frames currently on the stack (see
JvmtiThreadState::_cur_stack_depth) and will trigger the assert.
The proposed solution is to include a method_exit event in the native wrapper
frame if interpreted mode has been enabled. This needs updates to
SharedRuntime::generate_native_wrapper() for all platforms.
Kudos to Rickard for helping me write the code.
webrev:http://cr.openjdk.java.net/~sla/8041934/webrev.00/
bug:https://bugs.openjdk.java.net/browse/JDK-8041934
The fix has been verified by running the failing test in JPRT with -Xcomp.
Thanks,
/Staffan
