Hi Serguei, > The fix itself looks good to me.
thanks for looking at the fix. > 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. Sure, here is the new webrev.1 with a C++ version of the test agent: http://cr.openjdk.java.net/~rrich/webrevs/8249293/webrev.1/ I tested it on Linux and Windows but not yet on MacOS. Thanks, Richard. -----Original Message----- From: serguei.spit...@oracle.com <serguei.spit...@oracle.com> Sent: Freitag, 24. Juli 2020 00:00 To: Reingruber, Richard <richard.reingru...@sap.com>; serviceability-dev@openjdk.java.net Subject: Re: RFR(S) 8249293: Unsafe stackwalk in VM_GetOrSetLocal::doit_prologue() 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
