Hi Chris and JC,
Thank you for reviewing this change. Please review a new version of
the fix that uses
the approach Chris suggested ( disabling the single stepping during
the class resolution).
Just in case please find below the stack trace for this case when
loadClass() method is entered.
#0 SystemDictionary::load_instance_class(Symbol*, Handle,
Thread*) at open/src/hotspot/share/classfile/systemDictionary.cpp:1502
#1 SystemDictionary::resolve_instance_class_or_null(Symbol*,
Handle, Handle, Thread*) at
open/src/hotspot/share/classfile/systemDictionary.cpp:853
#2 SystemDictionary::resolve_instance_class_or_null_helper(Symbol*,
Handle, Handle, Thread*) at
open/src/hotspot/share/classfile/systemDictionary.cpp:271
#3 SystemDictionary::resolve_or_null(Symbol*, Handle, Handle,
Thread*) at open/src/hotspot/share/classfile/systemDictionary.cpp:254
#4 SystemDictionary::resolve_or_fail(Symbol*, Handle, Handle,
bool, Thread*) at
open/src/hotspot/share/classfile/systemDictionary.cpp:202
#5 ConstantPool::klass_at_impl(constantPoolHandle const&, int,
bool, Thread*) at open/src/hotspot/share/oops/constantPool.cpp:483
#6 ConstantPool::klass_at(int, Thread*) at
open/src/hotspot/share/oops/constantPool.hpp:382
#7 InterpreterRuntime::_new(JavaThread*, ConstantPool*, int) at
open/src/hotspot/share/interpreter/interpreterRuntime.cpp:234
# 8 <Stub Code>
....
Webrev: http://cr.openjdk.java.net/~dtitov/8163127/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8163127
Thanks,
Daniil
On 1/23/19, 3:53 PM, "Chris Plummer" <[email protected]> wrote:
Hi Daniil,
I don't see an explanation for why fromDepth is 1 and
afterPopDepth is 4.
currentDepth = getThreadFrameCount(thread);
fromDepth = step->fromStackDepth;
afterPopDepth = currentDepth-1;
step->fromStackDepth got setup when single stepping was
first setup for
this thread. There was also a notifyFramePop() done at this
time, but I
think that's just to catch exiting from the method you were single
stepping in, and has no bearing in the case we are looking at here,
where we area still some # of frames below where we user last
issued a
STEP_INTO. The FRAME_POP we are receiving now is not the one for
when
step->fromStackDepth was setup, but is for when we stepped into a
filtered method. I think this is what the "fromDepth >
afterPopDepth"
check is for. I think the current logic is correct for intended
handling
of a FRAME_POP event. Although your fix is probably solving the
problem,
I get the feeling it is enabling single stepping too soon in
many cases.
That many not turn up as an error in any tests, but could cause
debugging performance issues, or for the user to see spurious
single
step events that they were not expecting.
I think the bug actually occurs long before we ever get to
this point in
the code (and we should in fact not be getting here). In my last
entry
in the bug I mentioned JvmtiHideSingleStepping(), and how it is
used to
turn off single stepping while we are doing invoke and field
resolution,
but doesn't seem to be used during class resolution, which is
what we
are doing here. If it where used, then the agent would never
even see
the SINGLE_STEP when loadClass() is entered, therefore no
notifyFramePop() would be done, and we would not be relying on
this code
in handleFramePopEvent(). Instead, we would receive the next
SINGLE_STEP
event after cp resolution is complete, and we are finally
executing the
now resolved opc_new opcode.
I'm hoping Serguei and/or Alex can also comment on this,
since I think
they were dealing with JvmtiHideSingleStepping() last month.
thanks,
Chris
On 1/17/19 6:08 PM, Daniil Titov wrote:
> Please review the change that fixes JDB stepping issue for a
specific case when the single step request was initiated earlier in
the stack, previous calls were for methods in the filtered classes
(single stepping was disabled), handleMethodEnterEvent() re-enabled
stepping and the first bytecode upon entering the current method
requires resolving constant pool entry. In this case the execution
resumes in java.lang.Classloader.loadClass() and since it is also a
filtered class the single stepping is getting disabled again
(stepControl.c :593). When loadClass() exits a notifyFramePop() is
called on the loadClass() frame but due to condition fromDepth >=
afterPopDepth at stepControl.c :346 (that doesn't hold in this case,
in this case fromDepth is 1 and afterPopDepth is 4) the
notifyFramePop() fails to enable single stepping back. The fix
removes the excessive condition fromDepth >= afterPopDepth in
notifyFramePop() method (stepControl.c:346) to ensure that when a
method cal!
> led from the stepping frame (and during which we had
stepping disabled) has returned the stepping is re-enabled to
continue instructions steps in the original stepping frame.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8163127/webrev.01
> Bug: https://bugs.openjdk.java.net/browse/JDK-8163127
>
> Thanks!
> --Daniil
>
>
