The globally writable lock file for the mysql-test-run.pl port allocation was not always made globally writable, causing failures for subsequent runs by other users on the same machine. === modified file 'mysql-test/lib/mtr_unique.pm' --- mysql-test/lib/mtr_unique.pm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) Index: mysql-test/lib/mtr_unique.pm =================================================================== --- mysql-test/lib/mtr_unique.pm.orig 2009-04-07 13:40:36.000000000 +0200 +++ mysql-test/lib/mtr_unique.pm 2009-04-07 13:42:05.000000000 +0200 @@ -62,13 +62,14 @@ sub mtr_get_unique_id($$) { die 'lock file is a symbolic link'; } - chmod 0777, "$file.sem"; open SEM, ">", "$file.sem" or die "can't write to $file.sem"; + chmod 0777, "$file.sem"; flock SEM, LOCK_EX or die "can't lock $file.sem"; if(! -e $file) { open FILE, ">", $file or die "can't create $file"; close FILE; } + chmod 0777, $file; msg("HAVE THE LOCK"); @@ -76,7 +77,6 @@ sub mtr_get_unique_id($$) { die 'lock file is a symbolic link'; } - chmod 0777, $file; open FILE, "+<", $file or die "can't open $file"; #select undef,undef,undef,0.2; seek FILE, 0, 0; @@ -136,6 +136,7 @@ sub mtr_release_unique_id($) { } open SEM, ">", "$file.sem" or die "can't write to $file.sem"; + chmod 0777, "$file.sem"; flock SEM, LOCK_EX or die "can't lock $file.sem"; msg("HAVE THE LOCK"); @@ -148,6 +149,7 @@ sub mtr_release_unique_id($) { open FILE, ">", $file or die "can't create $file"; close FILE; } + chmod 0777, "$file.sem"; open FILE, "+<", $file or die "can't open $file"; #select undef,undef,undef,0.2; seek FILE, 0, 0; --

=== modified file 'sql/mysqld.cc' --- sql/mysqld.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: sql/mysqld.cc =================================================================== --- sql/mysqld.cc.orig 2009-04-07 13:40:36.000000000 +0200 +++ sql/mysqld.cc 2009-04-07 13:42:09.000000000 +0200 @@ -4815,10 +4815,10 @@ static bool read_init_file(char *file_na DBUG_ENTER("read_init_file"); DBUG_PRINT("enter",("name: %s",file_name)); if (!(file=my_fopen(file_name,O_RDONLY,MYF(MY_WME)))) - return(1); + DBUG_RETURN(1); bootstrap(file); (void) my_fclose(file,MYF(MY_WME)); - return 0; + DBUG_RETURN(0); } --

(Some versions of GCC implement struct_var = struct_var using memcpy(), which violates specs for memcpy() (not allowed for overlapping source and destination). GCC bug 19410: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19410 === modified file 'sql/sql_select.cc' --- sql/sql_select.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) Index: sql/sql_select.cc =================================================================== --- sql/sql_select.cc.orig 2009-04-07 13:40:36.000000000 +0200 +++ sql/sql_select.cc 2009-04-07 13:42:10.000000000 +0200 @@ -1994,8 +1994,17 @@ JOIN::exec() tmp_fields_list2, tmp_all_fields2, fields_list.elements, tmp_all_fields1)) DBUG_VOID_RETURN; - curr_join->tmp_fields_list2= tmp_fields_list2; - curr_join->tmp_all_fields2= tmp_all_fields2; +#ifdef HAVE_purify + /* + Some GCCs use memcpy() for struct assignment, even for x=x. + GCC bug 19410: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19410 + */ + if (curr_join != this) +#endif + { + curr_join->tmp_fields_list2= tmp_fields_list2; + curr_join->tmp_all_fields2= tmp_all_fields2; + } } curr_fields_list= &curr_join->tmp_fields_list2; curr_all_fields= &curr_join->tmp_all_fields2; --

