The problem was due to a misuse of GCC asm constraints used to
implement a atomic load. On x86_64, the load was implemented
as a cmpxchg which implicitly uses the eax register as a
source and destination operand, yet the dummy value used for
comparison wasn't being properly loaded into eax (and other
problems).
The core problem is that cmpxchg is unnecessary as a load
on x86_64 as there are other simpler instructions such
as xadd. Even though, such instructions are only used to
have a memory barrier as load and stores are atomic by
definition. Hence, the solution is to explicitly issue the
required CPU and compiler barriers.
include/atomic/x86-gcc.h:
Issue a synchronizing instruction before loading the value.
Afterwards, issue a compiler barrier to prevent reordering.
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
Bug#52261: 64 bit atomic operations do not work on Solaris i386
gcc in debug compilation
One of the various problems was that the source operand to
CMPXCHG8b was marked as a input/output operand, causing GCC
to use the EBX register as the destination register for the
CMPXCHG8b instruction. This could lead to crashes as the EBX
register is also implicitly used by the instruction, causing
the value to be potentially garbaged and a protection fault
once the value is used to access a position in memory.
Another problem was the lack of proper clobbers for the atomic
operations and, also, a discrepancy between the implementations
for the Compare and Set operation. The specific problems are
described and fixed by Kristian Nielsen patches:
Patch: 1
Fix bugs in my_atomic_cas*(val,cmp,new) that *cmp is accessed
after CAS succeds.
In the gcc builtin implementation, problem was that *cmp was
read again after atomic CAS to check if old *val == *cmp;
this fails if CAS is successful and another thread modifies
*cmp in-between.
In the x86-gcc implementation, problem was that *cmp was set
also in the case of successful CAS; this means there is a
window where it can clobber a value written by another thread
after successful CAS.
Patch 2:
Add a GCC asm "memory" clobber to primitives that imply a
memory barrier.
This signifies to GCC that any potentially aliased memory
must be flushed before the operation, and re-read after the
operation, so that read or modification in other threads of
such memory values will work as intended.
In effect, it makes these primitives work as memory barriers
for the compiler as well as the CPU. This is better and more
correct than adding "volatile" to variables.
include/atomic/gcc_builtins.h:
Do not read from *cmp after the operation as it might be
already gone if the operation was successful.
include/atomic/nolock.h:
Prefer system provided atomics over the broken x86 asm.
include/atomic/x86-gcc.h:
Do not mark source operands as input/output operands.
Add proper memory clobbers.
include/my_atomic.h:
Add notes about my_atomic_add and my_atomic_cas behaviors.
unittest/mysys/my_atomic-t.c:
Remove work around, if it fails, there is either a problem
with the atomic operations code or the specific compiler
version should be black-listed.
The atomic operations implementation on 5.1 has a few problems,
which might cause tests to abort randomly. Since no code in 5.1
uses atomic operations, simply remove the code.
Conflicts
=========
Text conflict in .bzr-mysql/default.conf
Text conflict in libmysqld/CMakeLists.txt
Text conflict in libmysqld/Makefile.am
Text conflict in mysql-test/collections/default.experimental
Text conflict in mysql-test/extra/rpl_tests/rpl_row_sp006.test
Text conflict in mysql-test/suite/binlog/r/binlog_tmp_table.result
Text conflict in mysql-test/suite/rpl/r/rpl_loaddata.result
Text conflict in mysql-test/suite/rpl/r/rpl_loaddata_fatal.result
Text conflict in mysql-test/suite/rpl/r/rpl_row_create_table.result
Text conflict in mysql-test/suite/rpl/r/rpl_row_sp006_InnoDB.result
Text conflict in mysql-test/suite/rpl/r/rpl_stm_log.result
Text conflict in mysql-test/suite/rpl_ndb/r/rpl_ndb_circular_simplex.result
Text conflict in mysql-test/suite/rpl_ndb/r/rpl_ndb_sp006.result
Text conflict in mysql-test/t/mysqlbinlog.test
Text conflict in sql/CMakeLists.txt
Text conflict in sql/Makefile.am
Text conflict in sql/log_event_old.cc
Text conflict in sql/rpl_rli.cc
Text conflict in sql/slave.cc
Text conflict in sql/sql_binlog.cc
Text conflict in sql/sql_lex.h
21 conflicts encountered.
NOTE
====
mysql-5.1-rpl-merge has been made a mirror of mysql-next-mr:
- "mysql-5.1-rpl-merge$ bzr pull ../mysql-next-mr"
This is the first cset (merge/...) committed after pulling
from mysql-next-mr.
Use compiler provided atomic builtins as a 'backend' for
MySQL's atomic primitives. The builtins are available on
a handful of platforms and compilers.
configure.in:
Check if the compiler provides atomic builtins and
that __sync_lock_test_and_set stores the correct value.
include/atomic/nolock.h:
Use the atomic builtins if available.
include/atomic/gcc_builtins.h:
Implement the atomic ADD, SWAP, CAS, STORE (or operation
optimized away) and LOAD primitives using atomic builtins
provided by the compiler.
Changed header to GPL version 2 only
client/mysqlslap.c:
Changed header to GPL version 2 only
include/atomic/nolock.h:
Changed header to GPL version 2 only
include/atomic/rwlock.h:
Changed header to GPL version 2 only
include/atomic/x86-gcc.h:
Changed header to GPL version 2 only
include/atomic/x86-msvc.h:
Changed header to GPL version 2 only
include/my_atomic.h:
Changed header to GPL version 2 only
include/my_trie.h:
Changed header to GPL version 2 only
include/my_vle.h:
Changed header to GPL version 2 only
include/mysql/plugin.h:
Changed header to GPL version 2 only
mysys/my_atomic.c:
Changed header to GPL version 2 only
mysys/my_getncpus.c:
Changed header to GPL version 2 only
mysys/my_memmem.c:
Changed header to GPL version 2 only
mysys/my_vle.c:
Changed header to GPL version 2 only
mysys/trie.c:
Changed header to GPL version 2 only
plugin/Makefile.am:
Changed header to GPL version 2 only
server-tools/instance-manager/IMService.h:
Changed header to GPL version 2 only
server-tools/instance-manager/WindowsService.h:
Changed header to GPL version 2 only
server-tools/instance-manager/exit_codes.h:
Changed header to GPL version 2 only
server-tools/instance-manager/user_management_commands.h:
Changed header to GPL version 2 only
sql/authors.h:
Changed header to GPL version 2 only
sql/contributors.h:
Changed header to GPL version 2 only
sql/event_data_objects.cc:
Changed header to GPL version 2 only
sql/event_data_objects.h:
Changed header to GPL version 2 only
sql/event_db_repository.cc:
Changed header to GPL version 2 only
sql/event_db_repository.h:
Changed header to GPL version 2 only
sql/event_queue.cc:
Changed header to GPL version 2 only
sql/event_queue.h:
Changed header to GPL version 2 only
sql/event_scheduler.cc:
Changed header to GPL version 2 only
sql/event_scheduler.h:
Changed header to GPL version 2 only
sql/events.cc:
Changed header to GPL version 2 only
sql/events.h:
Changed header to GPL version 2 only
sql/ha_ndbcluster_binlog.cc:
Changed header to GPL version 2 only
sql/ha_ndbcluster_binlog.h:
Changed header to GPL version 2 only
sql/ha_ndbcluster_tables.h:
Changed header to GPL version 2 only
sql/ha_partition.cc:
Changed header to GPL version 2 only
sql/ha_partition.h:
Changed header to GPL version 2 only
sql/item_xmlfunc.cc:
Changed header to GPL version 2 only
sql/item_xmlfunc.h:
Changed header to GPL version 2 only
sql/log.h:
Changed header to GPL version 2 only
sql/partition_element.h:
Changed header to GPL version 2 only
sql/partition_info.cc:
Changed header to GPL version 2 only
sql/partition_info.h:
Changed header to GPL version 2 only
sql/rpl_filter.cc:
Changed header to GPL version 2 only
sql/rpl_filter.h:
Changed header to GPL version 2 only
sql/rpl_injector.cc:
Changed header to GPL version 2 only
sql/rpl_injector.h:
Changed header to GPL version 2 only
sql/rpl_mi.cc:
Changed header to GPL version 2 only
sql/rpl_mi.h:
Changed header to GPL version 2 only
sql/rpl_rli.cc:
Changed header to GPL version 2 only
sql/rpl_rli.h:
Changed header to GPL version 2 only
sql/rpl_tblmap.cc:
Changed header to GPL version 2 only
sql/rpl_tblmap.h:
Changed header to GPL version 2 only
sql/rpl_utility.cc:
Changed header to GPL version 2 only
sql/rpl_utility.h:
Changed header to GPL version 2 only
sql/sql_binlog.cc:
Changed header to GPL version 2 only
sql/sql_partition.cc:
Changed header to GPL version 2 only
sql/sql_partition.h:
Changed header to GPL version 2 only
sql/sql_plugin.cc:
Changed header to GPL version 2 only
sql/sql_plugin.h:
Changed header to GPL version 2 only
sql/sql_servers.cc:
Changed header to GPL version 2 only
sql/sql_servers.h:
Changed header to GPL version 2 only
sql/sql_tablespace.cc:
Changed header to GPL version 2 only
sql/sql_yacc.yy.bak:
Changed header to GPL version 2 only
storage/Makefile.am:
Changed header to GPL version 2 only
storage/archive/Makefile.am:
Changed header to GPL version 2 only
storage/blackhole/Makefile.am:
Changed header to GPL version 2 only
storage/csv/Makefile.am:
Changed header to GPL version 2 only
storage/example/Makefile.am:
Changed header to GPL version 2 only
storage/federated/Makefile.am:
Changed header to GPL version 2 only
storage/innobase/handler/Makefile.am:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/AllocNodeId.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/CreateFilegroup.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/CreateFilegroupImpl.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/CreateObj.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/DictObjOp.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/DihFragCount.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/DropFilegroup.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/DropFilegroupImpl.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/DropObj.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/Extent.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/LgmanContinueB.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/PgmanContinueB.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/RestoreContinueB.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/RestoreImpl.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/RouteOrd.hpp:
Changed header to GPL version 2 only
storage/ndb/include/kernel/signaldata/TsmanContinueB.hpp:
Changed header to GPL version 2 only
storage/ndb/include/ndbapi/NdbIndexStat.hpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/mgmapi_logevent/mgmapi_logevent.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/mgmapi_logevent_dual/mgmapi_logevent_dual.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_async/ndbapi_async.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_async1/ndbapi_async1.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_event/ndbapi_event.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_retries/ndbapi_retries.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_scan/ndbapi_scan.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_simple/ndbapi_simple.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_simple_dual/ndbapi_simple_dual.cpp:
Changed header to GPL version 2 only
storage/ndb/ndbapi-examples/ndbapi_simple_index/ndbapi_simple_index.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbdih/printSysfile.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbtup/DbtupDiskAlloc.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbtup/DbtupVarAlloc.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbtup/Undo_buffer.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbtup/Undo_buffer.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbtup/tuppage.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbtup/tuppage.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/dbtux/DbtuxStat.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/diskpage.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/lgman.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/lgman.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/pgman.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/pgman.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/print_file.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/record_types.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/restore.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/restore.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/tsman.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/blocks/tsman.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/DLCFifoList.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/DLCHashTable.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/DynArr256.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/DynArr256.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/KeyTable2Ref.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/LinearPool.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/NdbdSuperPool.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/NdbdSuperPool.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/Pool.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/Pool.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/RWPool.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/RWPool.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/Rope.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/SLFifoList.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/WOPool.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/WOPool.hpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/bench_pool.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/ndbd_malloc_impl.cpp:
Changed header to GPL version 2 only
storage/ndb/src/kernel/vm/ndbd_malloc_impl.hpp:
Changed header to GPL version 2 only
storage/ndb/src/mgmsrv/ParamInfo.cpp:
Changed header to GPL version 2 only
storage/ndb/src/ndbapi/NdbIndexStat.cpp:
Changed header to GPL version 2 only
storage/ndb/test/ndbapi/testIndexStat.cpp:
Changed header to GPL version 2 only
storage/ndb/test/tools/listen.cpp:
Changed header to GPL version 2 only
storage/ndb/tools/restore/ndb_nodegroup_map.h:
Changed header to GPL version 2 only
strings/my_strchr.c:
Changed header to GPL version 2 only
unittest/mysys/base64-t.c:
Changed header to GPL version 2 only
unittest/mysys/bitmap-t.c:
Changed header to GPL version 2 only
unittest/mysys/my_atomic-t.c:
Changed header to GPL version 2 only
unittest/mytap/tap.c:
Changed header to GPL version 2 only
unittest/mytap/tap.h:
Changed header to GPL version 2 only
win/Makefile.am:
Changed header to GPL version 2 only
include/atomic/nolock.h:
support for xadd emulation
include/atomic/x86-msvc.h:
syntax error
include/my_atomic.h:
support for xadd emulation, cleanup
mysys/my_atomic.c:
assert
rename *.t* to *-t* to be automake-friendly
simplify Makefiles
test_atomic.c:
move to unittest, add GPL comment, fix warnings, convert to tap framework.
configure:
remove custom tests for available types, use AC_CHECK_TYPE instead
x86-gcc.h:
fix gcc -ansi errors while maintaining readability
ignore:
added *-t
unittest/mysys/base64-t.c:
Rename: unittest/mysys/base64.t.c -> unittest/mysys/base64-t.c
unittest/mysys/bitmap-t.c:
Rename: unittest/mysys/bitmap.t.c -> unittest/mysys/bitmap-t.c
unittest/mytap/t/basic-t.c:
Rename: unittest/mytap/t/basic.t.c -> unittest/mytap/t/basic-t.c
unittest/examples/no_plan-t.c:
Rename: unittest/examples/no_plan.t.c -> unittest/examples/no_plan-t.c
unittest/examples/simple-t.c:
Rename: unittest/examples/simple.t.c -> unittest/examples/simple-t.c
unittest/examples/skip-t.c:
Rename: unittest/examples/skip.t.c -> unittest/examples/skip-t.c
unittest/examples/skip_all-t.c:
Rename: unittest/examples/skip_all.t.c -> unittest/examples/skip_all-t.c
unittest/examples/todo-t.c:
Rename: unittest/examples/todo.t.c -> unittest/examples/todo-t.c
BitKeeper/etc/ignore:
added *-t
config/ac-macros/misc.m4:
remove custom AC_TRY_RUN tests for available types, use AC_CHECK_TYPE instead
configure.in:
remove custom tests for available types, use AC_CHECK_TYPE instead
include/atomic/x86-gcc.h:
fix gcc -ansi errors while maintaining readability
include/my_global.h:
remove custom tests for available types, use AC_CHECK_TYPE instead
include/my_sys.h:
add missing declaration
mysys/Makefile.am:
move test_atomic to unittest
unittest/Makefile.am:
simplifications, correct permissions in chmod
unittest/README.txt:
rename *.t* to *-t* to be automake-friendly
unittest/examples/Makefile.am:
rename *.t* to *-t* to be automake-friendly
simplify Makefile
unittest/mysys/Makefile.am:
rename *.t* to *-t* to be automake-friendly
simplify Makefile
unittest/mysys/my_atomic-t.c:
move mysys/test_atomic.c to unittest, add GPL comment, fix warnings,
convert to tap framework.
unittest/mytap/t/Makefile.am:
rename *.t* to *-t* to be automake-friendly
simplify Makefile
unittest/unit.pl:
rename *.t* to *-t* to be automake-friendly
