RFR(m): 8220351: Cross-modifying code

[Robbin Ehn]

Hi Erik,

On 2019-03-11 09:52, Erik Österlund wrote:
> 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.
> 

Thanks!

> 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.

Yes sure! But I'll hold off and see if there any better suggestion than 
cross_modify_fence.

/Robbin

> 
> 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
>