[Bug libstdc++/46914] New: std::atomic<int*>::exchange(...) doesn't store correct value.

[Bug libstdc++/46914] New: std::atomic<int*>::exchange(...) doesn't store correct value.

[frtsang at hotmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

           Summary: std::atomic<int*>::exchange(...) doesn't store correct
                    value.
           Product: gcc
           Version: 4.4.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: frtsang@hotmail.com


Run the following to compile the program below.
$ g++ -std=c++0x ./test.cpp -o test.tsk

//test.cpp
#include <iostream>
#include <stdatomic.h>

int main(int argc, char **argv)
{
    int *a = new int(10);
    std::cout << "a = " << a << " *a = " << *a << std::endl;

    std::atomic<int*> x(a);
    int *ptr = x;
    std::cout << "ptr = " << ptr << " *ptr " << *ptr << std::endl;

    int *b = new int(11);
    std::cout << "b = " << b << " *b = " << *b << std::endl;

    std::cout << "x.exchange(b) = " << x.exchange(b) << std::endl;
    ptr = x;
    std::cout << "ptr = " << ptr << " *ptr " << *ptr << std::endl;

    int *c = new int(12);
    std::cout << "c = " << c << " *c = " << *c << std::endl;

    std::cout << "x.exchange(c) = " << x.exchange(c) << std::endl;
    ptr = x;
    std::cout << "ptr = " << ptr << " *ptr " << *ptr << std::endl;

    return 0;
}

$ g++ --version
g++ (Ubuntu 4.4.3-4ubuntu5) 4.4.3
Copyright (C) 2009 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

It runs and gives the following result.

./test.tsk 
a = 0x9b9c008 *a = 10
ptr = 0x9b9c008 *ptr 10
b = 0x9b9c018 *b = 11
x.exchange(b) = 0x9b9c008
ptr = 0x1ec71b *ptr -388381823
c = 0x9b9c028 *c = 12
x.exchange(c) = 0x1ec71b
ptr = 0x1ec71b *ptr -388381823

After "x" is initialized with "a", it doesn't store the new value correctly in
its "exchange" member function.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[frtsang at hotmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #1 from Francis <frtsang at hotmail dot com> 2010-12-12 20:15:04 UTC ---
Forget to mention my system type

$ uname -a
Linux my_mach_name 2.6.32-26-generic #48-Ubuntu SMP Wed Nov 24 09:00:03 UTC
2010 i686 GNU/Linux

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> 2010-12-12 20:51:40 UTC ---
I cannot reproduce this on anything from current 4.4 branch to mainline (with
<atomic>). I get:

a = 0x603010 *a = 10
ptr = 0x603010 *ptr 10
b = 0x603030 *b = 11
x.exchange(b) = 0x603010
ptr = 0x603030 *ptr 11
c = 0x603050 *c = 12
x.exchange(c) = 0x603030
ptr = 0x603050 *ptr 12

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely.gcc at gmail dot
                   |                            |com

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-27 14:30:27 UTC ---
Jon, can you help me trying to reproduce this? I can't, it seems an Ubuntu
issue.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-27 14:43:49 UTC ---
I think I tried to reproduce it on Fedora last month and failed - I'll look
again at the weekend

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-27 15:02:52 UTC ---
Thanks!

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-01-27 15:06:17 UTC ---
as requested in the bug reporting guidelines, please provide the output of 'gcc
-v' (not just gcc --version) so we can see how your compiler was configured

I know the Debian gcc packages default to -march=i486 but I don't know about
Ubuntu

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #7 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-27 15:33:47 UTC ---
Ok. In any case we should not risk miscompiling stuff, better bailing out at
compile time. I see that in atomic_base.h the configure-time macros
_GLIBCXX_ATOMIC_BUILTINS_* are used and I se no uses of the
__GCC_HAVE_SYNC_COMPARE_AND_SWAP_* macros, the only one sensitive to the
command line switches. But apparently the PR is about compiling *without* any
special command line switch affecting the availability of the builtins, thus I
don't see how things could possibly go wrong. Something also to keep in mind is
that [GLIBCXX_ENABLE_ATOMIC_BUILTINS] in acinclude.m4 changed recently: I don't
think we want to go through big efforts fixing this issue affecting only C++0x
facilities in case it shows up only on 4.4.x, say, and it's fixed in mainline.
I'll also try to run further tests in mainline, multilib too.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |paolo.carlini at oracle dot
                   |                            |com

--- Comment #8 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-27 15:45:01 UTC ---
I can confirm my negative results with multilib mainline default configured.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #9 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-27 15:58:19 UTC ---
Likewise for 4_5-branch.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[frtsang at hotmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #10 from Francis <frtsang at hotmail dot com> 2011-01-28 05:00:23 UTC ---
This is how my compiler is configured.
$ /usr/bin$ ./gcc-4.4 -v
Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 4.4.3-4ubuntu5'
--with-bugurl=file:///usr/share/doc/gcc-4.4/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --enable-shared
--enable-multiarch --enable-linker-build-id --with-system-zlib
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.4 --program-suffix=-4.4 --enable-nls
--enable-clocale=gnu --enable-libstdcxx-debug --enable-plugin --enable-objc-gc
--enable-targets=all --disable-werror --with-arch-32=i486 --with-tune=generic
--enable-checking=release --build=i486-linux-gnu --host=i486-linux-gnu
--target=i486-linux-gnu
Thread model: posix
gcc version 4.4.3 (Ubuntu 4.4.3-4ubuntu5)

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #11 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-28 09:43:44 UTC ---
Ok, with --with-arch-32=i486 in the configure options and -m32 on the command
line the problem can be reproduced.

Frankly, I don't understand what is going on: in that case, by default,
everything should remain consistent and the _GLIBCXX_ATOMIC_BUILTINS_* used
when building atomic.cc should be still the correct ones for user code too (if
some different -march were used on the command line, then I could see possible
troubles, again, because the atomic headers don't use macros sensitive to the
actual command line options).

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bkoz at redhat dot com

--- Comment #12 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-01-28 10:22:27 UTC ---
I'm making some progress: using --with-arch-32=i586 is fine, and the reason is,
in that case the _GLIBCXX_ATOMIC_BUILTINS_* are *all* true (in the i486 case,
_GLIBCXX_ATOMIC_BUILTINS_8 is false). Now, the testcase only uses ints, but in
atomic_base.h we require either all true, or the special case of at least
_GLIBCXX_ATOMIC_BUILTINS_1 true, that is  _GLIBCXX_ATOMIC_PROPERTY == 1, or
none true, that is _GLIBCXX_ATOMIC_PROPERTY == 0.

Given that, I suppose, Ubuntu and Debian have been using -i486 only because of
the crazyness old GCCs defaulting to i386 and now everything is different
finally (essentially the current default is a bit more than pentium4), I don't
consider this issue particularly serious.

Still, it would be nice to understand why both _GLIBCXX_ATOMIC_PROPERTY == 1
(as this PR shows) and _GLIBCXX_ATOMIC_PROPERTY == 0 (as trying
--with-arch-32=i386 shows), both don't work. Maybe Benjamin has ideas...

Note: the command line per se doesn't seem to matter, is the way the *.so is
built that matters.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.02.01 17:11:44
                 CC|paolo.carlini at oracle dot |
                   |com                         |
         AssignedTo|unassigned at gcc dot       |paolo.carlini at oracle dot
                   |gnu.org                     |com
   Target Milestone|---                         |4.6.0
     Ever Confirmed|0                           |1

--- Comment #13 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-02-01 17:11:44 UTC ---
I see the (absolutely stupid) typo we have in the code. Fix forthcoming.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #15 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-02-01 18:15:58 UTC ---
Author: paolo
Date: Tue Feb  1 18:15:56 2011
New Revision: 169495

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169495
Log:
2011-02-01  Paolo Carlini  <paolo.carlini@oracle.com>

    PR libstdc++/46914
    * include/bits/atomic_0.h (_ATOMIC_STORE_, _ATOMIC_MODIFY_,
    _ATOMIC_CMPEXCHNG_): Rename __v -> __w, and __m -> __n, to
    avoid name conflicts.


Modified:
    branches/gcc-4_5-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_5-branch/libstdc++-v3/include/bits/atomic_0.h

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #14 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-02-01 18:15:41 UTC ---
Author: paolo
Date: Tue Feb  1 18:15:39 2011
New Revision: 169494

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169494
Log:
2011-02-01  Paolo Carlini  <paolo.carlini@oracle.com>

    PR libstdc++/46914
    * include/bits/atomic_0.h (_ATOMIC_STORE_, _ATOMIC_MODIFY_,
    _ATOMIC_CMPEXCHNG_): Rename __v -> __w, and __m -> __n, to
    avoid name conflicts.


Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/atomic_0.h

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.6.0                       |4.5.3

--- Comment #16 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-02-01 18:16:45 UTC ---
Fixed for 4.5.3 and 4.6.0.

[Bug libstdc++/46914] std::atomic<int*>::exchange(...) doesn't store correct value.

[dnovillo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46914

--- Comment #17 from Diego Novillo <dnovillo at gcc dot gnu.org> 2011-02-02 18:16:47 UTC ---
Author: dnovillo
Date: Wed Feb  2 18:16:41 2011
New Revision: 169753

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169753
Log:
2011-02-01  Paolo Carlini  <paolo.carlini@oracle.com>

    PR libstdc++/46914
    * include/bits/atomic_0.h (_ATOMIC_STORE_, _ATOMIC_MODIFY_,
    _ATOMIC_CMPEXCHNG_): Rename __v -> __w, and __m -> __n, to
    avoid name conflicts.

Modified:
    branches/google/integration/libstdc++-v3/ChangeLog
    branches/google/integration/libstdc++-v3/include/bits/atomic_0.h