RFR 8163127: Debugger classExclusionFilter does not work correctly with method references

[Daniil Titov]

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" <chris.plummer at oracle.com> 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
    >
    >