RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects

[Hohensee, Paul]

In vframe_hp.cpp:

In jvmtiDeferredocalVariableSet::update_monitors, the calculation of lock_index

      int lock_index = val->index() - (method()->max_locals() + method()->max_stack());

looks wrong. If lock_index is supposed to be zero-based, then method()->max_stack() should be subtracted from val->index(), vis

      int lock_index = val->index() - (method()->max_locals() - method()->max_stack());

A terrifically minor nit, but using ‘l’ as the induction variable instead of ‘i’ is a bit odd. (

Otherwise fine.

Thanks,

Paul

On 12/22/17, 11:09 AM, "hotspot-compiler-dev on behalf of Tom Rodriguez" <hotspot-compiler-dev-bounces at openjdk.java.net on behalf of tom.rodriguez at oracle.com> wrote:

    Could I get some reviews?  This doesn't change the way the logic behaves 
    for the core use of JVMTI.  It just extends the encoding so that numbers 
    after the locals are interpreted as expression stack and monitor values. 
      I believe there are existing tests of the JVMTI set locals part and 
    JVMCI is the only only consumer of the monitor and expression stack part.
    
    tom
    
    Tom Rodriguez wrote:
    > JVMCI adds the ability to introspect on deoptimized frames which
    > requires early materialization of escape analyzed objects. The
    > jvmtiDeferredLocalVariableSet machinery is reused to capture the local
    > updates required for this. The existing code only updates locals,
    > leaving the stack and monitor information with out of date values. This
    > can lead to deadlocks and incorrect execution. The fix is to slightly
    > generalize jvmtiDeferredLocalVariableSet to handle expression stack and
    > monitor slots. Tested with new graal regression test
    > https://github.com/graalvm/graal/blob/7fd37bde8955780a57049964d87a51aa2407d86b/compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/HotSpotStackIntrospectionTest.java
    > and previously failing Truffle use cases. I assume the new test case
    > will come across with a normal graal update. The clean mach5 run is in
    > the bug report.
    >
    > http://cr.openjdk.java.net/~never/8192004/webrev
    > https://bugs.openjdk.java.net/browse/JDK-8192004