RFR(m): 8220351: Cross-modifying code

[Robbin Ehn]

Hi David,

> 
> I finally realized how this translates into covering all the return-to-Java 
> paths, without having as many unnecessary calls to cross_modify_barrier. If we 
> return to Java after a safepoint (or action that could have installed modified 
> code) we need the barrier. If a safepoint has occurred then we must have become 
> safepoint-safe at some point, which means we either were in _thread_blocked (by 
> one means or another) or _thread_in_native. So we guard the transitions out of 
> those states.
> 

Yes, great!

> 
> I suggest creating subtasks for those platforms so this additional work isn't 
> forgotten.

Fixed.

> 
>> The deprecated global poll path does not contain proper barriers, and will be 
>> obsoleted in JDK 14.
> 
> Ok.
> 
> A couple of minor typos:
> 
> src/hotspot/share/runtime/safepointMechanism.inline.hpp
> 
> +     // JavaThread will disarm it self  ...
> 
> s/it self/itself/
> 
> ---
> src/hotspot/share/runtime/thread.cpp
> 
> +   // from here is probably far enought, ...
> 
> s/enought/enough/
> 
> +   // Since we are not using a regular thread-state tranisition helper here,
> 
> s/tranisition/transition/

Fixed, thanks!

/Robbin

> 
> Thanks,
> David
> -----
> 
>> Thanks, Robbin
>>
>> On 2019-03-08 16:24, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8220351
>>> Changeset:
>>> http://cr.openjdk.java.net/~rehn/8220351/webrev/
>>>
>>> After a JavaThread have been in a safepoint/(handshake) safe state it can start
>>> executing updated code. E.g. an oop in the instruction stream can have been
>>> updated.
>>>
>>> Most hardware's require a barrier or that the code cross modified is far away to
>>> guarantee that the thread executing the updated instruction stream sees the
>>> modification.
>>>
>>> What far away is and how far an update instruction stream is from a safepoint
>>> safe state is not clear.
>>>
>>> To be compliant with those architectures an instruction stream barrier must be
>>> added when leaving the safepoint safe state.
>>>
>>> There may be crashes today due to this missing barrier.
>>> A new CPU with deeper pipeline or changes to the VM which moves a safepoint safe
>>> state closer to a nmethod can increase changes of a crash.
>>>
>>> A few benchmarks will see a performance regression with the extra serializing,
>>> such as Octane-Crypto -5% (x86).
>>>
>>> With a much more elaborate fix, such as changing the local poll to have more
>>> than two states (armed/disarmed), it would be possible to only emit such
>>> instruction stream barrier when the poll have been armed while the thread was
>>> safepoint safe.
>>>
>>> Passes t1-3 on our platforms, and test built what I can.
>>>
>>> Thanks, Robbin