Shenandoah: SBSA::load_at() should save/restore registers when calling SATB barrier

[Zhengyu Gu]

Hi Roman,

Thanks for reviewing.

On 10/4/19 3:07 PM, Roman Kennke wrote:
> Is it ok for that particular piece of code to not push/pop rax and rbx?

save_registers()/restore_registers() do not push/pop rax and rbx.

> Otherwise you may just use push_CPU_state() and pop_CPU_state() which
> may also fit your bill?Nope, does not work.

Thanks,

-Zhengyu

> 
> Otherwise seems ok.
> 
> Roman
> 
> 
>> In SBSA::load_at(), it calls shenandoah_write_barrier_pre() for
>> references that need to be kept alive. However, the call has
>> side-effects that may mess up registers. Especially, the slow-path
>> contains runtime call.
>>
>> This problem impacts c2i_entry_barrier(), where upstream uses
>> resolve_weak_handle() which keeps the handle alive, while Shenandoah
>> uses peek_weak_handle(), which does not keep the handle alive, so it
>> bypasses SATB barrier. I think this just hides the problem.
>>
>> Shenandoah should just save and restore register around
>> shenandoah_write_barrier_pre() call to eliminate register issues, hence,
>> it can fallback to upstream c2i_entry_barrier() implementation and
>> eliminate the need for peek_weak_handle().
>>
>> This fix also fixes the intermittent is_loader_alive() assertion we have
>> seen in nightly tests. While this assertion is relative rare on x86_64,
>> but it is quite reproducible on x86_32 (because of it has less
>> registers(?)). With this fix, I have yet seen once.
>>
>>
>> Webrev:
>> http://cr.openjdk.java.net/~zgu/shenandoah/load_at_keepalive_fix/webrev.00/
>>
>> Test:
>>    hotspot_gc_shenandoah (fastdebug and release) x86_64 and x86_32 on Linux
>>
>> Thanks,
>>
>> -Zhengyu
>>
>