[Maria-developers] bzr commit into MariaDB 5.1, with Maria 1.5:maria branch (knielsen:2686)
#At lp:maria 2686 knielsen@knielsen-hq.org 2009-03-18 Fix some Valgrind warnings. modified: dbug/dbug.c mysql-test/valgrind.supp sql/mysqld.cc sql/scheduler.cc sql/sql_select.cc === modified file 'dbug/dbug.c' --- a/dbug/dbug.c 2009-03-12 22:27:35 +0000 +++ b/dbug/dbug.c 2009-03-18 14:08:05 +0000 @@ -506,6 +506,7 @@ int DbugParse(CODE_STATE *cs, const char rel= control[0] == '+' || control[0] == '-'; if ((!rel || (!stack->out_file && !stack->next))) { + FreeState(cs, stack, 0); stack->flags= 0; stack->delay= 0; stack->maxdepth= 0; @@ -1648,10 +1649,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->out_file != stderr && state->out_file != stdout) DBUGCloseFile(cs, state->prof_file); if (free_state) free((void*) state); === modified file 'mysql-test/valgrind.supp' --- a/mysql-test/valgrind.supp 2008-11-21 14:21:50 +0000 +++ b/mysql-test/valgrind.supp 2009-03-18 14:08:05 +0000 @@ -229,6 +229,44 @@ } { + libz deflate_slow 1 + Memcheck:Cond + fun:deflate_slow + fun:deflate + fun:do_flush + fun:azflush +} + +{ + libz deflate_slow 2 + Memcheck:Value8 + fun:deflate_slow + fun:deflate + fun:do_flush + fun:azflush +} + +{ + libz deflate_slow 3 + Memcheck:Cond + fun:deflate_slow + fun:deflate + fun:do_flush + fun:azclose +} + +{ + libz _tr_flush_block + Memcheck:Value8 + fun:compress_block + fun:_tr_flush_block + fun:deflate_slow + fun:deflate + fun:do_flush + fun:azflush +} + +{ libz deflate Memcheck:Cond obj:*/libz.so.* @@ -256,6 +294,14 @@ fun:do_flush } +{ + libz deflate4 + Memcheck:Cond + fun:deflate + fun:do_flush + fun:azclose +} + # # Warning from my_thread_init becasue mysqld dies before kill thread exists # @@ -379,7 +425,7 @@ } { - dlclose memory loss from plugin + dlclose memory loss from plugin variant 1 Memcheck:Leak fun:calloc fun:_dlerror_run @@ -388,6 +434,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 @@ -588,3 +647,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 +} === modified file 'sql/mysqld.cc' --- a/sql/mysqld.cc 2009-03-13 13:31:54 +0000 +++ b/sql/mysqld.cc 2009-03-18 14:08:05 +0000 @@ -4813,10 +4813,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); } @@ -4837,6 +4837,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); } === modified file 'sql/scheduler.cc' --- a/sql/scheduler.cc 2009-03-12 22:27:35 +0000 +++ b/sql/scheduler.cc 2009-03-18 14:08:05 +0000 @@ -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); === modified file 'sql/sql_select.cc' --- a/sql/sql_select.cc 2009-02-19 09:01:25 +0000 +++ b/sql/sql_select.cc 2009-03-18 14:08:05 +0000 @@ -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;
Hi, knielsen! On Mar 18, knielsen@knielsen-hq.org wrote:
#At lp:maria
2686 knielsen@knielsen-hq.org 2009-03-18 Fix some Valgrind warnings. modified: dbug/dbug.c mysql-test/valgrind.supp sql/mysqld.cc sql/scheduler.cc sql/sql_select.cc
=== modified file 'dbug/dbug.c' --- a/dbug/dbug.c 2009-03-12 22:27:35 +0000 +++ b/dbug/dbug.c 2009-03-18 14:08:05 +0000 @@ -506,6 +506,7 @@ int DbugParse(CODE_STATE *cs, const char rel= control[0] == '+' || control[0] == '-'; if ((!rel || (!stack->out_file && !stack->next))) { + FreeState(cs, stack, 0);
you may be freeing uuninitialized data here. What are you trying to fix anyway ?
stack->flags= 0; stack->delay= 0; stack->maxdepth= 0; @@ -1648,10 +1649,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->out_file != stderr && state->out_file != stdout)
typo. you obviously want s/out_file/prof_file/g
DBUGCloseFile(cs, state->prof_file); if (free_state) free((void*) state);
Regards / Mit vielen Grüßen, Sergei -- __ ___ ___ ____ __ / |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@sun.com> / /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect /_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB München 161028 <___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer Vorsitzender des Aufsichtsrates: Martin Häring
Sergei Golubchik <sergii@pisem.net> writes:
=== modified file 'dbug/dbug.c' --- a/dbug/dbug.c 2009-03-12 22:27:35 +0000 +++ b/dbug/dbug.c 2009-03-18 14:08:05 +0000 @@ -506,6 +506,7 @@ int DbugParse(CODE_STATE *cs, const char rel= control[0] == '+' || control[0] == '-'; if ((!rel || (!stack->out_file && !stack->next))) { + FreeState(cs, stack, 0);
you may be freeing uuninitialized data here.
Ok, that's not good, obviously.
What are you trying to fix anyway ?
The leak from this Valgrind warning: ==28234== 51 bytes in 1 blocks are definitely lost in loss record 3 of 7 ==28234== at 0x4C22FAB: malloc (vg_replace_malloc.c:207) ==28234== by 0xAA3452: DbugMalloc (dbug.c:2164) ==28234== by 0xAA2864: ListAddDel (dbug.c:1489) ==28234== by 0xAA009E: DbugParse (dbug.c:572) ==28234== by 0xAA0A27: _db_set_init_ (dbug.c:913) ==28234== by 0x66C4C3: mysqld_get_one_option (mysqld.cc:7942) ==28234== by 0xA89E5C: handle_options (my_getopt.c:530) ==28234== by 0x6720C4: get_options(int*, char**) (mysqld.cc:8524) ==28234== by 0x672590: init_common_variables(char const*, int, char**, char const**) (mysqld.cc:3312) ==28234== by 0x673EAB: main (mysqld.cc:4318) If I remember correctly, it is init_settings.keywords that is not de-allocated correctly when DbugParse is called multiple times. Due to BUG#43418, mysql-test-run was not detecting all Valgrind warnings. And after I fixed that bug, a number of additional warnings surfaced, this on included. If you have a better suggestion for silencing this leak, that would be great. Otherwise I need to look a bit deeper, I admit I did not properly check for the possibility of freeing uninitialised pointers.
@@ -1648,10 +1649,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->out_file != stderr && state->out_file != stdout)
typo. you obviously want s/out_file/prof_file/g
Well spotted! Thanks a lot, Sergei! - Kristian.
participants (3)
-
knielsen@knielsen-hq.org
-
Kristian Nielsen
-
Sergei Golubchik