RFR: JDK-8061964: Insufficient compiler barriers for GCC in OrderAccess functions

[Mikael Gerdin]

Hi all,

I have an updated webrev with the following changes:

* compiler_barrier() calls added after the loads to all 
load_acquire-variants.
* the dummy store in release() is removed.

Full webrev at:
http://cr.openjdk.java.net/~mgerdin/8061964/webrev.1/

Incremental webrev at:
http://cr.openjdk.java.net/~mgerdin/8061964/webrev.0_to_1/

Thanks for all the feedback so far!

/Mikael

On 2014-11-03 14:01, Mikael Gerdin wrote:
> Hi all,
>
> Please review this attempt at fixing the OrderAccess functions on Linux
> x86 with GCC.
>
> While working on another bug I recently discovered that g++ was
> reordering stores across a call to OrderAccess::storestore on Linux x86.
>
> The G1 code attempts to do an ordered publishing of two values:
> _saved_mark_word = _top;
> OrderAccess::storestore();
> _gc_time_stamp = curr_gc_time_stamp;
>
> The types involved are
> HeapWord* _top, _saved_mark_word;
> volatile unsigned _gc_time_stamp;
>
> The incorrect behavior seems to have started when JDK-6973570 was fixed
> in JDK 7.
> Below, _top is at offset 0x58, _saved_mark_word at 0x18 and
> _gc_time_stamp at 0x138, %rbx is "this".
>
> /net/jre/onestop/jdk/1.7.0/promoted/all//b108/binaries/linux-x64/jre/lib/amd64/server/libjvm.so:
>
>    3d9f4d:       39 d0                   cmp    %edx,%eax
>    3d9f4f:       73 1c                   jae    3d9f6d
> <G1OffsetTableContigSpace::set_saved_mark()+0x3d>
>    3d9f51:       48 8b 43 58             mov    0x58(%rbx),%rax
>    3d9f55:       48 89 43 18             mov    %rax,0x18(%rbx)
>    3d9f59:       48 8b 05 40 f9 70 00    mov    0x70f940(%rip),%rax    #
> ae98a0 <_DYNAMIC+0x12f8>
>    3d9f60:       48 c7 00 00 00 00 00    movq   $0x0,(%rax)
>    3d9f67:       89 93 38 01 00 00       mov    %edx,0x138(%rbx)
>
> /net/jre/onestop/jdk/1.7.0/promoted/all//b109/binaries/linux-x64/jre/lib/amd64/server/libjvm.so
>
>    3da05d:       39 d0                   cmp    %edx,%eax
>    3da05f:       73 15                   jae    3da076
> <G1OffsetTableContigSpace::set_saved_mark()+0x36>
>    3da061:       48 8b 43 58             mov    0x58(%rbx),%rax
>    3da065:       c7 45 f4 00 00 00 00    movl   $0x0,-0xc(%rbp)
>    3da06c:       89 93 38 01 00 00       mov    %edx,0x138(%rbx)
>    3da072:       48 89 43 18             mov    %rax,0x18(%rbx)
>
> In b109 the store of %rax to 0x18(%rbx) has been ordered after the store
> of %edx to 0x138(%rbx) in the same build as JDK-6973570 was integrated.
>
> My suggestion to fix this is to extend all the OrderAccess::release*
> variants on x86 with a:
> __asm__ volatile ("" : : : "memory");
> to attempt to prevent GCC from reordering any memory accesses across
> those function calls.
>
> I've verified that this solves the issue in the assembly with our
> current JDK 9 build platform compilers.
> I've also verified that this particular piece of code is compiled
> correctly on our other x86 platforms: Solaris, Windows and OS X.
>
> Webrev:
> http://cr.openjdk.java.net/~mgerdin/8061964/webrev.0/
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8061964
> Testing:
> JPRT, inspecting generated assembly for the function
> G1OffsetTableContigSpace::record_top_and_timestamp (as the method is
> currently named).
> Suggestions of further testing is greatly appreciated.
>
> Thanks
> Mikael