RFR 8163127: Debugger classExclusionFilter does not work correctly with method references
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Thu Jan 24 21:57:32 UTC 2019
Hi Daniil,
Must be good enough.
Thanks!
Serguei
On 1/24/19 1:38 PM, Daniil Titov wrote:
> Hi Serguei,
>
> The testing is not fully completed yet. I ran locally jdk_jdi, vmTestbase_nsk_jdi, vmTestbase_nsk_jdb and have the same tests plus serviceability still running in Mach5. I am also starting tier1,tier2 and tier3 jobs.
>
> Best regards,
> Daniil
>
> On 1/24/19, 12:22 PM, "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote:
>
> Hi Daniil,
>
> I wonder what tests do you run to make sure nothing is broken.
>
> Thanks,
> Serguei
>
>
> On 1/24/19 11:19, Chris Plummer wrote:
> > Hi Daniil,
> >
> > Thanks for the stack track. I was just about to send an email asking
> > for it when your new RFR arrived.
> >
> > The fix looks good. I think you also need to apply it here:
> >
> > InterpreterRuntime::ldc()
> > InterpreterRuntime::anewarray()
> > InterpreterRuntime::multianewarray()
> > InterpreterRuntime::quicken_io_cc()
> >
> > I wonder if it wouldn't be better if you moved the disabling into
> > ConstantPool::klass_at_impl()
> >
> > thanks,
> >
> > Chris
> >
> > On 1/24/19 10:38 AM, Daniil Titov wrote:
> >> 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
> >> >
> >> >
> >>
> >>
> >
> >
>
>
>
>
More information about the serviceability-dev
mailing list