RFR(m): 8220351: Cross-modifying code

[Erik Österlund]

Hi Robbin,

This looks good to me. I think you have managed to find all the 
transitions from inactive to active, w.r.t. the safepoint synchronizer. 
At least I am not aware of the existence of any others.

The only thing I'm not quite sure about is the name of the new 
synchronization primitive (even though I was the one to suggest it... 
sorry my bad :C). The name "instruction_pipeline" seems a bit 
implementation specific about what HW architectural features need to be 
taken care of due to cross-modifying code, which may or may not apply to 
a given platform. Perhaps cross_modify_fence(), or something along those 
lines, would be better. That makes it more clear what we are protecting 
against, as opposed to what HW architectural features that might concern 
on a given platform.

I don't need another webrev for the name change.

Thanks,
/Erik

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