United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-7187554 : JSR 292: JVMTI PopFrame needs to handle appendix arguments

Details
Type:
Bug
Submit Date:
2012-07-27
Status:
Resolved
Updated Date:
2014-09-04
Project Name:
JDK
Resolved Date:
2013-08-07
Component:
hotspot
OS:
generic
Sub-Component:
jvmti
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
hs23,hs24,hs25,7u60
Fixed Versions:
hs25 (b46)

Related Reports
Backport:
Backport:
Backport:
Backport:
Backport:
Duplicate:
Duplicate:
Relates:
Relates:
Relates:

Sub Tasks

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.  We need a way to push the appendix on the stack again before reexecution.

The bug manifests as:

assert(false) failed: DEBUG MESSAGE: MemberName required for invokeVirtual etc.
                                    

Comments
7u60-critical-request justification:

This bug fix is better to be in the release because it is a part of the JSR-292 support
in the JVMTI HotSwap API (includes RedefineClasses, RetransformClasses and PopFrame).

This bug is one of 12 bug fixes that depend on each other and must be integrated
in the order:
  https://jbs.oracle.com/bugs/browse/JDK-7194607
  https://jbs.oracle.com/bugs/browse/JDK-8005128
  https://jbs.oracle.com/bugs/browse/JDK-8006542
  https://jbs.oracle.com/bugs/browse/JDK-8006546
  https://jbs.oracle.com/bugs/browse/JDK-8006731
  https://jbs.oracle.com/bugs/browse/JDK-8008511
  https://jbs.oracle.com/bugs/browse/JDK-8007037
  https://jbs.oracle.com/bugs/browse/JDK-8014288
  https://jbs.oracle.com/bugs/browse/JDK-8013945
  https://jbs.oracle.com/bugs/browse/JDK-8014052
  https://jbs.oracle.com/bugs/browse/JDK-7187554
  https://jbs.oracle.com/bugs/browse/JDK-8023004

All the fixes above have been already integrated into the JDK 8
and tested in the hotspot-rt nightly for several months.

Risk: low
  The fixes touch the JVMTI HotSwap API that includes RedefineClasses, RetransformClasses and PopFrame.
  The risk is only to introduce regressions in this part of the JVMTI implementation.
  This impacts only the debugging and profiling tools that use the JVMTI HotSwap feature.
  There are very small chances for regressions to sneak into the class file
  constant pool processing and method handles implementation.

Webrevs and reviewers:
  The 7u60 webrevs location is:
    http://javaweb.sfbay.sun.com/java/svc/ss45998/webrevs/2013/hotspot/7u_port/

  The fixes above were already passed the review process before integration
  into JDK 8. The reviewers were: twisti, jrose, coleenp, dholmes, etc.
  The 7u60 edition of fixes must be reviewed at least by jrose and twisti.

Level of effort:
  All fixes need a secondary review phase after rebase from jdk8 to 7u60 repository.

Testing coverage:
  The folllowing test suites must be run to ensure correctness of the fixes:
    JTREG tests: com/sun/jdi, java/lang/instrument
    NSK tests: vm.mlvm.testlist, nsk.jvmti.testlist, nsk.jdi.testlist, nsk.jdwp.testlist

Result of not integrating:
  The users will not be able to use HotSwap technology for debuging and profiling
  Java code that depends on the JSR-292 implementation.
  In that case the integration of these fixes will have to be requested/escalated
  in 7 updates by the tool vendors and/or customers.

                                     
2014-01-10
Adding the review comment from John Rose (that was sent after the fix was pushed) and my reply.
It is important to keep these records in a case we file a new "stability" bug on this.

-----------------------
John,

Thank you for the comments!
In fact, I was very reluctant to implement it the way as it is in the webrev.
I'm in favor of the choice #3, and think, it is much better from the stability point of view.
Restoring the MemberName slot at deoptimization is not going to cause a performance degradation.

If you and Christian are Ok with it I can file a new compiler bug to cover this issue.

Thanks,
Serguei


On 8/12/13 3:30 PM, John Rose wrote:
> 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 
                                     
