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 > >
