From c15b344a9d0067792036edee1b29bc26d42fa1cc Mon Sep 17 00:00:00 2001 From: Joerg Bruehe Date: Mon, 13 Sep 2010 12:14:48 +0200 Subject: Selective transfer of a bugfix patch into 5.5.6-rc. The first part is the functional change, the second is needed as a compile fix on Windows (header file order). | committer: Marc Alff | branch nick: mysql-5.5-bugfixing-56521 | timestamp: Thu 2010-09-09 14:28:47 -0600 | message: | Bug#56521 Assertion failed: (m_state == 2), function allocated_to_free, pfs_lock.h (138) | | Before this fix, it was possible to build the server: | - with the performance schema | - with a dummy implementation of my_atomic (MY_ATOMIC_MODE_DUMMY). | | In this case, the resulting binary will just crash, | as this configuration is not supported. | | This fix enforces that the build will fail with a compilation error in this | configuration, instead of resulting in a broken binary. | committer: Tor Didriksen | branch nick: 5.5-bugfixing-56521 | timestamp: Fri 2010-09-10 11:10:38 +0200 | message: | Header files should be self-contained --- storage/perfschema/ha_perfschema.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/storage/perfschema/ha_perfschema.cc b/storage/perfschema/ha_perfschema.cc index 0fac734f7a0..e5e324d6c6b 100644 --- a/storage/perfschema/ha_perfschema.cc +++ b/storage/perfschema/ha_perfschema.cc @@ -20,6 +20,7 @@ #include "my_global.h" #include "my_pthread.h" +#include "my_atomic.h" #include "sql_plugin.h" #include "mysql/plugin.h" #include "ha_perfschema.h" @@ -28,6 +29,17 @@ #include "pfs_instr_class.h" #include "pfs_instr.h" +#ifdef MY_ATOMIC_MODE_DUMMY +/* + The performance schema can can not function with MY_ATOMIC_MODE_DUMMY, + a fully functional implementation of MY_ATOMIC should be used instead. + If the build fails with this error message: + - either use a different ./configure --with-atomic-ops option + - or do not build with the performance schema. +*/ +#error "The performance schema needs a functional MY_ATOMIC implementation." +#endif + handlerton *pfs_hton= NULL; static handler* pfs_create_handler(handlerton *hton, -- cgit v1.2.1 From 1d5209438cd800950ba44101a20b599abccba4a5 Mon Sep 17 00:00:00 2001 From: Davi Arnaut Date: Fri, 17 Sep 2010 17:34:15 -0300 Subject: Bug#52419: x86 assembly based atomic CAS causes test failures 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 --- include/atomic/x86-gcc.h | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/include/atomic/x86-gcc.h b/include/atomic/x86-gcc.h index 8baa84e110e..90602ef900c 100644 --- a/include/atomic/x86-gcc.h +++ b/include/atomic/x86-gcc.h @@ -111,9 +111,9 @@ On some platforms (e.g. Mac OS X and Solaris) the ebx register is held as a pointer to the global offset table. Thus we're not allowed to use the b-register on those platforms when compiling - PIC code, to avoid this we push ebx and pop ebx and add a movl - instruction to avoid having ebx in the interface of the assembler - instruction. + PIC code, to avoid this we push ebx and pop ebx. The new value + is copied directly from memory to avoid problems with a implicit + manipulation of the stack pointer by the push. cmpxchg8b works on both 32-bit platforms and 64-bit platforms but the code here is only used on 32-bit platforms, on 64-bit @@ -121,11 +121,13 @@ fine. */ #define make_atomic_cas_body64 \ - int32 ebx=(set & 0xFFFFFFFF), ecx=(set >> 32); \ - asm volatile ("push %%ebx; movl %3, %%ebx;" \ - LOCK_prefix "; cmpxchg8b %0; setz %2; pop %%ebx" \ - : "=m" (*a), "+A" (*cmp), "=c" (ret) \ - : "m" (ebx), "c" (ecx), "m" (*a) \ + asm volatile ("push %%ebx;" \ + "movl (%%ecx), %%ebx;" \ + "movl 4(%%ecx), %%ecx;" \ + LOCK_prefix "; cmpxchg8b %0;" \ + "setz %2; pop %%ebx" \ + : "=m" (*a), "+A" (*cmp), "=c" (ret) \ + : "c" (&set), "m" (*a) \ : "memory", "esp") #endif -- cgit v1.2.1