Bug#21554 (sp_cache.cc: violates C++ aliasing rules)

The problem reported is a compile bug,
reported by the development GCC team with GCC 4.2.

The original issue can no longer be reproduced in MySQL 5.1,
since the configure script no longer define HAVE_ATOMIC_ADD,
which caused the Linux atomic functions to be used (and cause a problem
with an invalid cast).

This patch implements some code cleanup for 5.1 only, which was identified
during the investigation of this issue.

With this patch, statistics maintained in THD::status_var are by definition
owned by the running thread, and do not need to be protected against race
conditions. These statistics are maintained by the status_var_* helpers,
which do not require any lock.


include/my_global.h:
  General cleanup of thread_safe_increment / statistic_increment
include/my_pthread.h:
  General cleanup of thread_safe_increment / statistic_increment
sql/filesort.cc:
  General cleanup of thread_safe_increment / statistic_increment
sql/handler.cc:
  General cleanup of thread_safe_increment / statistic_increment
sql/sql_insert.cc:
  General cleanup of thread_safe_increment / statistic_increment
sql/sql_parse.cc:
  General cleanup of thread_safe_increment / statistic_increment
sql/sql_prepare.cc:
  General cleanup of thread_safe_increment / statistic_increment
sql/sql_select.cc:
  General cleanup of thread_safe_increment / statistic_increment

1 files changed, 42 insertions, 15 deletions

diff --git a/include/my_pthread.h b/include/my_pthread.h
index 340dc32981a..ef52bbcb16c 100644
--- a/include/my_pthread.h
+++ b/include/my_pthread.h
@@ -710,33 +710,60 @@ extern uint my_thread_end_wait_time;
 
 extern uint thd_lib_detected;
 
-	/* statistics_xxx functions are for not essential statistic */
-
-#ifndef thread_safe_increment
-#ifdef HAVE_ATOMIC_ADD
-#define thread_safe_increment(V,L) atomic_inc((atomic_t*) &V)
-#define thread_safe_decrement(V,L) atomic_dec((atomic_t*) &V)
-#define thread_safe_add(V,C,L)     atomic_add((C),(atomic_t*) &V)
-#define thread_safe_sub(V,C,L)     atomic_sub((C),(atomic_t*) &V)
-#else
+/*
+  thread_safe_xxx functions are for critical statistic or counters.
+  The implementation is guaranteed to be thread safe, on all platforms.
+  Note that the calling code should *not* assume the counter is protected
+  by the mutex given, as the implementation of these helpers may change
+  to use my_atomic operations instead.
+*/
+
+/*
+  Warning:
+  When compiling without threads, this file is not included.
+  See the *other* declarations of thread_safe_xxx in include/my_global.h
+*/
+#ifdef THREAD
 #define thread_safe_increment(V,L) \
         (pthread_mutex_lock((L)), (V)++, pthread_mutex_unlock((L)))
 #define thread_safe_decrement(V,L) \
         (pthread_mutex_lock((L)), (V)--, pthread_mutex_unlock((L)))
-#define thread_safe_add(V,C,L) (pthread_mutex_lock((L)), (V)+=(C), pthread_mutex_unlock((L)))
+#define thread_safe_add(V,C,L) \
+        (pthread_mutex_lock((L)), (V)+=(C), pthread_mutex_unlock((L)))
 #define thread_safe_sub(V,C,L) \
         (pthread_mutex_lock((L)), (V)-=(C), pthread_mutex_unlock((L)))
-#endif /* HAVE_ATOMIC_ADD */
+#endif
+
+/*
+  statistics_xxx functions are for non critical statistic,
+  maintained in global variables.
+  When compiling with SAFE_STATISTICS:
+  - race conditions can not occur.
+  - some locking occurs, which may cause performance degradation.
+
+  When compiling without SAFE_STATISTICS:
+  - race conditions can occur, making the result slightly inaccurate.
+  - the lock given is not honored.
+*/
 #ifdef SAFE_STATISTICS
-#define statistic_increment(V,L)   thread_safe_increment((V),(L))
-#define statistic_decrement(V,L)   thread_safe_decrement((V),(L))
-#define statistic_add(V,C,L)       thread_safe_add((V),(C),(L))
+#define statistic_increment(V,L) thread_safe_increment((V),(L))
+#define statistic_decrement(V,L) thread_safe_decrement((V),(L))
+#define statistic_add(V,C,L)     thread_safe_add((V),(C),(L))
+#define statistic_sub(V,C,L)     thread_safe_sub((V),(C),(L))
 #else
 #define statistic_decrement(V,L) (V)--
 #define statistic_increment(V,L) (V)++
 #define statistic_add(V,C,L)     (V)+=(C)
+#define statistic_sub(V,C,L)     (V)-=(C)
 #endif /* SAFE_STATISTICS */
-#endif /* thread_safe_increment */
+
+/*
+  No locking needed, the counter is owned by the thread
+*/
+#define status_var_increment(V) (V)++
+#define status_var_decrement(V) (V)--
+#define status_var_add(V,C)     (V)+=(C)
+#define status_var_sub(V,C)     (V)-=(C)
 
 #ifdef  __cplusplus
 }