2013-08-20
URL:   http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/ca0165daa6ec
User:  amurillo
Date:  2013-08-16 14:58:16 +0000

                                     
2013-08-16
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/ca0165daa6ec
User:  sspitsyn
Date:  2013-08-07 03:55:53 +0000

                                     
2013-08-07
I'm in process to send my fix for open review.
                                     
2013-07-24
SQE is ok to defer this from 7u40
                                     
2013-07-11
7u40-defer-request justification: Requesting deferral to 7u60 as the fix for this will not be ready in time for 7u40.
                                     
2013-07-09
need SQE-OK before this can be approved.
                                     
2013-07-09
The test is passed with my prototype of the fix on sparc and x86.
But there is still an issue with the compressed oops on the 64-bit platforms.
                                     
2013-07-03
A workaround for this issue is to call the JVMTI PopFrame once more.
Popping the problematic MethodHandles adapter frame "LambdaForm$DMH.invokeStatic_LLI_I()"
allows to return to the adapter invoke instruction that can be re-executed without problems.
I'm still learning how it could be possible to reload the appendix argument to the expression stack.

The following is a part of the tracing with the " -XX:+TraceInvokeDynamic" flag
just before the JVMTI PopFrame was called:

[2013-06-19T22:17:32.23] - stepBreakPopReturn.c, 60: Entering method: Lvm/mlvm/indy/func/jvmti/stepBreakPopReturn/INDIFY_Test;.bootstrap
[2013-06-19T22:17:32.23] ### TRACE 0: Lookup vm.mlvm.indy.func.jvmti.stepBreakPopReturn.INDIFY_Test; method name = greet; method type = (Object,String,int)int
[2013-06-19T22:17:32.23] set_method_handle bc=186 appendix=0xe35cc208 method_type=0xe35433b0 method=0xfc079750 
[2013-06-19T22:17:32.23] {method}
[2013-06-19T22:17:32.23]  - this oop:          0xfc079750
[2013-06-19T22:17:32.23]  - method holder:     'java/lang/invoke/LambdaForm$MH'
[2013-06-19T22:17:32.23]  - constants:         0xfc079648 constant pool [26] {0xfc07964c} for 'java/lang/invoke/LambdaForm$MH' cache=0xfc0797b0
[2013-06-19T22:17:32.23]  - access:            0x8  static 
[2013-06-19T22:17:32.23]  - name:              'linkToCallSite'
[2013-06-19T22:17:32.23]  - signature:         '(Ljava/lang/Object;Ljava/lang/Object;ILjava/lang/Object;)I'
[2013-06-19T22:17:32.23]  - max stack:         6
[2013-06-19T22:17:32.23]  - max locals:        5
[2013-06-19T22:17:32.23]  - size of params:    4
[2013-06-19T22:17:32.23]  - method size:       14
[2013-06-19T22:17:32.23]  - intrinsic id:      165 _compiledLambdaForm
[2013-06-19T22:17:32.23]  - vtable index:      -2
[2013-06-19T22:17:32.23]  - i2i entry:         0xf8c11500
[2013-06-19T22:17:32.23]  - adapters:          AHE@0x081a2e54: 0xaaaa0000 i2c: 0xf8cb33c0 c2i: 0xf8cb345d c2iUV: 0xf8cb343c
[2013-06-19T22:17:32.23]  - compiled entry     0xf8cb345d
[2013-06-19T22:17:32.23]  - code size:         18
[2013-06-19T22:17:32.23]  - code start:        0xfc07972c
[2013-06-19T22:17:32.23]  - code end (excl):   0xfc07973e
[2013-06-19T22:17:32.23]  - checked ex length: 0
[2013-06-19T22:17:32.23]  - localvar length:   0
[2013-06-19T22:17:32.23] java.lang.invoke.ConstantCallSite 
[2013-06-19T22:17:32.23]  - klass: 'java/lang/invoke/ConstantCallSite'
[2013-06-19T22:17:32.23]  - ---- fields (total size 4 words):
[2013-06-19T22:17:32.23]  - 'target' 'Ljava/lang/invoke/MethodHandle;' @8  a 'java/lang/invoke/DirectMethodHandle' (e35cf848)
[2013-06-19T22:17:32.23]  - private final 'isFrozen' 'Z' @12  true
[2013-06-19T22:17:32.23]                  -------------
[2013-06-19T22:17:32.23]   0  (0xfc028e50)  [00|ba|  201]
[2013-06-19T22:17:32.23]                  [   0xfc079750]
[2013-06-19T22:17:32.23]                  [   0x00000016]
[2013-06-19T22:17:32.23]                  [   0x33400004]
[2013-06-19T22:17:32.23]                  -------------
[2013-06-19T22:17:32.23] set_method_handle bc=232 appendix=0x00000000 (unused) method_type=0x00000000 (unused) method=0xfc070688 
[2013-06-19T22:17:32.23] {method}
[2013-06-19T22:17:32.23]  - this oop:          0xfc070688
[2013-06-19T22:17:32.23]  - method holder:     'java/lang/invoke/MethodHandle'
[2013-06-19T22:17:32.23]  - constants:         0xfc070618 constant pool [3]/preresolution {0xfc07061c} for 'java/lang/invoke/MethodHandle' (extra)
[2013-06-19T22:17:32.23]  - access:            0x1110  final native synthetic 
[2013-06-19T22:17:32.23]  - name:              'invokeBasic'
[2013-06-19T22:17:32.23]  - signature:         '(Ljava/lang/Object;Ljava/lang/Object;I)I'
[2013-06-19T22:17:32.23]  - max stack:         2
[2013-06-19T22:17:32.23]  - max locals:        0
[2013-06-19T22:17:32.23]  - size of params:    4
[2013-06-19T22:17:32.23]  - method size:       16
[2013-06-19T22:17:32.23]  - intrinsic id:      160 _invokeBasic
[2013-06-19T22:17:32.23]  - vtable index:      -2
[2013-06-19T22:17:32.23]  - i2i entry:         0xf8cce6e0
[2013-06-19T22:17:32.23]  - adapters:          AHE@0x081a2e54: 0xaaaa0000 i2c: 0xf8cb33c0 c2i: 0xf8cb345d c2iUV: 0xf8cb343c
[2013-06-19T22:17:32.23]  - compiled entry     0xf8cdcc20
[2013-06-19T22:17:32.23]  - code size:         0
[2013-06-19T22:17:32.23]  - checked ex length: 0
[2013-06-19T22:17:32.23]  - localvar length:   0
[2013-06-19T22:17:32.23]  - compiled code: nmethod   3685   45     n       java.lang.invoke.MethodHandle::invokeBasic(LLI)I (native)
[2013-06-19T22:17:32.23]  - native function:   0xfe190d00
[2013-06-19T22:17:32.23]  - signature handler: 0x00000000
[2013-06-19T22:17:32.25]                  -------------
[2013-06-19T22:17:32.25]   0  (0xfc0797c8)  [00|e8|   22]
[2013-06-19T22:17:32.25]                  [   0xfc070688]
[2013-06-19T22:17:32.25]                  [   0x00000000]
[2013-06-19T22:17:32.25]                  [   0x30400004]
[2013-06-19T22:17:32.25]                  -------------
[2013-06-19T22:17:32.25] - stepBreakPopReturn.c, 60: Entering method: Lvm/mlvm/indy/func/jvmti/stepBreakPopReturn/INDIFY_Test;.target
[2013-06-19T22:17:32.25] - stepBreakPopReturn.c, 84: Single step event: Lvm/mlvm/indy/func/jvmti/stepBreakPopReturn/INDIFY_Test; .target :1
[2013-06-19T22:17:32.25] - stepBreakPopReturn.c, 96:  Pop a frame 1

                                     
2013-06-20
Added a relation to the bug which in fact caused several tests to fail instead of this one. If the 8008511 is fixed then the following tests are passed:
  vm/mlvm/indy/func/jvmti/redefineClassInBootstrap
  vm/mlvm/indy/func/jvmti/redefineClassInTarget
  vm/mlvm/indy/func/jvmti/mergeCP_indy2manyDiff_b
  vm/mlvm/indy/func/jvmti/mergeCP_indy2manySame_b
                                     
2013-03-13
RULE vm/mlvm/indy/func/jvmti/stepBreakPopReturn Exception java.lang.NullPointerException 
                                     
2013-03-12



Hardware and Software, Engineered to Work Together