• Davi Arnaut's avatar
    Bug#52419: x86 assembly based atomic CAS causes test failures · 1d520943
    Davi Arnaut authored
    The problem was that the x86 assembly based atomic CAS
    (compare and swap) implementation could copy the wrong
    value to the ebx register, where the cmpxchg8b expects
    to see part of the "comparand" value. Since the original
    value in the ebx register is saved in the stack (that is,
    the push instruction causes the stack pointer to change),
    a wrong offset could be used if the compiler decides to
    put the source of the comparand value in the stack.
    
    The solution is to copy the comparand value directly from
    memory. Since the comparand value is 64-bits wide, it is
    copied in two steps over to the ebx and ecx registers.
    
    include/atomic/x86-gcc.h:
      For reference, an excerpt from a faulty binary follows.
      
      It is a disassembly of my_atomic-t, compiled at -O3 with
      ICC 11.0. Most of the code deals with preparations for
      a atomic cmpxchg8b operation. This instruction compares
      the value in edx:eax with the destination operand. If the
      values are equal, the value in ecx:ebx is stored in the
      destination, otherwise the value in the destination operand
      is copied into edx:eax.
      
      In this case, my_atomic_add64 is implemented as a compare
      and exchange. The addition is done over temporary storage
      and loaded into the destination if the original term value
      is still valid.
      
        volatile int64 a64;
        int64 b=0x1000200030004000LL;
        a64=0;
            mov    0xfffffda8(%ebx),%eax
            xor    %ebp,%ebp
            mov    %ebp,(%eax)
            mov    %ebp,0x4(%eax)
        my_atomic_add64(&a64, b);
            mov    0xfffffda8(%ebx),%ebp      # Load address of a64
            mov    0x0(%ebp),%edx             # Copy value
            mov    0x4(%ebp),%ecx
            mov    %edx,0xc(%esp)             # Assign to tmp var in the stack
            mov    %ecx,0x10(%esp)
            add    $0x30004000,%edx           # Sum values
            adc    $0x10002000,%ecx
            mov    %edx,0x8(%esp)             # Save part of result for later
            mov    0x0(%ebp),%esi             # Copy value of a64 again
            mov    0x4(%ebp),%edi
            mov    0xc(%esp),%eax             # Load the value of a64 used
            mov    0x10(%esp),%edx            # for comparison
            mov    %esi,(%esp)
            mov    %edi,0x4(%esp)
            push   %ebx                       # Push %ebx into stack. Changes esp.
            mov    0x8(%esp),%ebx             # Wrong restore of the result.
            lock cmpxchg8b 0x0(%ebp)
            sete   %cl
            pop    %ebx
    1d520943
x86-gcc.h 5.64 KB