RFR 8192004: InspectedFrame.materializeVirtualObjects only updates locals with new objects
hohensee at amazon.com
Fri Jan 19 18:25:20 UTC 2018
Plainly I missed the parens.
On 1/18/18, 8:56 AM, "Tom Rodriguez" <tom.rodriguez at oracle.com> wrote:
Hohensee, Paul wrote:
> 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());
The whole point is to linearize the numbering of locals + stack +
monitors. The locks are in the last section so we need to remove locals
+ stack. Did you miss the parentheses?
> A terrifically minor nit, but using ‘l’ as the induction variable instead of ‘i’ is a bit odd. (
That is minor. :) It's l for locals and comes from the original code
which only operates on locals. I think I'll leave it.
> Otherwise fine.
> 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 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://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_graalvm_graal_blob_7fd37bde8955780a57049964d87a51aa2407d86b_compiler_src_org.graalvm.compiler.hotspot.test_src_org_graalvm_compiler_hotspot_test_HotSpotStackIntrospectionTest.java&d=DwIGaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=z7nIdItnZDaCCWKkUI4TIwrvK5oo0Gcyu76zdeNUJ6o&m=bEBUxT7opUyW4i5zri3c18nkJeDzKdf3Xg8dA7KCgmQ&s=NnzTxQnCMbtxi8DNI18Ylx7FoXBtP9s0y3kNeEcyCHE&e=
> > 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
More information about the hotspot-compiler-dev