[Bug libc/17403] New: atomic_full_barrier is incorrect for x86 and x86_64

[Bug libc/17403] New: atomic_full_barrier is incorrect for x86 and x86_64

[triegel at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17403

            Bug ID: 17403
           Summary: atomic_full_barrier is incorrect for x86 and x86_64
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: triegel at redhat dot com
                CC: drepper.fsp at gmail dot com

atomic_full_barrier on these archs currently is just a compiler barrier,
whereas it should use mfence to be an actual full barrier.

This bug doesn't seem to cause incorrect behavior currently:
* nptl/pthread_mutex_setprioceiling.c has a full barrier immediately followed
by a futex syscall, which is a full barrier too effectively.
* sysdeps/nptl/fork.c seems to only need an acquire barrier, so on x86 the
current compiler-only barrier that the full barrier will result in is
sufficient.
* Other generic code that uses full barriers (eg, sem_post) currently has
custom asm versions on x86/x86_64.  However, once we remove those, the lack of
a correct full barrier will be a bug.

Nonetheless, I think we need to fix the full barriers as soon as we add other
code that uses atomic_full_barrier and is used on x86/x86_64.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/17403] atomic_full_barrier is incorrect for x86 and x86_64

[bugdal at aerifal dot cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=17403

Rich Felker <bugdal at aerifal dot cx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugdal at aerifal dot cx

--- Comment #1 from Rich Felker <bugdal at aerifal dot cx> ---
This could use some explanation as to why you think it's wrong. In the original
x86 memory model, there is absolutely no need for memory barriers (though
compiler barriers are of course needed). My understanding is that there are
some newer instructions available which can optionally relax memory ordering,
but it's not clear to me if they would be used by the compiler in any way that
would necessitate having mfence or similar instructions alongside atomics. If
so, I believe this is an ABI incompatibility issue, since the atomics in use
may be built for i486 (where they don't have access to any explicit barrier
instruction). So from that standpoint some clarification is really needed to
determine if we have a bigger problem on our hands...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/17403] atomic_full_barrier is incorrect for x86 and x86_64

[triegel at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17403

--- Comment #2 from Torvald Riegel <triegel at redhat dot com> ---
I'm not quite sure about the really old ones such as i486, but the HW memory
model is TSO, which still needs Store/Load barriers -- it's not sequential
consistency that the HW provides.  So for stuff like Dekker synchronization you
need mfence.  See http://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/17403] atomic_full_barrier is incorrect for x86 and x86_64

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17403

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  c47ca9647f9b72692e62f94fe468cd5568f49129 (commit)
      from  b2faf4eadf5e9e9a71abcfe4da71fad91707ccac (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c47ca9647f9b72692e62f94fe468cd5568f49129

commit c47ca9647f9b72692e62f94fe468cd5568f49129
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed Oct 29 10:34:36 2014 +0100

    Fix atomic_full_barrier on x86 and x86_64.

    This fixes BZ #17403 by defining atomic_full_barrier,
    atomic_read_barrier, and atomic_write_barrier on x86 and x86_64.  A full
    barrier is implemented through an atomic idempotent modification to the
    stack and not through using mfence because the latter can supposedly be
    somewhat slower due to having to provide stronger guarantees wrt.
    self-modifying code, for example.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                       |    8 ++++++++
 NEWS                            |   25 +++++++++++++------------
 sysdeps/i386/i486/bits/atomic.h |    7 +++++++
 sysdeps/x86_64/bits/atomic.h    |    7 +++++++
 4 files changed, 35 insertions(+), 12 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/17403] atomic_full_barrier is incorrect for x86 and x86_64

[triegel at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17403

Torvald Riegel <triegel at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Torvald Riegel <triegel at redhat dot com> ---
Fixed with c47ca9647f9b72692e62f94fe468cd5568f49129.

-- 
You are receiving this mail because:
You are on the CC list for the bug.