RFR(S) 8249293: Unsafe stackwalk in VM_GetOrSetLocal::doit_prologue()

[serguei.spitsyn at oracle.com]

Hi Richard,

Thank you for filing the CR and taking care about it!
The fix itself looks good to me.
I still need another look at new test.
Could you, please, convert the agent of new test to C++?
It will make it a little bit simpler.

Thanks,
Serguei


On 7/20/20 01:15, Reingruber, Richard wrote:
> Hi,
>
> please help review this fix for VM_GetOrSetLocal. It moves the unsafe stackwalk from the vm
> operation prologue before the safepoint into the doit() method executed at the safepoint.
>
> Webrev: http://cr.openjdk.java.net/~rrich/webrevs/8249293/webrev.0/index.html
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8249293
>
> According to the JVMTI spec on local variable access it is not required to suspend the target thread
> T [1]. The operation will simply fail with JVMTI_ERROR_NO_MORE_FRAMES if T is executing
> bytecodes. It will succeed though if T is blocked because of synchronization or executing some native
> code.
>
> The issue is that in the latter case the stack walk in VM_GetOrSetLocal::doit_prologue() to prepare
> the access to the local variable is unsafe, because it is done before the safepoint and it races
> with T returning to execute bytecodes making its stack not walkable. The included test shows that
> this can crash the VM if T wins the race.
>
> Manual testing:
>
>    - new test test/hotspot/jtreg/serviceability/jvmti/GetLocalVariable/GetLocalWithoutSuspendTest.java
>    - test/hotspot/jtreg/vmTestbase/nsk/jvmti
>    - test/hotspot/jtreg/serviceability/jvmti
>
> Nightly regression tests @SAP: JCK and JTREG, also in Xcomp mode, SPECjvm2008, SPECjbb2015,
> Renaissance Suite, SAP specific tests with fastdebug and release builds on all platforms
>
> Thanks, Richard.
>
> [1] https://docs.oracle.com/en/java/javase/14/docs/specs/jvmti.html#local