(different system library versions generate slightly different stack traces to be suppressed). === modified file 'mysql-test/valgrind.supp' --- mysql-test/valgrind.supp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) Index: mysql-test/valgrind.supp =================================================================== --- mysql-test/valgrind.supp.orig 2009-04-07 13:40:36.000000000 +0200 +++ mysql-test/valgrind.supp 2009-04-07 13:42:12.000000000 +0200 @@ -379,7 +379,7 @@ } { - dlclose memory loss from plugin + dlclose memory loss from plugin variant 1 Memcheck:Leak fun:calloc fun:_dlerror_run @@ -388,6 +388,19 @@ } { + dlclose memory loss from plugin variant 2 + Memcheck:Leak + fun:malloc + fun:_dl_close_worker + fun:_dl_close + fun:_dl_catch_error + fun:_dlerror_run + fun:dlclose + fun:_Z15free_plugin_memP12st_plugin_dl + fun:_Z13plugin_dl_delPK19st_mysql_lex_string +} + +{ dlopen / ptread_cancel_init memory loss on Suse Linux 10.3 32/64 bit Memcheck:Leak fun:*alloc --

=== modified file 'mysql-test/valgrind.supp' --- mysql-test/valgrind.supp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) Index: mysql-test/valgrind.supp =================================================================== --- mysql-test/valgrind.supp.orig 2009-04-07 13:42:12.000000000 +0200 +++ mysql-test/valgrind.supp 2009-04-07 13:42:14.000000000 +0200 @@ -601,3 +601,19 @@ fun:dlopen* } +# +# In glibc (checked version 2.7), inet_ntoa allocates an 18-byte +# per-thread static buffer for the return value. That memory is freed +# at thread exit, however if called from the main thread, Valgrind +# does not see the free (test main.no-threads). +# +# Since inet_ntoa() does not allocate memory dynamically per-call, this +# suppression is safe. +# + +{ + inet_ntoa thread local storage + Memcheck:Leak + fun:malloc + fun:inet_ntoa +} --

The zlib code does operations on uninitialised parts of internal memory, but subsequently does bounds checks and ignores the results of any such undefined operations. This causes Valgrind to give false alarms. Fix by making memory allocated by zlib initialised #ifdef HAVE_purify. Also make zlib use my_malloc() for better leak checking etc. This is better than adding lots of suppressions for zlib operations: - No need to constantly add new suppressions for different versions of zlib. - Less risk of suppressing a real problem (like trying to compress/decompress uninitialised data). === modified file 'storage/archive/azio.c' --- storage/archive/azio.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) Index: storage/archive/azio.c =================================================================== --- storage/archive/azio.c.orig 2009-04-07 13:40:36.000000000 +0200 +++ storage/archive/azio.c 2009-04-07 13:42:16.000000000 +0200 @@ -37,6 +37,46 @@ void putLong(File file, uLong x); uLong getLong(azio_stream *s); void read_header(azio_stream *s, unsigned char *buffer); +/* + Valgrind normally gives false alarms for zlib operations, in the form of + "conditional jump depends on uninitialised values" etc. The reason is + explained in the zlib FAQ (http://www.zlib.net/zlib_faq.html#faq36): + + "That is intentional for performance reasons, and the output of deflate + is not affected." + + Also discussed on a blog + (http://www.sirena.org.uk/log/2006/02/19/zlib-generating-valgrind-warnings/): + + "...loop unrolling in the zlib library causes the mentioned + “Conditional jump or move depends on uninitialised value(s)” + warnings. These are safe since the results of the comparison are + subsequently ignored..." + + "the results of the calculations are discarded by bounds checking done + after the loop exits" + + Fix by initializing the memory allocated by zlib when running under Valgrind. + + This fix is safe, since such memory is only used internally by zlib, so we + will not hide any bugs in mysql this way. +*/ +static void *az_allocator(void *dummy, uInt items, uInt size) +{ + return my_malloc((size_t)items * (size_t)size, +#ifdef HAVE_purify + MY_ZEROFILL +#else + MYF(0) +#endif + ); +} + +static void az_free(void *dummy, void *address) +{ + my_free(address, MYF(MY_ALLOW_ZERO_PTR)); +} + /* =========================================================================== Opens a gzip (.gz) file for reading or writing. The mode parameter is as in fopen ("rb" or "wb"). The file is given either by file descriptor @@ -52,8 +92,8 @@ int az_open (azio_stream *s, const char int level = Z_DEFAULT_COMPRESSION; /* compression level */ int strategy = Z_DEFAULT_STRATEGY; /* compression strategy */ - s->stream.zalloc = (alloc_func)0; - s->stream.zfree = (free_func)0; + s->stream.zalloc = az_allocator; + s->stream.zfree = az_free; s->stream.opaque = (voidpf)0; memset(s->inbuf, 0, AZ_BUFSIZE_READ); memset(s->outbuf, 0, AZ_BUFSIZE_WRITE); --

