diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2023-03-28 11:44:24 +0300 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2023-03-28 11:44:24 +0300 |
commit | dfa90257f6a7be45b421798e56e7e9c1f27caf77 (patch) | |
tree | be28b2c37aa50b742be57ea4e046a42905c7604c | |
parent | a8b616d1e92ca9a4f4ba929aba41f64b19b2d169 (diff) | |
download | mariadb-git-dfa90257f6a7be45b421798e56e7e9c1f27caf77.tar.gz |
MDEV-30936 clang 15.0.7 -fsanitize=memory fails massively
handle_slave_io(), handle_slave_sql(), os_thread_exit():
Remove a redundant pthread_exit(nullptr) call, because it
would cause SIGSEGV.
mysql_print_status(): Add MEM_MAKE_DEFINED() to work around
some missing instrumentation around mallinfo2().
que_graph_free_stat_list(): Invoke que_node_get_next(node) before
que_graph_free_recursive(node). That is the logical and
MSAN_OPTIONS=poison_in_dtor=1 compatible way of freeing memory.
ins_node_t::~ins_node_t(): Invoke mem_heap_free(entry_sys_heap).
que_graph_free_recursive(): Rely on ins_node_t::~ins_node_t().
fts_t::~fts_t(): Invoke mem_heap_free(fts_heap).
fts_free(): Replace with direct calls to fts_t::~fts_t().
The failures in free_root() due to MSAN_OPTIONS=poison_in_dtor=1
will be covered in MDEV-30942.
-rw-r--r-- | sql/slave.cc | 6 | ||||
-rw-r--r-- | sql/sql_test.cc | 4 | ||||
-rw-r--r-- | storage/innobase/buf/buf0flu.cc | 4 | ||||
-rw-r--r-- | storage/innobase/dict/dict0dict.cc | 4 | ||||
-rw-r--r-- | storage/innobase/dict/dict0load.cc | 3 | ||||
-rw-r--r-- | storage/innobase/dict/dict0mem.cc | 2 | ||||
-rw-r--r-- | storage/innobase/fil/fil0crypt.cc | 4 | ||||
-rw-r--r-- | storage/innobase/fts/fts0fts.cc | 23 | ||||
-rw-r--r-- | storage/innobase/handler/ha_innodb.cc | 3 | ||||
-rw-r--r-- | storage/innobase/handler/handler0alter.cc | 6 | ||||
-rw-r--r-- | storage/innobase/include/fts0fts.h | 8 | ||||
-rw-r--r-- | storage/innobase/include/os0thread.h | 2 | ||||
-rw-r--r-- | storage/innobase/include/row0ins.h | 1 | ||||
-rw-r--r-- | storage/innobase/os/os0thread.cc | 4 | ||||
-rw-r--r-- | storage/innobase/que/que0que.cc | 13 | ||||
-rw-r--r-- | storage/innobase/row/row0mysql.cc | 4 | ||||
-rw-r--r-- | storage/innobase/trx/trx0roll.cc | 4 |
17 files changed, 32 insertions, 63 deletions
diff --git a/sql/slave.cc b/sql/slave.cc index d4de869ba58..17379eda5a6 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -5060,8 +5060,7 @@ err_during_init: DBUG_LEAVE; // Must match DBUG_ENTER() my_thread_end(); ERR_remove_state(0); - pthread_exit(0); - return 0; // Avoid compiler warnings + return nullptr; } /* @@ -5766,8 +5765,7 @@ err_during_init: DBUG_LEAVE; // Must match DBUG_ENTER() my_thread_end(); ERR_remove_state(0); - pthread_exit(0); - return 0; // Avoid compiler warnings + return nullptr; } diff --git a/sql/sql_test.cc b/sql/sql_test.cc index 07ebcc7a37a..41adcce2135 100644 --- a/sql/sql_test.cc +++ b/sql/sql_test.cc @@ -623,6 +623,10 @@ Next alarm time: %lu\n", #elif defined(HAVE_MALLINFO) struct mallinfo info= mallinfo(); #endif +#if __has_feature(memory_sanitizer) + /* Work around missing MSAN instrumentation */ + MEM_MAKE_DEFINED(&info, sizeof info); +#endif #if defined(HAVE_MALLINFO) || defined(HAVE_MALLINFO2) char llbuff[10][22]; printf("\nMemory status:\n\ diff --git a/storage/innobase/buf/buf0flu.cc b/storage/innobase/buf/buf0flu.cc index ee8bdaeb19d..77b1886ea5f 100644 --- a/storage/innobase/buf/buf0flu.cc +++ b/storage/innobase/buf/buf0flu.cc @@ -2465,9 +2465,7 @@ next: my_thread_end(); /* We count the number of threads in os_thread_exit(). A created thread should always use that to exit and not use return() to exit. */ - os_thread_exit(); - - OS_THREAD_DUMMY_RETURN; + return os_thread_exit(); } /** Initialize page_cleaner. */ diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index a607b2b9576..cfc39dd8e32 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -1962,8 +1962,8 @@ void dict_sys_t::remove(dict_table_t* table, bool lru, bool keep) #ifdef BTR_CUR_HASH_ADAPT if (table->fts) { fts_optimize_remove_table(table); - fts_free(table); - table->fts = NULL; + table->fts->~fts_t(); + table->fts = nullptr; } table->autoinc_mutex.lock(); diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc index b7ace9db87f..5db679dbfc9 100644 --- a/storage/innobase/dict/dict0load.cc +++ b/storage/innobase/dict/dict0load.cc @@ -3092,7 +3092,8 @@ func_exit: /* the table->fts could be created in dict_load_column when a user defined FTS_DOC_ID is present, but no FTS */ - fts_free(table); + table->fts->~fts_t(); + table->fts = nullptr; } else if (fts_optimize_wq) { fts_optimize_add_table(table); } else if (table->can_be_evicted) { diff --git a/storage/innobase/dict/dict0mem.cc b/storage/innobase/dict/dict0mem.cc index 9af27b6485d..bbdab865e3a 100644 --- a/storage/innobase/dict/dict0mem.cc +++ b/storage/innobase/dict/dict0mem.cc @@ -224,7 +224,7 @@ dict_mem_table_free( || DICT_TF2_FLAG_IS_SET(table, DICT_TF2_FTS_HAS_DOC_ID) || DICT_TF2_FLAG_IS_SET(table, DICT_TF2_FTS_ADD_DOC_ID)) { if (table->fts) { - fts_free(table); + table->fts->~fts_t(); } } diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 15e427a0ea5..3bc0f3e1f1f 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -2240,9 +2240,7 @@ DECLARE_THREAD(fil_crypt_thread)(void*) /* We count the number of threads in os_thread_exit(). A created thread should always use that to exit and not use return() to exit. */ - os_thread_exit(); - - OS_THREAD_DUMMY_RETURN; + return os_thread_exit(); } /********************************************************************* diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc index e35a7efa678..d0931de9614 100644 --- a/storage/innobase/fts/fts0fts.cc +++ b/storage/innobase/fts/fts0fts.cc @@ -833,7 +833,8 @@ void fts_clear_all(dict_table_t *table, trx_t *trx) fts_optimize_remove_table(table); fts_drop_tables(trx, table); - fts_free(table); + table->fts->~fts_t(); + table->fts= nullptr; DICT_TF2_FLAG_UNSET(table, DICT_TF2_FTS); } @@ -5350,14 +5351,14 @@ fts_t::~fts_t() { ut_ad(add_wq == NULL); - if (cache != NULL) { + if (cache) { fts_cache_clear(cache); fts_cache_destroy(cache); - cache = NULL; } /* There is no need to call ib_vector_free() on this->indexes because it is stored in this->fts_heap. */ + mem_heap_free(fts_heap); } /*********************************************************************//** @@ -5381,22 +5382,6 @@ fts_create( } /*********************************************************************//** -Free the FTS resources. */ -void -fts_free( -/*=====*/ - dict_table_t* table) /*!< in/out: table with FTS indexes */ -{ - fts_t* fts = table->fts; - - fts->~fts_t(); - - mem_heap_free(fts->fts_heap); - - table->fts = NULL; -} - -/*********************************************************************//** Take a FTS savepoint. */ UNIV_INLINE void diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index ef7b8e51794..e3dfcc8c321 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -12622,7 +12622,8 @@ int create_table_info_t::create_table(bool create_fk) m_table->name.m_name); if (m_table->fts) { - fts_free(m_table); + m_table->fts->~fts_t(); + m_table->fts = nullptr; } my_error(ER_WRONG_NAME_FOR_INDEX, MYF(0), diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index e454a997340..e3c802a7b46 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -1028,7 +1028,8 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx old_v_cols[i].~dict_v_col_t(); } if (instant_table->fts) { - fts_free(instant_table); + instant_table->fts->~fts_t(); + instant_table->fts = nullptr; } dict_mem_table_free(instant_table); } @@ -8753,7 +8754,8 @@ innobase_rollback_sec_index( && !DICT_TF2_FLAG_IS_SET(user_table, DICT_TF2_FTS_HAS_DOC_ID) && !innobase_fulltext_exist(table)) { - fts_free(user_table); + user_table->fts->~fts_t(); + user_table->fts = nullptr; } } diff --git a/storage/innobase/include/fts0fts.h b/storage/innobase/include/fts0fts.h index 011731c5e7f..9c2153b7ca3 100644 --- a/storage/innobase/include/fts0fts.h +++ b/storage/innobase/include/fts0fts.h @@ -607,14 +607,6 @@ fts_create( dict_table_t* table); /*!< out: table with FTS indexes */ -/**********************************************************************//** -Free the FTS resources. */ -void -fts_free( -/*=====*/ - dict_table_t* table); /*!< in/out: table with - FTS indexes */ - /*********************************************************************//** Run OPTIMIZE on the given table. @return DB_SUCCESS if all OK */ diff --git a/storage/innobase/include/os0thread.h b/storage/innobase/include/os0thread.h index ed989045f18..1e0bbbb2bd1 100644 --- a/storage/innobase/include/os0thread.h +++ b/storage/innobase/include/os0thread.h @@ -86,7 +86,7 @@ We do not return an error code because if there is one, we crash here. */ os_thread_t os_thread_create(os_thread_func_t func, void *arg= nullptr); /** Detach and terminate the current thread. */ -ATTRIBUTE_NORETURN void os_thread_exit(); +os_thread_ret_t os_thread_exit(); /*****************************************************************//** The thread sleeps at least the time given in microseconds. */ diff --git a/storage/innobase/include/row0ins.h b/storage/innobase/include/row0ins.h index 34427dc6dc7..75db0ad04b2 100644 --- a/storage/innobase/include/row0ins.h +++ b/storage/innobase/include/row0ins.h @@ -177,6 +177,7 @@ struct ins_node_t trx_id(0), entry_sys_heap(mem_heap_create(128)) { } + ~ins_node_t() { mem_heap_free(entry_sys_heap); } que_common_t common; /*!< node type: QUE_NODE_INSERT */ ulint ins_type;/* INS_VALUES, INS_SEARCHED, or INS_DIRECT */ dtuple_t* row; /*!< row to insert */ diff --git a/storage/innobase/os/os0thread.cc b/storage/innobase/os/os0thread.cc index f3533acfaac..ccf9b5c4a92 100644 --- a/storage/innobase/os/os0thread.cc +++ b/storage/innobase/os/os0thread.cc @@ -86,7 +86,7 @@ os_thread_t os_thread_create(os_thread_func_t func, void *arg) } /** Detach and terminate the current thread. */ -ATTRIBUTE_NORETURN void os_thread_exit() +os_thread_ret_t os_thread_exit() { #ifdef UNIV_DEBUG_THREAD_CREATION ib::info() << "Thread exits, id " << os_thread_get_curr_id(); @@ -100,8 +100,8 @@ ATTRIBUTE_NORETURN void os_thread_exit() ExitThread(0); #else pthread_detach(pthread_self()); - pthread_exit(NULL); #endif + OS_THREAD_DUMMY_RETURN; } /*****************************************************************//** diff --git a/storage/innobase/que/que0que.cc b/storage/innobase/que/que0que.cc index 125c50fbc8b..759c2cb95e2 100644 --- a/storage/innobase/que/que0que.cc +++ b/storage/innobase/que/que0que.cc @@ -360,9 +360,9 @@ que_graph_free_stat_list( que_node_t* node) /*!< in: first query graph node in the list */ { while (node) { + que_node_t* next = que_node_get_next(node); que_graph_free_recursive(node); - - node = que_node_get_next(node); + node = next; } } @@ -421,19 +421,10 @@ que_graph_free_recursive( break; case QUE_NODE_INSERT: - ins = static_cast<ins_node_t*>(node); que_graph_free_recursive(ins->select); - ins->select = NULL; - ins->~ins_node_t(); - - if (ins->entry_sys_heap != NULL) { - mem_heap_free(ins->entry_sys_heap); - ins->entry_sys_heap = NULL; - } - break; case QUE_NODE_PURGE: purge = static_cast<purge_node_t*>(node); diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index f815251bdd5..514d4b3ecd9 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -3225,8 +3225,8 @@ row_drop_ancillary_fts_tables( /* fts_que_graph_free_check_lock would try to acquire dict mutex lock */ table->fts->dict_locked = true; - - fts_free(table); + table->fts->~fts_t(); + table->fts = nullptr; } return(DB_SUCCESS); diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc index 23aa950a14a..fbe5a7e9b0a 100644 --- a/storage/innobase/trx/trx0roll.cc +++ b/storage/innobase/trx/trx0roll.cc @@ -845,9 +845,7 @@ DECLARE_THREAD(trx_rollback_all_recovered)(void*) /* We count the number of threads in os_thread_exit(). A created thread should always use that to exit and not use return() to exit. */ - os_thread_exit(); - - OS_THREAD_DUMMY_RETURN; + return os_thread_exit(); } /****************************************************************//** |