This fix will be delicate and may have regressions if the exact code shape (of the PopFrame-ed invokestatic call) changes.
Note that member_name_arg_or_null assumes that the value in Local#0 is a DMH; there will be asserts thrown if this fails. It also assumes that the member name held by the DMH in fact corresponds to the linker call (linkToStatic, linkToVirtual, etc.) being PopFrame-ed. In fact, the linker call might in some cases be run from another source than "aload0; getfield member". Requiring that this correspondence exist always is a new internal interface that is hard to document and verify; it may also inhibit present or future optimizations. There are other approaches that might be more robust: 1. Do not allow PopFrame to linker calls, since they (by definition) throw away their trailing MemberName argument. 2. Do not make such frames visible to the user. 3. Modify the linker primitives to store a copy of their trailing MemberName argument to a new slot in the interpreter frame and compiled frame. Be sure to populate this new slot on deoptimization. — John P.S. Sorry about the delay in commenting; I am just digging out from under JVMLS business! On Jul 30, 2013, at 10:00 AM, serguei.spit...@oracle.com wrote: > The updated webrev: > http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/7187554-JVMTI-JSR292.3/ > > Thanks, > Serguei > > On 7/29/13 10:11 PM, David Holmes wrote: >> Hi Serguei, >> >> I'm fine with restoring to what was in the first webrev. >> >> Further trimming of the JVMTI code is something the embedded folk could look >> at. It may not be worthwhile. >> >> Thanks, >> David >> >> On 30/07/2013 3:05 PM, serguei.spit...@oracle.com wrote: >>> On 7/29/13 8:22 PM, David Holmes wrote: >>>> On 30/07/2013 10:34 AM, serguei.spit...@oracle.com wrote: >>>>> Christian and David, >>>>> >>>>> Thank you for the quick review and comments! >>>>> >>>>> This is a new version of webrev: >>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/7187554-JVMTI-JSR292.2 >>>>> >>>>> >>>> >>>> Sorry. You need that guard after all - at least you do if you continue >>>> to use it in interpreterRuntime - otherwise member_name_arg_or_null >>>> will not exist: >>>> >>>> __ call_VM(rax, CAST_FROM_FN_PTR(address, >>>> InterpreterRuntime::member_name_arg_or_null), rax, rdx, rsi); >>>> >>> You are right, nice catch again. >>> Probably, it was the reason, I did not remove the #if/#endif in the >>> first place. :) >>> >>>> I'm a little surprised that the assembly code does not have that whole >>>> section guarded with INCLUDE_JVMTI - perhaps it should? >>> It should. >>> But it can be non-trivial because of other dependencies like the above. >>> To make it right, both _remove_activation_preserving_args_entry and >>> generate_earlyret_entry_for >>> must be isolated with #if INCLUDE_JVMTI. >>> Then more files have to be involved in this chain of changes: >>> interpreter/templateInterpreter.hpp >>> interpreter/templateInterpreter.hpp >>> memory/universe.hpp >>> memory/universe.cpp >>> code/codeCache.hpp >>> code/codeCache.cpp >>> . . . etc .. >>> >>> Please, note, that the HOTSWAP macro is used in many places as well. >>> I'm not sure we still need it, so that another decision is needed for it. >>> >>> So, it seems that this kind of clean up is going far beyond my fix. >>> My suggestion is to restore the "#if INCLUDE_JVMTI" in 3 >>> platform-dependent files as it was in the webrev v1. >>> I'm a little bit reluctant to open another clean-up bug for this issue >>> but maybe it is needed. >>> Please, let me know if you are comfortable with this solution. >>> >>> Thanks, >>> Serguei >>> >>>> >>>> Run this through a JPRT test build for productEmb to check that the >>>> minimal VM builds ok. >>>> >>>> David >>>> ----- >>>> >>>>> >>>>> Thanks, >>>>> Serguei >>>>> >>>>> >>>>> On 7/28/13 9:11 PM, David Holmes wrote: >>>>>> Hi Serguei, >>>>>> >>>>>> On 26/07/2013 10:14 AM, serguei.spit...@oracle.com wrote: >>>>>>> >>>>>>> Please, review the fix for: >>>>>>> bug: http://bugs.sun.com/view_bug.do?bug_id=7187554 >>>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-7187554 >>>>>>> >>>>>>> Open webrev: >>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/7187554-JVMTI-JSR292.1 >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> In the templateInterpreter code why did you put this guard on your new >>>>>> code (from x86_32 version): >>>>>> >>>>>> 1923 #if INCLUDE_JVMTI >>>>>> >>>>>> when the whole chunk of code this is situated in is specifically for >>>>>> JVMTI support >>>>>> >>>>>> 1824 // >>>>>> 1825 // JVMTI PopFrame support >>>>>> 1826 // >>>>>> >>>>>> ??? >>>>>> >>>>>> David >>>>>> ----- >>>>>> >>>>>>> >>>>>>> Summary: >>>>>>> Restore the appendix argument of a polymorphic intrinsic call >>>>>>> needed for a invokestatic re-execution after JVMTI PopFrame(). >>>>>>> >>>>>>> Description >>>>>>> When JVMTI's PopFrame removes a frame that was called via a call >>>>>>> site >>>>>>> that >>>>>>> takes an appendix and that call site is reexecuted the appendix is >>>>>>> not on >>>>>>> the stack anymore because it got removed by the adapter. >>>>>>> This fix is to detect such a case and push the appendix on the >>>>>>> stack >>>>>>> again before reexecution. >>>>>>> >>>>>>> >>>>>>> Testing: >>>>>>> UTE tests - in progress: vm.mlvm.testlist, nsk.jvmti.testlist, >>>>>>> nsk.jdi.testlist >>>>>>> >>>>>>> Thanks, >>>>>>> Serguei >>>>> >>> >
