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

[Bertrand Delsart]

Looks good.

Bertrand (not a Reviewer).

On 05/11/2014 14:59, Mikael Gerdin wrote:
> 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


-- 
Bertrand Delsart,                     Grenoble Engineering Center
Oracle,         180 av. de l'Europe,          ZIRST de Montbonnot
38334 Saint Ismier,                                        FRANCE
bertrand.delsart at oracle.com             Phone : +33 4 76 18 81 23

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: This email message is for the sole use of the intended
recipient(s) and may contain confidential and privileged
information. Any unauthorized review, use, disclosure or
distribution is prohibited. If you are not the intended recipient,
please contact the sender by reply email and destroy all copies of
the original message.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~