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, "[email protected]" <[email protected]> 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" <[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 >> > >> > >> >> > >
