Skip to content

Commit a10ae35

Browse files
Davi ArnautDavi Arnaut
Davi Arnaut
authored and
Davi Arnaut
committed
Bug#34043: Server loops excessively in _checkchunk() when safemalloc is enabled
Essentially, the problem is that safemalloc is excruciatingly slow as it checks all allocated blocks for overrun at each memory management primitive, yielding a almost exponential slowdown for the memory management functions (malloc, realloc, free). The overrun check basically consists of verifying some bytes of a block for certain magic keys, which catches some simple forms of overrun. Another minor problem is violation of aliasing rules and that its own internal list of blocks is prone to corruption. Another issue with safemalloc is rather the maintenance cost as the tool has a significant impact on the server code. Given the magnitude of memory debuggers available nowadays, especially those that are provided with the platform malloc implementation, maintenance of a in-house and largely obsolete memory debugger becomes a burden that is not worth the effort due to its slowness and lack of support for detecting more common forms of heap corruption. Since there are third-party tools that can provide the same functionality at a lower or comparable performance cost, the solution is to simply remove safemalloc. Third-party tools can provide the same functionality at a lower or comparable performance cost. The removal of safemalloc also allows a simplification of the malloc wrappers, removing quite a bit of kludge: redefinition of my_malloc, my_free and the removal of the unused second argument of my_free. Since free() always check whether the supplied pointer is null, redudant checks are also removed. Also, this patch adds unit testing for my_malloc and moves my_realloc implementation into the same file as the other memory allocation primitives.
1 parent d4cd1f8 commit a10ae35

File tree

217 files changed

+1012
-1821
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

217 files changed

+1012
-1821
lines changed

‎BUILD/SETUP.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,13 @@ fi
122122
# Override -DFORCE_INIT_OF_VARS from debug_cflags. It enables the macro
123123
# LINT_INIT(), which is only useful for silencing spurious warnings
124124
# of static analysis tools. We want LINT_INIT() to be a no-op in Valgrind.
125-
valgrind_flags="-USAFEMALLOC -UFORCE_INIT_OF_VARS -DHAVE_purify "
125+
valgrind_flags="-UFORCE_INIT_OF_VARS -DHAVE_purify "
126126
valgrind_flags="$valgrind_flags -DMYSQL_SERVER_SUFFIX=-valgrind-max"
127127
valgrind_configs="--with-valgrind"
128128
#
129129
# Used in -debug builds
130130
debug_cflags="-DUNIV_MUST_NOT_INLINE -DEXTRA_DEBUG -DFORCE_INIT_OF_VARS "
131-
debug_cflags="$debug_cflags -DSAFEMALLOC -DPEDANTIC_SAFEMALLOC -DSAFE_MUTEX"
131+
debug_cflags="$debug_cflags -DSAFE_MUTEX"
132132
error_inject="--with-error-inject "
133133
#
134134
# Base C++ flags for all builds

