mirror/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-01-15 19:42:28 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

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

Browse source 
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
This commit is contained in:
unknown 2007-05-22 13:41:40 -06:00
parent 3d01594f34
commit bb64e39e0a
8 changed files with 87 additions and 84 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
include/my_global.h
View file

@ -441,17 +441,7 @@ C_MODE_END
#ifdef HAVE_ALLOCA_H
#include <alloca.h>
#endif
#ifdef HAVE_ATOMIC_ADD
#define new my_arg_new
#define need_to_restore_new 1
C_MODE_START
#include <asm/atomic.h>
C_MODE_END
#ifdef need_to_restore_new /* probably safer than #ifdef new */
#undef new
#undef need_to_restore_new
#endif
#endif
#include <errno.h> /* Recommended by debian */
/* We need the following to go around a problem with openssl on solaris */
#if defined(HAVE_CRYPT_H)
@ -1449,10 +1439,13 @@ do { doubleget_union _tmp; \
#ifndef THREAD
#define thread_safe_increment(V,L) (V)++
#define thread_safe_decrement(V,L) (V)--
#define thread_safe_add(V,C,L) (V)+=(C)
#define thread_safe_sub(V,C,L) (V)-=(C)
#define statistic_increment(V,L) (V)++
#define statistic_decrement(V,L) (V)--
#define statistic_add(V,C,L) (V)+=(C)
#define statistic_sub(V,C,L) (V)-=(C)
#endif
#ifdef HAVE_CHARSET_utf8

55
include/my_pthread.h
View file

@ -710,33 +710,60 @@ extern uint my_thread_end_wait_time;
extern uint thd_lib_detected;
/* statistics_xxx functions are for not essential statistic */
/*
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.
*/
#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
/*
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
}

7
sql/filesort.cc
View file

@ -171,11 +171,11 @@ ha_rows filesort(THD *thd, TABLE *table, SORT_FIELD *sortorder, uint s_length,
if (select && select->quick)
{
statistic_increment(thd->status_var.filesort_range_count, &LOCK_status);
status_var_increment(thd->status_var.filesort_range_count);
}
else
{
statistic_increment(thd->status_var.filesort_scan_count, &LOCK_status);
status_var_increment(thd->status_var.filesort_scan_count);
}
#ifdef CAN_TRUST_RANGE
if (select && select->quick && select->quick->records > 0L)
@ -1120,8 +1120,7 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
THD::killed_state not_killable;
DBUG_ENTER("merge_buffers");
statistic_increment(current_thd->status_var.filesort_merge_passes,
&LOCK_status);
status_var_increment(current_thd->status_var.filesort_merge_passes);
if (param->not_killable)
{
killed= &not_killable;

19
sql/handler.cc
View file

@ -639,7 +639,7 @@ int ha_prepare(THD *thd)
for (; *ht; ht++)
{
int err;
statistic_increment(thd->status_var.ha_prepare_count,&LOCK_status);
status_var_increment(thd->status_var.ha_prepare_count);
if ((*ht)->prepare)
{
if ((err= (*(*ht)->prepare)(*ht, thd, all)))
@ -734,7 +734,7 @@ int ha_commit_trans(THD *thd, bool all)
my_error(ER_ERROR_DURING_COMMIT, MYF(0), err);
error= 1;
}
statistic_increment(thd->status_var.ha_prepare_count,&LOCK_status);
status_var_increment(thd->status_var.ha_prepare_count);
}
DBUG_EXECUTE_IF("crash_commit_after_prepare", abort(););
if (error || (is_real_trans && xid &&
@ -781,7 +781,7 @@ int ha_commit_one_phase(THD *thd, bool all)
my_error(ER_ERROR_DURING_COMMIT, MYF(0), err);
error=1;
}
statistic_increment(thd->status_var.ha_commit_count,&LOCK_status);
status_var_increment(thd->status_var.ha_commit_count);
*ht= 0;
}
trans->nht=0;
@ -837,7 +837,7 @@ int ha_rollback_trans(THD *thd, bool all)
my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err);
error=1;
}
statistic_increment(thd->status_var.ha_rollback_count,&LOCK_status);
status_var_increment(thd->status_var.ha_rollback_count);
*ht= 0;
}
trans->nht=0;
@ -1252,8 +1252,7 @@ int ha_rollback_to_savepoint(THD *thd, SAVEPOINT *sv)
my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err);
error=1;
}
statistic_increment(thd->status_var.ha_savepoint_rollback_count,
&LOCK_status);
status_var_increment(thd->status_var.ha_savepoint_rollback_count);
trans->no_2pc|=(*ht)->prepare == 0;
}
/*
@ -1268,7 +1267,7 @@ int ha_rollback_to_savepoint(THD *thd, SAVEPOINT *sv)
my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err);
error=1;
}
statistic_increment(thd->status_var.ha_rollback_count,&LOCK_status);
status_var_increment(thd->status_var.ha_rollback_count);
*ht=0; // keep it conveniently zero-filled
}
DBUG_RETURN(error);
@ -1301,7 +1300,7 @@ int ha_savepoint(THD *thd, SAVEPOINT *sv)
my_error(ER_GET_ERRNO, MYF(0), err);
error=1;
}
statistic_increment(thd->status_var.ha_savepoint_count,&LOCK_status);
status_var_increment(thd->status_var.ha_savepoint_count);
}
sv->nht=trans->nht;
#endif /* USING_TRANSACTIONS */
@ -1489,7 +1488,7 @@ handler *handler::clone(MEM_ROOT *mem_root)
void handler::ha_statistic_increment(ulong SSV::*offset) const
{
statistic_increment(table->in_use->status_var.*offset, &LOCK_status);
status_var_increment(table->in_use->status_var.*offset);
}
void **handler::ha_data(THD *thd) const
@ -2836,7 +2835,7 @@ int ha_discover(THD *thd, const char *db, const char *name,
error= 0;
if (!error)
statistic_increment(thd->status_var.ha_discover_count,&LOCK_status);
status_var_increment(thd->status_var.ha_discover_count);
DBUG_RETURN(error);
}

2
sql/sql_insert.cc
View file

@ -2538,7 +2538,7 @@ bool Delayed_insert::handle_inserts(void)
if (table->s->blob_fields)
free_delayed_insert_blobs(table);
thread_safe_sub(delayed_rows_in_use,1,&LOCK_delayed_status);
thread_safe_decrement(delayed_rows_in_use,&LOCK_delayed_status);
thread_safe_increment(delayed_insert_writes,&LOCK_delayed_status);
pthread_mutex_lock(&mutex);

41
sql/sql_parse.cc
View file

@ -721,8 +721,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
case COM_INIT_DB:
{
LEX_STRING tmp;
statistic_increment(thd->status_var.com_stat[SQLCOM_CHANGE_DB],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_CHANGE_DB]);
thd->convert_string(&tmp, system_charset_info,
packet, packet_length-1, thd->charset());
if (!mysql_change_db(thd, &tmp, FALSE))
@ -757,7 +756,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
break;
}
statistic_increment(thd->status_var.com_other, &LOCK_status);
status_var_increment(thd->status_var.com_other);
thd->enable_slow_log= opt_log_slow_admin_statements;
db.str= thd->alloc(db_len + tbl_len + 2);
db.length= db_len;
@ -773,7 +772,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
}
case COM_CHANGE_USER:
{
statistic_increment(thd->status_var.com_other, &LOCK_status);
status_var_increment(thd->status_var.com_other);
char *user= (char*) packet, *packet_end= packet+ packet_length;
char *passwd= strend(user)+1;
@ -956,8 +955,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
/* used as fields initializator */
lex_start(thd);
statistic_increment(thd->status_var.com_stat[SQLCOM_SHOW_FIELDS],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_SHOW_FIELDS]);
bzero((char*) &table_list,sizeof(table_list));
if (thd->copy_db_to(&table_list.db, &dummy))
break;
@ -1023,8 +1021,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
LEX_STRING db, alias;
HA_CREATE_INFO create_info;
statistic_increment(thd->status_var.com_stat[SQLCOM_CREATE_DB],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_CREATE_DB]);
if (thd->LEX_STRING_make(&db, packet, packet_length -1) ||
thd->LEX_STRING_make(&alias, db.str, db.length) ||
check_db_name(&db))
@ -1043,8 +1040,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
}
case COM_DROP_DB: // QQ: To be removed
{
statistic_increment(thd->status_var.com_stat[SQLCOM_DROP_DB],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_DROP_DB]);
LEX_STRING db;
if (thd->LEX_STRING_make(&db, packet, packet_length - 1) ||
@ -1073,7 +1069,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
ushort flags;
uint32 slave_server_id;
statistic_increment(thd->status_var.com_other,&LOCK_status);
status_var_increment(thd->status_var.com_other);
thd->enable_slow_log= opt_log_slow_admin_statements;
if (check_global_access(thd, REPL_SLAVE_ACL))
break;
@ -1099,8 +1095,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
case COM_REFRESH:
{
bool not_used;
statistic_increment(thd->status_var.com_stat[SQLCOM_FLUSH],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_FLUSH]);
ulong options= (ulong) (uchar) packet[0];
if (check_global_access(thd,RELOAD_ACL))
break;
@ -1112,7 +1107,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
#ifndef EMBEDDED_LIBRARY
case COM_SHUTDOWN:
{
statistic_increment(thd->status_var.com_other, &LOCK_status);
status_var_increment(thd->status_var.com_other);
if (check_global_access(thd,SHUTDOWN_ACL))
break; /* purecov: inspected */
/*
@ -1164,8 +1159,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
#endif
general_log_print(thd, command, NullS);
statistic_increment(thd->status_var.com_stat[SQLCOM_SHOW_STATUS],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_SHOW_STATUS]);
calc_sum_of_all_status(&current_global_status_var);
if (!(uptime= (ulong) (thd->start_time - server_start_time)))
queries_per_second1000= 0;
@ -1201,12 +1195,11 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
break;
}
case COM_PING:
statistic_increment(thd->status_var.com_other, &LOCK_status);
status_var_increment(thd->status_var.com_other);
send_ok(thd); // Tell client we are alive
break;
case COM_PROCESS_INFO:
statistic_increment(thd->status_var.com_stat[SQLCOM_SHOW_PROCESSLIST],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_SHOW_PROCESSLIST]);
if (!thd->security_ctx->priv_user[0] &&
check_global_access(thd, PROCESS_ACL))
break;
@ -1217,15 +1210,14 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
break;
case COM_PROCESS_KILL:
{
statistic_increment(thd->status_var.com_stat[SQLCOM_KILL], &LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_KILL]);
ulong id=(ulong) uint4korr(packet);
sql_kill(thd,id,false);
break;
}
case COM_SET_OPTION:
{
statistic_increment(thd->status_var.com_stat[SQLCOM_SET_OPTION],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[SQLCOM_SET_OPTION]);
uint opt_command= uint2korr(packet);
switch (opt_command) {
@ -1244,7 +1236,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
break;
}
case COM_DEBUG:
statistic_increment(thd->status_var.com_other, &LOCK_status);
status_var_increment(thd->status_var.com_other);
if (check_global_access(thd, SUPER_ACL))
break; /* purecov: inspected */
mysql_print_status();
@ -1783,8 +1775,7 @@ mysql_execute_command(THD *thd)
#ifdef HAVE_REPLICATION
} /* endif unlikely slave */
#endif
statistic_increment(thd->status_var.com_stat[lex->sql_command],
&LOCK_status);
status_var_increment(thd->status_var.com_stat[lex->sql_command]);
switch (lex->sql_command) {
case SQLCOM_SHOW_EVENTS:

12
sql/sql_prepare.cc
View file

@ -2430,7 +2430,7 @@ void mysql_stmt_fetch(THD *thd, char *packet, uint packet_length)
/* First of all clear possible warnings from the previous command */
mysql_reset_thd_for_next_command(thd);
statistic_increment(thd->status_var.com_stmt_fetch, &LOCK_status);
status_var_increment(thd->status_var.com_stmt_fetch);
if (!(stmt= find_prepared_statement(thd, stmt_id, "mysql_stmt_fetch")))
DBUG_VOID_RETURN;
@ -2494,7 +2494,7 @@ void mysql_stmt_reset(THD *thd, char *packet)
/* First of all clear possible warnings from the previous command */
mysql_reset_thd_for_next_command(thd);
statistic_increment(thd->status_var.com_stmt_reset, &LOCK_status);
status_var_increment(thd->status_var.com_stmt_reset);
if (!(stmt= find_prepared_statement(thd, stmt_id, "mysql_stmt_reset")))
DBUG_VOID_RETURN;
@ -2598,7 +2598,7 @@ void mysql_stmt_get_longdata(THD *thd, char *packet, ulong packet_length)
#endif
DBUG_ENTER("mysql_stmt_get_longdata");
statistic_increment(thd->status_var.com_stmt_send_long_data, &LOCK_status);
status_var_increment(thd->status_var.com_stmt_send_long_data);
#ifndef EMBEDDED_LIBRARY
/* Minimal size of long data packet is 6 bytes */
if (packet_length <= MYSQL_LONG_DATA_HEADER)
@ -2849,7 +2849,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len)
However, it seems handy if com_stmt_prepare is increased always,
no matter what kind of prepare is processed.
*/
statistic_increment(thd->status_var.com_stmt_prepare, &LOCK_status);
status_var_increment(thd->status_var.com_stmt_prepare);
/*
alloc_query() uses thd->memroot && thd->query, so we should call
@ -2972,7 +2972,7 @@ bool Prepared_statement::execute(String *expanded_query, bool open_cursor)
Query_arena *old_stmt_arena;
bool error= TRUE;
statistic_increment(thd->status_var.com_stmt_execute, &LOCK_status);
status_var_increment(thd->status_var.com_stmt_execute);
/* Check if we got an error when sending long data */
if (state == Query_arena::ERROR)
@ -3094,7 +3094,7 @@ error:
bool Prepared_statement::deallocate()
{
/* We account deallocate in the same manner as mysql_stmt_close */
statistic_increment(thd->status_var.com_stmt_close, &LOCK_status);
status_var_increment(thd->status_var.com_stmt_close);
if (flags & (uint) IS_IN_USE)
{
my_error(ER_PS_NO_RECURSION, MYF(0));

20
sql/sql_select.cc
View file

@ -6282,8 +6282,7 @@ make_join_readinfo(JOIN *join, ulonglong options)
join->thd->server_status|=SERVER_QUERY_NO_GOOD_INDEX_USED;
tab->read_first_record= join_init_quick_read_record;
if (statistics)
statistic_increment(join->thd->status_var.select_range_check_count,
&LOCK_status);
status_var_increment(join->thd->status_var.select_range_check_count);
}
else
{
@ -6293,15 +6292,13 @@ make_join_readinfo(JOIN *join, ulonglong options)
if (tab->select && tab->select->quick)
{
if (statistics)
statistic_increment(join->thd->status_var.select_range_count,
&LOCK_status);
status_var_increment(join->thd->status_var.select_range_count);
}
else
{
join->thd->server_status|=SERVER_QUERY_NO_INDEX_USED;
if (statistics)
statistic_increment(join->thd->status_var.select_scan_count,
&LOCK_status);
status_var_increment(join->thd->status_var.select_scan_count);
}
}
else
@ -6309,15 +6306,13 @@ make_join_readinfo(JOIN *join, ulonglong options)
if (tab->select && tab->select->quick)
{
if (statistics)
statistic_increment(join->thd->status_var.select_full_range_join_count,
&LOCK_status);
status_var_increment(join->thd->status_var.select_full_range_join_count);
}
else
{
join->thd->server_status|=SERVER_QUERY_NO_INDEX_USED;
if (statistics)
statistic_increment(join->thd->status_var.select_full_join_count,
&LOCK_status);
status_var_increment(join->thd->status_var.select_full_join_count);
}
}
if (!table->no_keyread)
@ -9377,7 +9372,7 @@ create_tmp_table(THD *thd,TMP_TABLE_PARAM *param,List<Item> &fields,
(int) distinct, (int) save_sum_fields,
(ulong) rows_limit,test(group)));
statistic_increment(thd->status_var.created_tmp_tables, &LOCK_status);
status_var_increment(thd->status_var.created_tmp_tables);
if (use_temp_pool && !(test_flags & TEST_KEEP_TMP_TABLES))
temp_pool_slot = bitmap_lock_set_next(&temp_pool);
@ -10249,8 +10244,7 @@ static bool create_myisam_tmp_table(TABLE *table,TMP_TABLE_PARAM *param,
table->db_stat=0;
goto err;
}
statistic_increment(table->in_use->status_var.created_tmp_disk_tables,
&LOCK_status);
status_var_increment(table->in_use->status_var.created_tmp_disk_tables);
share->db_record_offset= 1;
DBUG_RETURN(0);
err: