RFR(XS) 8252593: [TESTBUG] serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java failed with JVMTI_ERROR_INVALID_SLOT

[Reingruber, Richard]

Hi David,

> > 
> > Furthermore I have corrected the type of the parameter waitCycles of
> > Java_GetLocalWithoutSuspendTest_notifyAgentToGetLocal() to be jint. Now it
> > matches the declaration in GetLocalWithoutSuspendTest.java.

> Hmmm ... could this mean that the attempt to read a 64-bit value where 
> only a 32-bit value had actually been pushed as a parameter, could 
> result in reading a junk value that sometimes caused the loop to exit 
> immediately?

On Linux x86_64 32 bit integers would be sign extended to 64 bit integers [1][2]
except if passing in register [3]. The 4th parameter would be passed in a
register. And that's only the native wrapper case. For the interpreter I
couldn't figure it out in 5min.

Long story short: maybe. I conducted tests (40x concurrently) where the target
never waited in native but did not get a JVMTI_ERROR_INVALID_SLOT.

The string construction for the log() calls is the only code I see that could
produce the error. But I'd expect the stack to be too shallow there. targetDepth
is usually well above 1000. So we would get JVMTI_ERROR_NO_MORE_FRAMES.

I'm unsure what happens if the safepoint stops the target thread at a poll
return (CompiledMethod::is_at_poll_return()). I will try that also. Likely not
before monday.

Thanks, Richard.


[1] Passing 32 bit value to native function
    https://github.com/openjdk/jdk/blob/57a27a6fb981a474ce7a6fbad0ec9e4f7164857e/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L2383

[2] move32_64
    https://github.com/openjdk/jdk/blob/57a27a6fb981a474ce7a6fbad0ec9e4f7164857e/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L1126

[3] move32_64 register to register
    https://github.com/openjdk/jdk/blob/57a27a6fb981a474ce7a6fbad0ec9e4f7164857e/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L1145


-----Original Message-----
From: David Holmes <david.holmes at oracle.com> 
Sent: Donnerstag, 3. September 2020 00:24
To: Reingruber, Richard <richard.reingruber at sap.com>; serviceability-dev <serviceability-dev at openjdk.java.net>
Subject: Re: RFR(XS) 8252593: [TESTBUG] serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java failed with JVMTI_ERROR_INVALID_SLOT

Hi Richard,

On 3/09/2020 12:37 am, Reingruber, Richard wrote:
> Hi,
> 
> please help review this fix for  a TESTBUG in
> serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java
> 
> Webrev: http://cr.openjdk.java.net/~rrich/webrevs/8252593/webrev.0/
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8252593
> 
> With this change the JVMTI test agent silently accepts 
> JVMTI_ERROR_INVALID_SLOT as result of a JVMTI GetLocalObject() call.
> The target frame of the call varies because the target thread is running. In
> rare cases it might not be one of the many frames of
> GetLocalWithoutSuspendTest.recursiveMethod() as intended but the frame 
> of a static method without parameters which the target thread might call after 
> returning from all the recursive calls. This would result in JVMTI_ERROR_INVALID_SLOT.
> 
> Anyway JVMTI_ERROR_INVALID_SLOT has to be expected and should be silently
> ignored.
> 
> Furthermore I have corrected the type of the parameter waitCycles of
> Java_GetLocalWithoutSuspendTest_notifyAgentToGetLocal() to be jint. Now it
> matches the declaration in GetLocalWithoutSuspendTest.java.

Hmmm ... could this mean that the attempt to read a 64-bit value where 
only a 32-bit value had actually been pushed as a parameter, could 
result in reading a junk value that sometimes caused the loop to exit 
immediately?

David

> Thanks, Richard.
>