After implementing pool_of_threads, the thd->connect_utime and thd->start_utime are not always correctly initialized (depending on which thread model is used). === modified file 'sql/mysqld.cc' --- sql/mysqld.cc | 1 + sql/scheduler.cc | 1 + 2 files changed, 2 insertions(+) Index: sql/mysqld.cc =================================================================== --- sql/mysqld.cc.orig 2009-04-07 13:42:09.000000000 +0200 +++ sql/mysqld.cc 2009-04-07 13:42:18.000000000 +0200 @@ -4839,6 +4839,7 @@ void handle_connection_in_main_thread(TH safe_mutex_assert_owner(&LOCK_thread_count); thread_cache_size=0; // Safety threads.append(thd); + thd->connect_utime= thd->start_utime= my_micro_time(); (void) pthread_mutex_unlock(&LOCK_thread_count); handle_one_connection((void*) thd); } Index: sql/scheduler.cc =================================================================== --- sql/scheduler.cc.orig 2009-04-07 13:40:35.000000000 +0200 +++ sql/scheduler.cc 2009-04-07 13:42:18.000000000 +0200 @@ -470,6 +470,7 @@ static void libevent_add_connection(THD DBUG_VOID_RETURN; } threads.append(thd); + thd->connect_utime= thd->start_utime= my_micro_time(); libevent_thd_add(thd); pthread_mutex_unlock(&LOCK_thread_count); --

If DbugParse() is called multiple times, the stack->keywords for the top stack frame could be overwritten without being freed, causing a memory leak reported by Valgrind. === modified file 'dbug/dbug.c' --- dbug/dbug.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) Index: dbug/dbug.c =================================================================== --- dbug/dbug.c.orig 2009-04-07 13:40:35.000000000 +0200 +++ dbug/dbug.c 2009-04-08 00:35:07.000000000 +0200 @@ -506,6 +506,9 @@ int DbugParse(CODE_STATE *cs, const char rel= control[0] == '+' || control[0] == '-'; if ((!rel || (!stack->out_file && !stack->next))) { + /* If overwriting previous state, be sure to free old to avoid leak. */ + if (stack->out_file) + FreeState(cs, stack, 0); stack->flags= 0; stack->delay= 0; stack->maxdepth= 0; @@ -1648,10 +1651,12 @@ static void FreeState(CODE_STATE *cs, st FreeList(state->processes); if (!is_shared(state, p_functions)) FreeList(state->p_functions); - if (!is_shared(state, out_file)) + if (!is_shared(state, out_file) && + state->out_file != stderr && state->out_file != stdout) DBUGCloseFile(cs, state->out_file); (void) fflush(cs->stack->out_file); - if (state->prof_file) + if (state->prof_file && + state->prof_file != stderr && state->prof_file != stdout) DBUGCloseFile(cs, state->prof_file); if (free_state) free((void*) state); --

The sql_string.h String class has some strange code to null-terminate the string for String::c_ptr(). It checks if the string is already null-terminated, and only writes the '\0' at the end if it is not already there. The reason for this I think is to avoid the re-allocation of the buffer to fit the null termination at append time if it is not needed due to never calling c_ptr(). However, the problem case here is a different one, where the null termination fits in the given buffer, but the value is uninitialised. In this case, checking before zeroing works (if in a somewhat twisted way). But it causes a false Valgrind alarm. This is a minimal fix; not sure if it is the right one. Maybe it would be better to fix the c_str() method to not use this tricky way of working? --- sql/sql_string.h | 8 ++++++++ 1 file changed, 8 insertions(+) Index: work-5.1-buildbot/sql/sql_string.h =================================================================== --- work-5.1-buildbot.orig/sql/sql_string.h 2009-04-08 00:34:49.000000000 +0200 +++ work-5.1-buildbot/sql/sql_string.h 2009-04-08 00:35:38.000000000 +0200 @@ -99,7 +99,15 @@ public: inline const char *ptr() const { return Ptr; } inline char *c_ptr() { + /* + This code null-terminates the string if not already null-terminated. + This works whether Ptr[str_length] is undefined or not, but if undefined + it generates a Valgrind error. So in the Valgrind case, null-terminate + unconditionally. + */ +#ifndef HAVE_purify if (!Ptr || Ptr[str_length]) /* Should be safe */ +#endif (void) realloc(str_length); return Ptr; } --

In set_var.cc, several methods construct a String object passing too large lenght for given buffer. The String class assumes 1 more byte is available after the given length for zero termination in String::c_ptr(). Fix by passing proper lenght in constructor call. --- sql/set_var.cc | 14 +++++++------- sql/sql_string.h | 4 ++++ 2 files changed, 11 insertions(+), 7 deletions(-) Index: work-5.1-buildbot/sql/set_var.cc =================================================================== --- work-5.1-buildbot.orig/sql/set_var.cc 2009-04-08 00:34:49.000000000 +0200 +++ work-5.1-buildbot/sql/set_var.cc 2009-04-08 00:35:43.000000000 +0200 @@ -1740,7 +1740,7 @@ bool sys_var::check_enum(THD *thd, set_v { char buff[STRING_BUFFER_USUAL_SIZE]; const char *value; - String str(buff, sizeof(buff), system_charset_info), *res; + String str(buff, sizeof(buff) - 1, system_charset_info), *res; if (var->value->result_type() == STRING_RESULT) { @@ -1777,7 +1777,7 @@ bool sys_var::check_set(THD *thd, set_va bool not_used; char buff[STRING_BUFFER_USUAL_SIZE], *error= 0; uint error_len= 0; - String str(buff, sizeof(buff), system_charset_info), *res; + String str(buff, sizeof(buff) - 1, system_charset_info), *res; if (var->value->result_type() == STRING_RESULT) { @@ -1942,7 +1942,7 @@ bool sys_var_thd_date_time_format::updat bool sys_var_thd_date_time_format::check(THD *thd, set_var *var) { char buff[STRING_BUFFER_USUAL_SIZE]; - String str(buff,sizeof(buff), system_charset_info), *res; + String str(buff,sizeof(buff) - 1, system_charset_info), *res; DATE_TIME_FORMAT *format; if (!(res=var->value->val_str(&str))) @@ -2047,7 +2047,7 @@ bool sys_var_collation::check(THD *thd, if (var->value->result_type() == STRING_RESULT) { char buff[STRING_BUFFER_USUAL_SIZE]; - String str(buff,sizeof(buff), system_charset_info), *res; + String str(buff,sizeof(buff) - 1, system_charset_info), *res; if (!(res=var->value->val_str(&str))) { my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), name, "NULL"); @@ -2082,7 +2082,7 @@ bool sys_var_character_set::check(THD *t if (var->value->result_type() == STRING_RESULT) { char buff[STRING_BUFFER_USUAL_SIZE]; - String str(buff,sizeof(buff), system_charset_info), *res; + String str(buff,sizeof(buff) - 1, system_charset_info), *res; if (!(res=var->value->val_str(&str))) { if (!nullable) @@ -3620,7 +3620,7 @@ bool sys_var_thd_storage_engine::check(T { char buff[STRING_BUFFER_USUAL_SIZE]; const char *value; - String str(buff, sizeof(buff), &my_charset_latin1), *res; + String str(buff, sizeof(buff) - 1, &my_charset_latin1), *res; var->save_result.plugin= NULL; if (var->value->result_type() == STRING_RESULT) @@ -3737,7 +3737,7 @@ sys_var_thd_sql_mode:: symbolic_mode_representation(THD *thd, ulonglong val, LEX_STRING *rep) { char buff[STRING_BUFFER_USUAL_SIZE*8]; - String tmp(buff, sizeof(buff), &my_charset_latin1); + String tmp(buff, sizeof(buff) - 1, &my_charset_latin1); tmp.length(0); Index: work-5.1-buildbot/sql/sql_string.h =================================================================== --- work-5.1-buildbot.orig/sql/sql_string.h 2009-04-08 00:35:38.000000000 +0200 +++ work-5.1-buildbot/sql/sql_string.h 2009-04-08 00:35:43.000000000 +0200 @@ -63,6 +63,10 @@ public: Ptr=(char*) str; str_length=(uint) strlen(str); Alloced_length=0; alloced=0; str_charset=cs; } + /* + NOTE: the following two contructors needs the size of memory for STR to be + at least LEN+1 (to make room for zero termination in c_ptr()). + */ String(const char *str,uint32 len, CHARSET_INFO *cs) { Ptr=(char*) str; str_length=len; Alloced_length=0; alloced=0; --