gcc 4.3.2 causes atomicops_unittest failure

[GoogleCodeExporter]

What steps will reproduce the problem?
./configure && make check

What is the expected output? What do you see instead?
Check failed: value == new_value
FAIL: atomicops_unittest

What version of the product are you using? On what operating system?
gcc 4.3.2 causes this if -O2 is used.  Making certain local variables 
volatile in the unittest makes the problem go away.  I haven't looked into 
whether this is a GCC or perftools bug beyond that.  The failure occurs at 
line 169 after a call to NoBarrier_AtomicIncrement in TestAtomicOps<int>.

I am on an i686-linux machine.  I can provide an assembly listing of a 
minimal test case if desired.  

Please provide any additional information below.

Original issue reported on code.google.com by dvi...@gmail.com on 16 Oct 2008 at 5:38

[GoogleCodeExporter]

Yes, assembly would be very helpful if you could provide it.  I'm trying to
reproduce, but am having trouble finding an i686 machine I can install gcc 
4.3.2 on.
 Hopefully the assembly will make it clear what's going wrong even without needing to
reproduce it.

Original comment by csilv...@gmail.com on 16 Oct 2008 at 10:00

• Changed state: Accepted

[GoogleCodeExporter]

Never mind -- I managed to find a machine I can reproduce this on.

Original comment by csilv...@gmail.com on 16 Oct 2008 at 11:20

[GoogleCodeExporter]

Here it is anyway in case anyone else wants a peek:

        .file   "atomicops_unittest.cc"
        .text
        .p2align 4,,15
.globl main
        .type   main, @function
main:
.LFB606:
        leal    4(%esp), %ecx
.LCFI0:
        andl    $-16, %esp
        pushl   -4(%ecx)
.LCFI1:
        movl    $1, %edx
        pushl   %ebp
.LCFI2:
        movl    %esp, %ebp
.LCFI3:
        pushl   %ecx
.LCFI4:
        subl    $16, %esp
.LCFI5:
        leal    -8(%ebp), %eax
        movl    $2147483647, -8(%ebp)
#APP
# 97 "../base/atomicops-internals-x86.h" 1
        lock; xaddl %edx,(%eax)
# 0 "" 2
#NO_APP
        xorl    %eax, %eax
        cmpl    $-2147483648, -8(%ebp)
        sete    %al
        addl    $16, %esp
        popl    %ecx
        subl    $1, %eax
        andl    $-6, %eax
        addl    $19, %eax
        popl    %ebp
        leal    -4(%ecx), %esp
        ret

Original comment by dvi...@gmail.com on 16 Oct 2008 at 11:55

[GoogleCodeExporter]

C code:

#include "base/logging.h"
#include "base/atomicops.h"

int main(int argc, char** argv) {
  Atomic32 test_val = (static_cast<uint64>(1) << 31);
  Atomic32 value = -1 ^ test_val;
  Atomic32 new_value = base::subtle::NoBarrier_AtomicIncrement(&value, 1);
  if( test_val != value )
      return 13;
  if( value != new_value )
      return 19;
  return 0;
}

Original comment by dvi...@gmail.com on 16 Oct 2008 at 11:55

[GoogleCodeExporter]

Here's a minimal program that reproduces the bug (I've verified it's in gcc 
4.4.0
[trunk] as well as 4.3.2), uses gcc, and doesn't require any includes or 
anything. 
This is on i686:

typedef int Atomic32;

inline Atomic32 NoBarrier_AtomicIncrement(volatile Atomic32* ptr,
                                          Atomic32 increment) {
  Atomic32 temp = increment;
  __asm__ __volatile__("lock; xaddl %0,%1"
                       : "+r" (temp), "+m" (*ptr)
                       : : "memory");
  // temp now holds the old value of *ptr
  return temp + increment;
}

int main(int argc, char** argv) {
  Atomic32 test_val = (1ULL << 31);
  Atomic32 value = -1 ^ test_val;
  Atomic32 new_value = NoBarrier_AtomicIncrement(&value, 1);
  if( test_val != value )
      return 13;
  if( value != new_value )
      return 19;
  return 0;
}

You must compile with -O2 to see the bug.  Our guess is this is a compiler bug, 
and
the compiler is trying to remove the last if, which it has decided can never be 
true,
but is removing it "backwards", so it *always* returns 19 instead of *never*
returning 19.  But we're looking into it a bit more to see if we can figure out
what's going on.

Original comment by csilv...@gmail.com on 17 Oct 2008 at 12:21

[GoogleCodeExporter]

OK, turns out it wasn't a compiler bug.  Here is what is happening:

After inining and assertion propagation, the checking code becomes:

new_value = temp + 1;
if (MIN_INT != value)
  return 13;
if (MIN_INT != new_value)
   return 19;

The compiler knows that there is no value temp could have, such that temp + 1 ==
MIN_INT (because the compiler here is ignoring machine-value reality about 
overflow).

The solution is to use unsigned values.  We'll fix this for the next release.

Original comment by csilv...@gmail.com on 17 Oct 2008 at 9:26

• Changed state: Started

[GoogleCodeExporter]

This should be fixed in perftools 1.0rc1

Original comment by csilv...@gmail.com on 13 Dec 2008 at 1:45

• Changed state: Fixed