‎BUILD/build_mccge.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ set_ccache_usage()
10101010
set_valgrind_flags()
10111011
{
10121012
if test "x$valgrind_flag" = "xyes" ; then
1013-
loc_valgrind_flags="-USAFEMALLOC -UFORCE_INIT_OF_VARS -DHAVE_purify "
1013+
loc_valgrind_flags="-UFORCE_INIT_OF_VARS -DHAVE_purify "
10141014
loc_valgrind_flags="$loc_valgrind_flags -DMYSQL_SERVER_SUFFIX=-valgrind-max"
10151015
compiler_flags="$compiler_flags $loc_valgrind_flags"
10161016
with_flags="$with_flags --with-valgrind"
@@ -1066,7 +1066,7 @@ set_with_debug_flags()
10661066
if test "x$with_debug_flag" = "xyes" ; then
10671067
if test "x$developer_flag" = "xyes" ; then
10681068
loc_debug_flags="-DUNIV_MUST_NOT_INLINE -DEXTRA_DEBUG -DFORCE_INIT_OF_VARS "
1069-
loc_debug_flags="$loc_debug_flags -DSAFEMALLOC -DPEDANTIC_SAFEMALLOC"
1069+
loc_debug_flags="$loc_debug_flags"
10701070
compiler_flags="$compiler_flags $loc_debug_flags"
10711071
fi
10721072
fi

‎BUILD/compile-ia64-debug-max‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ gmake -k maintainer-clean || true
44
path=`dirname $0`
55
. "$path/autorun.sh"
66

7-
CC=ecc CFLAGS="-w1 -DEXTRA_DEBUG -DSAFEMALLOC -DSAFE_MUTEX -O2" CXX=ecc CXXFLAGS="-w1 -DEXTRA_DEBUG -DSAFEMALLOC -DSAFE_MUTEX -O2" ./configure --prefix=/usr/local/mysql --with-extra-charsets=complex --enable-thread-safe-client --with-mysqld-ldflags=-all-static --with-client-ldflags=-all-static --with-debug --with-innodb --with-embedded-server --with-archive-storage-engine
7+
CC=ecc CFLAGS="-w1 -DEXTRA_DEBUG -DSAFE_MUTEX -O2" CXX=ecc CXXFLAGS="-w1 -DEXTRA_DEBUG -DSAFE_MUTEX -O2" ./configure --prefix=/usr/local/mysql --with-extra-charsets=complex --enable-thread-safe-client --with-mysqld-ldflags=-all-static --with-client-ldflags=-all-static --with-debug --with-innodb --with-embedded-server --with-archive-storage-engine
88
gmake

‎CMakeLists.txt‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ ENDIF()
3434
SET(CUSTOM_C_FLAGS $ENV{CFLAGS})
3535

3636
OPTION(WITH_DEBUG "Use dbug/safemutex" OFF)
37-
OPTION(WITH_DEBUG_FULL "Use dbug and safemalloc/safemutex. Slow" OFF)
37+
OPTION(WITH_DEBUG_FULL "Use dbug and safemutex. Slow." OFF)
3838

3939
# Distinguish between community and non-community builds, with the
4040
# default being a community build. This does not impact the feature
@@ -175,14 +175,13 @@ IF(NOT CMAKE_BUILD_TYPE
175175
ENDIF()
176176
ENDIF()
177177

178-
# Add safemalloc and safemutex for debug condifurations, except on Windows
179-
# (C runtime library provides safemalloc functionality and safemutex has never
180-
# worked there)
178+
# Add safemutex for debug configurations, except on Windows
179+
# (safemutex has never worked on Windows)
181180
IF(WITH_DEBUG OR WITH_DEBUG_FULL AND NOT WIN32)
182181
FOREACH(LANG C CXX)
183182
IF(WITH_DEBUG_FULL)
184183
SET(CMAKE_${LANG}_FLAGS_DEBUG
185-
"${CMAKE_${LANG}_FLAGS_DEBUG} -DSAFEMALLOC -DSAFE_MUTEX")
184+
"${CMAKE_${LANG}_FLAGS_DEBUG} -DSAFE_MUTEX")
186185
ELSE()
187186
SET(CMAKE_${LANG}_FLAGS_DEBUG
188187
"${CMAKE_${LANG}_FLAGS_DEBUG} -DSAFE_MUTEX")

‎client/completion_hash.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
#include <my_global.h>
2424
#include <m_string.h>
25-
#undef SAFEMALLOC // Speed things up
2625
#include <my_sys.h>
2726
#include "completion_hash.h"
2827

@@ -213,7 +212,7 @@ void completion_hash_clean(HashTable *ht)
213212
void completion_hash_free(HashTable *ht)
214213
{
215214
completion_hash_clean(ht);
216-
my_free(ht->arBuckets, MYF(0));
215+
my_free(ht->arBuckets);
217216
}
218217

219218

‎client/mysql.cc‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,7 +1205,7 @@ int main(int argc,char *argv[])
12051205
strncmp(link_name, "/dev/null", 10) == 0)
12061206
{
12071207
/* The .mysql_history file is a symlink to /dev/null, don't use it */
1208-
my_free(histfile, MYF(MY_ALLOW_ZERO_PTR));
1208+
my_free(histfile);
12091209
histfile= 0;
12101210
}
12111211
}
@@ -1266,23 +1266,23 @@ sig_handler mysql_end(int sig)
12661266
glob_buffer.free();
12671267
old_buffer.free();
12681268
processed_prompt.free();
1269-
my_free(server_version,MYF(MY_ALLOW_ZERO_PTR));
1270-
my_free(opt_password,MYF(MY_ALLOW_ZERO_PTR));
1271-
my_free(opt_mysql_unix_port,MYF(MY_ALLOW_ZERO_PTR));
1272-
my_free(histfile,MYF(MY_ALLOW_ZERO_PTR));
1273-
my_free(histfile_tmp,MYF(MY_ALLOW_ZERO_PTR));
1274-
my_free(current_db,MYF(MY_ALLOW_ZERO_PTR));
1275-
my_free(current_host,MYF(MY_ALLOW_ZERO_PTR));
1276-
my_free(current_user,MYF(MY_ALLOW_ZERO_PTR));
1277-
my_free(full_username,MYF(MY_ALLOW_ZERO_PTR));
1278-
my_free(part_username,MYF(MY_ALLOW_ZERO_PTR));
1279-
my_free(default_prompt,MYF(MY_ALLOW_ZERO_PTR));
1269+
my_free(server_version);
1270+
my_free(opt_password);
1271+
my_free(opt_mysql_unix_port);
1272+
my_free(histfile);
1273+
my_free(histfile_tmp);
1274+
my_free(current_db);
1275+
my_free(current_host);
1276+
my_free(current_user);
1277+
my_free(full_username);
1278+
my_free(part_username);
1279+
my_free(default_prompt);
12801280
#ifdef HAVE_SMEM
1281-
my_free(shared_memory_base_name,MYF(MY_ALLOW_ZERO_PTR));
1281+
my_free(shared_memory_base_name);
12821282
#endif
1283-
my_free(current_prompt,MYF(MY_ALLOW_ZERO_PTR));
1283+
my_free(current_prompt);
12841284
while (embedded_server_arg_count > 1)
1285-
my_free(embedded_server_args[--embedded_server_arg_count],MYF(0));
1285+
my_free(embedded_server_args[--embedded_server_arg_count]);
12861286
mysql_server_end();
12871287
free_defaults(defaults_argv);
12881288
my_end(my_end_arg);
@@ -1736,7 +1736,7 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
17361736
if (argument)
17371737
{
17381738
char *start= argument;
1739-
my_free(opt_password, MYF(MY_ALLOW_ZERO_PTR));
1739+
my_free(opt_password);
17401740
opt_password= my_strdup(argument, MYF(MY_FAE));
17411741
while (*argument) *argument++= 'x'; // Destroy argument
17421742
if (*start)
@@ -1833,7 +1833,7 @@ static int get_options(int argc, char **argv)
18331833
if (argc == 1)
18341834
{
18351835
skip_updates= 0;
1836-
my_free(current_db, MYF(MY_ALLOW_ZERO_PTR));
1836+
my_free(current_db);
18371837
current_db= my_strdup(*argv, MYF(MY_WME));
18381838
}
18391839
if (tty_password)
@@ -2731,7 +2731,7 @@ static void get_current_db()
27312731
{
27322732
MYSQL_RES *res;
27332733

2734-
my_free(current_db, MYF(MY_ALLOW_ZERO_PTR));
2734+
my_free(current_db);
27352735
current_db= NULL;
27362736
/* In case of error below current_db will be NULL */
27372737
if (!mysql_query(&mysql, "SELECT DATABASE()") &&
@@ -4023,12 +4023,12 @@ com_connect(String *buffer, char *line)
40234023
tmp= get_arg(buff, 0);
40244024
if (tmp && *tmp)
40254025
{
4026-
my_free(current_db, MYF(MY_ALLOW_ZERO_PTR));
4026+
my_free(current_db);
40274027
current_db= my_strdup(tmp, MYF(MY_WME));
40284028
tmp= get_arg(buff, 1);
40294029
if (tmp)
40304030
{
4031-
my_free(current_host,MYF(MY_ALLOW_ZERO_PTR));
4031+
my_free(current_host);
40324032
current_host=my_strdup(tmp,MYF(MY_WME));
40334033
}
40344034
}
@@ -4200,7 +4200,7 @@ com_use(String *buffer __attribute__((unused)), char *line)
42004200
if (mysql_select_db(&mysql,tmp))
42014201
return put_error(&mysql);
42024202
}
4203-
my_free(current_db,MYF(MY_ALLOW_ZERO_PTR));
4203+
my_free(current_db);
42044204
current_db=my_strdup(tmp,MYF(MY_WME));
42054205
#ifdef HAVE_READLINE
42064206
if (select_db > 1)
@@ -4952,8 +4952,8 @@ static void add_int_to_prompt(int toadd)
49524952

49534953
static void init_username()
49544954
{
4955-
my_free(full_username,MYF(MY_ALLOW_ZERO_PTR));
4956-
my_free(part_username,MYF(MY_ALLOW_ZERO_PTR));
4955+
my_free(full_username);
4956+
my_free(part_username);
49574957

49584958
MYSQL_RES *result;
49594959
LINT_INIT(result);
@@ -4971,7 +4971,7 @@ static int com_prompt(String *buffer, char *line)
49714971
{
49724972
char *ptr=strchr(line, ' ');
49734973
prompt_counter = 0;
4974-
my_free(current_prompt,MYF(MY_ALLOW_ZERO_PTR));
4974+
my_free(current_prompt);
49754975
current_prompt=my_strdup(ptr ? ptr+1 : default_prompt,MYF(MY_WME));
49764976
if (!ptr)
49774977
tee_fprintf(stdout, "Returning to default PROMPT of %s\n", default_prompt);

‎client/mysqladmin.cc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
236236
if (argument)
237237
{
238238
char *start=argument;
239-
my_free(opt_password,MYF(MY_ALLOW_ZERO_PTR));
239+
my_free(opt_password);
240240
opt_password=my_strdup(argument,MYF(MY_FAE));
241241
while (*argument) *argument++= 'x'; /* Destroy argument */
242242
if (*start)
@@ -448,10 +448,10 @@ int main(int argc,char *argv[])
448448
} /* got connection */
449449

450450
mysql_close(&mysql);
451-
my_free(opt_password,MYF(MY_ALLOW_ZERO_PTR));
452-
my_free(user,MYF(MY_ALLOW_ZERO_PTR));
451+
my_free(opt_password);
452+
my_free(user);
453453
#ifdef HAVE_SMEM
454-
my_free(shared_memory_base_name,MYF(MY_ALLOW_ZERO_PTR));
454+
my_free(shared_memory_base_name);
455455
#endif
456456
free_defaults(save_argv);
457457
my_end(my_end_arg);
@@ -1008,8 +1008,8 @@ static int execute_commands(MYSQL *mysql,int argc, char **argv)
10081008
/* free up memory from prompted password */
10091009
if (typed_password != argv[1])
10101010
{
1011-
my_free(typed_password,MYF(MY_ALLOW_ZERO_PTR));
1012-
my_free(verified,MYF(MY_ALLOW_ZERO_PTR));
1011+
my_free(typed_password);
1012+
my_free(verified);
10131013
}
10141014
argc--; argv++;
10151015
break;

‎client/mysqlbinlog.cc‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,20 @@ TYPELIB base64_output_mode_typelib=
6868
{ array_elements(base64_output_mode_names) - 1, "",
6969
base64_output_mode_names, NULL };
7070
static enum_base64_output_mode opt_base64_output_mode= BASE64_OUTPUT_UNSPEC;
71-
static const char *opt_base64_output_mode_str= NullS;
72-
static const char* database= 0;
71+
static char *opt_base64_output_mode_str= NullS;
72+
static char* database= 0;
7373
static my_bool force_opt= 0, short_form= 0, remote_opt= 0;
7474
static my_bool debug_info_flag, debug_check_flag;
7575
static my_bool force_if_open_opt= 1;
7676
static ulonglong offset = 0;
77-
static const char* host = 0;
77+
static char* host = 0;
7878
static int port= 0;
7979
static uint my_end_arg;
8080
static const char* sock= 0;
8181
#ifdef HAVE_SMEM
8282
static char *shared_memory_base_name= 0;
8383
#endif
84-
static const char* user = 0;
84+
static char* user = 0;
8585
static char* pass = 0;
8686
static char *charset= 0;
8787

@@ -96,7 +96,7 @@ static my_time_t start_datetime= 0, stop_datetime= MY_TIME_T_MAX;
9696
static ulonglong rec_count= 0;
9797
static short binlog_flags = 0;
9898
static MYSQL* mysql = NULL;
99-
static const char* dirname_for_local_load= 0;
99+
static char* dirname_for_local_load= 0;
100100

101101
/**
102102
Pointer to the Format_description_log_event of the currently active binlog.
@@ -191,7 +191,7 @@ class Load_log_processor
191191
int init()
192192
{
193193
return init_dynamic_array(&file_names, sizeof(File_name_record),
194-
100,100 CALLER_INFO);
194+
100, 100);
195195
}
196196

197197
void init_by_dir_name(const char *dir)
@@ -213,7 +213,7 @@ class Load_log_processor
213213
{
214214
if (ptr->fname)
215215
{
216-
my_free(ptr->fname, MYF(MY_WME));
216+
my_free(ptr->fname);
217217
delete ptr->event;
218218
bzero((char *)ptr, sizeof(File_name_record));
219219
}
@@ -442,7 +442,7 @@ Exit_status Load_log_processor::process_first_event(const char *bname,
442442
{
443443
error("Could not construct local filename %s%s.",
444444
target_dir_name,bname);
445-
my_free(fname, MYF(0));
445+
my_free(fname);
446446
delete ce;
447447
DBUG_RETURN(ERROR_STOP);
448448
}
@@ -458,7 +458,7 @@ Exit_status Load_log_processor::process_first_event(const char *bname,
458458
if (set_dynamic(&file_names, (uchar*)&rec, file_id))
459459
{
460460
error("Out of memory.");
461-
my_free(fname, MYF(0));
461+
my_free(fname);
462462
delete ce;
463463
DBUG_RETURN(ERROR_STOP);
464464
}
@@ -822,7 +822,7 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
822822
*/
823823
convert_path_to_forward_slashes((char*) ce->fname);
824824
ce->print(result_file, print_event_info, TRUE);
825-
my_free((char*)ce->fname,MYF(MY_WME));
825+
my_free((void*)ce->fname);
826826
delete ce;
827827
}
828828
else
@@ -887,7 +887,7 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
887887
}
888888

889889
if (fname)
890-
my_free(fname, MYF(MY_WME));
890+
my_free(fname);
891891
break;
892892
}
893893
case TABLE_MAP_EVENT:
@@ -1222,11 +1222,11 @@ static void warning(const char *format,...)
12221222
*/
12231223
static void cleanup()
12241224
{
1225-
my_free(pass,MYF(MY_ALLOW_ZERO_PTR));
1226-
my_free((char*) database, MYF(MY_ALLOW_ZERO_PTR));
1227-
my_free((char*) host, MYF(MY_ALLOW_ZERO_PTR));
1228-
my_free((char*) user, MYF(MY_ALLOW_ZERO_PTR));
1229-
my_free((char*) dirname_for_local_load, MYF(MY_ALLOW_ZERO_PTR));
1225+
my_free(pass);
1226+
my_free(database);
1227+
my_free(host);
1228+
my_free(user);
1229+
my_free(dirname_for_local_load);
12301230

12311231
delete glob_description_event;
12321232
if (mysql)
@@ -1306,7 +1306,7 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
13061306
argument= (char*) ""; // Don't require password
13071307
if (argument)
13081308
{
1309-
my_free(pass,MYF(MY_ALLOW_ZERO_PTR));
1309+
my_free(pass);
13101310
char *start=argument;
13111311
pass= my_strdup(argument,MYF(MY_FAE));
13121312
while (*argument) *argument++= 'x'; /* Destroy argument */

‎client/mysqlcheck.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
291291
if (argument)
292292
{
293293
char *start = argument;
294-
my_free(opt_password, MYF(MY_ALLOW_ZERO_PTR));
294+
my_free(opt_password);
295295
opt_password = my_strdup(argument, MYF(MY_FAE));
296296
while (*argument) *argument++= 'x'; /* Destroy argument */
297297
if (*start)
@@ -470,7 +470,7 @@ static int process_selected_tables(char *db, char **table_names, int tables)
470470
}
471471
*--end = 0;
472472
handle_request_for_tables(table_names_comma_sep + 1, (uint) (tot_length - 1));
473-
my_free(table_names_comma_sep, MYF(0));
473+
my_free(table_names_comma_sep);
474474
}
475475
else
476476
for (; tables > 0; tables--, table_names++)
@@ -569,7 +569,7 @@ static int process_all_tables_in_db(char *database)
569569
*--end = 0;
570570
if (tot_length)
571571
handle_request_for_tables(tables + 1, tot_length - 1);
572-
my_free(tables, MYF(0));
572+
my_free(tables);
573573
}
574574
else
575575
{
@@ -727,7 +727,7 @@ static int handle_request_for_tables(char *tables, uint length)
727727
return 1;
728728
}
729729
print_result();
730-
my_free(query, MYF(0));
730+
my_free(query);
731731
return 0;
732732
}
733733

@@ -899,9 +899,9 @@ int main(int argc, char **argv)
899899
dbDisconnect(current_host);
900900
if (opt_auto_repair)
901901
delete_dynamic(&tables4repair);
902-
my_free(opt_password, MYF(MY_ALLOW_ZERO_PTR));
902+
my_free(opt_password);
903903
#ifdef HAVE_SMEM
904-
my_free(shared_memory_base_name,MYF(MY_ALLOW_ZERO_PTR));
904+
my_free(shared_memory_base_name);
905905
#endif
906906
my_end(my_end_arg);
907907
return(first_error!=0);

0 commit comments

Comments
 (0)