From 3c773cd855ba72439ee06a13dcf6143347dd984b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 14 May 2020 13:35:52 +0300 Subject: [PATCH 01/10] MDEV-19622: Fix a TokuDB result --- .../tokudb_parts/r/partition_auto_increment_tokudb.result | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/storage/tokudb/mysql-test/tokudb_parts/r/partition_auto_increment_tokudb.result b/storage/tokudb/mysql-test/tokudb_parts/r/partition_auto_increment_tokudb.result index b18f970d2ce..14b6052a7d3 100644 --- a/storage/tokudb/mysql-test/tokudb_parts/r/partition_auto_increment_tokudb.result +++ b/storage/tokudb/mysql-test/tokudb_parts/r/partition_auto_increment_tokudb.result @@ -1115,5 +1115,13 @@ SELECT * FROM t1; a 0 DROP TABLE t1; +# +# MDEV-19622 Assertion failures in +# ha_partition::set_auto_increment_if_higher upon UPDATE on Aria table +# +CREATE OR REPLACE TABLE t1 (pk INT AUTO_INCREMENT, a INT, KEY(pk)) ENGINE=myisam PARTITION BY HASH(a); +INSERT INTO t1 VALUES (1,1),(2,2); +UPDATE t1 SET pk = 0; +DROP TABLE t1; ############################################################################## SET GLOBAL tokudb_prelock_empty = @tokudb_prelock_empty_saved; From 11147bea20596b6f7bd3ff907d379667ae44b149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 14 May 2020 09:05:04 +0300 Subject: [PATCH 02/10] Fix GCC -Wstringop-truncation --- libmariadb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmariadb b/libmariadb index 2759b87d729..cdfecebc993 160000 --- a/libmariadb +++ b/libmariadb @@ -1 +1 @@ -Subproject commit 2759b87d72926b7c9b5426437a7c8dd15ff57945 +Subproject commit cdfecebc9932a0dd5516c10505bfe78d79132e7d From 7d51c35988186da89645d9873c5e9386fee0a9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 14 May 2020 09:06:38 +0300 Subject: [PATCH 03/10] Fix GCC -Wnonnull --- client/mysqltest.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index b363a3217cd..ada0da2e8ab 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -587,9 +587,10 @@ ATTRIBUTE_NORETURN static void cleanup_and_exit(int exit_code); ATTRIBUTE_NORETURN -void really_die(const char *msg); +static void really_die(const char *msg); void report_or_die(const char *fmt, ...); -void die(const char *fmt, ...); +ATTRIBUTE_NORETURN +static void die(const char *fmt, ...); static void make_error_message(char *buf, size_t len, const char *fmt, va_list args); ATTRIBUTE_NORETURN ATTRIBUTE_FORMAT(printf, 1, 2) void abort_not_supported_test(const char *fmt, ...); @@ -1554,7 +1555,7 @@ static void make_error_message(char *buf, size_t len, const char *fmt, va_list a s+= my_snprintf(s, end -s, "\n", start_lineno); } -void die(const char *fmt, ...) +static void die(const char *fmt, ...) { char buff[DIE_BUFF_SIZE]; va_list args; @@ -1563,7 +1564,7 @@ void die(const char *fmt, ...) really_die(buff); } -void really_die(const char *msg) +static void really_die(const char *msg) { static int dying= 0; fflush(stdout); From a12aed0398a33e113befc25e49967c340f96025f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 14 May 2020 10:55:32 +0300 Subject: [PATCH 04/10] Fix GCC 9.3.0 -Wunused-but-set-variable --- storage/innobase/btr/btr0cur.cc | 2 +- storage/innobase/fsp/fsp0fsp.cc | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index a187402328f..2bf657f037f 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -3050,7 +3050,7 @@ btr_cur_optimistic_insert( page_t* page; rec_t* dummy; ibool leaf; - ibool reorg; + ibool reorg __attribute__((unused)); ibool inherit = TRUE; ulint rec_size; dberr_t err; diff --git a/storage/innobase/fsp/fsp0fsp.cc b/storage/innobase/fsp/fsp0fsp.cc index 78d562be9a3..d308ff9de71 100644 --- a/storage/innobase/fsp/fsp0fsp.cc +++ b/storage/innobase/fsp/fsp0fsp.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, 2019, MariaDB Corporation. +Copyright (c) 2017, 2020, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -3185,7 +3185,6 @@ fseg_free_extent( #endif /* BTR_CUR_HASH_ADAPT */ mtr_t* mtr) { - ulint first_page_in_extent; xdes_t* descr; ulint not_full_n_used; ulint descr_n_used; @@ -3200,7 +3199,9 @@ fseg_free_extent( == FSEG_MAGIC_N_VALUE); ut_d(space->modify_check(*mtr)); - first_page_in_extent = page - (page % FSP_EXTENT_SIZE); +#if defined BTR_CUR_HASH_ADAPT || defined UNIV_DEBUG + const ulint first_page_in_extent = page - (page % FSP_EXTENT_SIZE); +#endif /* BTR_CUR_HASH_ADAPT || UNIV_DEBUG */ #ifdef BTR_CUR_HASH_ADAPT if (ahi) { From fc58c1721631fcc6c9414482b3b7e90cd8e7325d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 14 May 2020 12:06:25 +0300 Subject: [PATCH 05/10] MDEV-21336 Memory leaks related to innodb_debug_sync This essentially reverts commit b393e2cb0c079b30563dcc87a62002c9c778643c. The leak might have been fixed, but because the DEBUG_SYNC instrumentation for InnoDB purge threads was reverted in 10.5 commit 5e62b6a5e06eb02cbde1e34e95e26f42d87fce02 as part of introducing a thread pool, it is easiest to revert the entire change. --- .../gcol/r/innodb_virtual_debug_purge.result | 45 ------------- .../gcol/t/innodb_virtual_debug_purge.test | 64 ------------------- .../suite/sys_vars/r/sysvars_innodb.result | 12 ---- storage/innobase/fil/fil0pagecompress.cc | 1 - storage/innobase/handler/ha_innodb.cc | 38 ----------- storage/innobase/include/srv0srv.h | 7 -- storage/innobase/row/row0purge.cc | 21 ------ storage/innobase/row/row0vers.cc | 1 - storage/innobase/srv/srv0srv.cc | 13 ---- 9 files changed, 202 deletions(-) diff --git a/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result b/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result index 11dfdf8b52e..c9d95dae579 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_debug_purge.result @@ -233,48 +233,3 @@ set global debug_dbug= @saved_dbug; drop table t1; set debug_sync=reset; SET GLOBAL innodb_purge_rseg_truncate_frequency = @saved_frequency; -# -# MDEV-18546 ASAN heap-use-after-free -# in innobase_get_computed_value / row_purge -# -CREATE TABLE t1 ( -pk INT AUTO_INCREMENT, -b BIT(15), -v BIT(15) AS (b) VIRTUAL, -PRIMARY KEY(pk), -UNIQUE(v) -) ENGINE=InnoDB; -INSERT IGNORE INTO t1 (b) VALUES -(NULL),(b'011'),(b'000110100'), -(b'01101101010'),(b'01111001001011'),(NULL); -SET GLOBAL innodb_debug_sync = "ib_clust_v_col_before_row_allocated " - "SIGNAL before_row_allocated " - "WAIT_FOR flush_unlock"; -SET GLOBAL innodb_debug_sync = "ib_open_after_dict_open " - "SIGNAL purge_open " - "WAIT_FOR select_open"; -SET @saved_dbug= @@GLOBAL.debug_dbug; -set global debug_dbug= "d,ib_purge_virtual_index_callback"; -connect purge_waiter,localhost,root; -SET debug_sync= "now WAIT_FOR before_row_allocated"; -connection default; -REPLACE INTO t1 (pk, b) SELECT pk, b FROM t1; -connection purge_waiter; -connection default; -disconnect purge_waiter; -FLUSH TABLES; -SET GLOBAL innodb_debug_sync = reset; -SET debug_sync= "now SIGNAL flush_unlock WAIT_FOR purge_open"; -SET GLOBAL innodb_debug_sync = reset; -SET debug_sync= "ib_open_after_dict_open SIGNAL select_open"; -SELECT * FROM t1; -pk b v -1 NULL NULL -2   -3 4 4 -4 j j -5 K K -6 NULL NULL -DROP TABLE t1; -SET debug_sync= reset; -set global debug_dbug= @saved_dbug; diff --git a/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test b/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test index 62017e55fbc..238a4937e9f 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_debug_purge.test @@ -323,67 +323,3 @@ drop table t1; --source include/wait_until_count_sessions.inc set debug_sync=reset; SET GLOBAL innodb_purge_rseg_truncate_frequency = @saved_frequency; - ---echo # ---echo # MDEV-18546 ASAN heap-use-after-free ---echo # in innobase_get_computed_value / row_purge ---echo # - -CREATE TABLE t1 ( - pk INT AUTO_INCREMENT, - b BIT(15), - v BIT(15) AS (b) VIRTUAL, - PRIMARY KEY(pk), - UNIQUE(v) -) ENGINE=InnoDB; -INSERT IGNORE INTO t1 (b) VALUES - (NULL),(b'011'),(b'000110100'), - (b'01101101010'),(b'01111001001011'),(NULL); - -SET GLOBAL innodb_debug_sync = "ib_clust_v_col_before_row_allocated " - "SIGNAL before_row_allocated " - "WAIT_FOR flush_unlock"; -SET GLOBAL innodb_debug_sync = "ib_open_after_dict_open " - "SIGNAL purge_open " - "WAIT_FOR select_open"; - -# In 10.2 trx_undo_roll_ptr_is_insert(t_roll_ptr) condition never pass in purge, -# so this condition is forced to pass in row_vers_old_has_index_entry -SET @saved_dbug= @@GLOBAL.debug_dbug; -set global debug_dbug= "d,ib_purge_virtual_index_callback"; - -# The purge starts from REPLACE command. To avoid possible race, separate -# connection is used. ---connect(purge_waiter,localhost,root) ---send -SET debug_sync= "now WAIT_FOR before_row_allocated"; - ---connection default -REPLACE INTO t1 (pk, b) SELECT pk, b FROM t1; - ---connection purge_waiter -# Now we will definitely catch ib_clust_v_col_before_row_allocated ---reap ---connection default ---disconnect purge_waiter - -# purge hangs on the sync point. table is purged, ref_count is set to 0 -FLUSH TABLES; - -# Avoid hang on repeating purge. -# Reset Will be applied after first record is purged -SET GLOBAL innodb_debug_sync = reset; - -SET debug_sync= "now SIGNAL flush_unlock WAIT_FOR purge_open"; - -# Avoid hang on repeating purge -SET GLOBAL innodb_debug_sync = reset; - -# select unblocks purge thread -SET debug_sync= "ib_open_after_dict_open SIGNAL select_open"; -SELECT * FROM t1; - -# Cleanup -DROP TABLE t1; -SET debug_sync= reset; -set global debug_dbug= @saved_dbug; \ No newline at end of file diff --git a/mysql-test/suite/sys_vars/r/sysvars_innodb.result b/mysql-test/suite/sys_vars/r/sysvars_innodb.result index 08739f6bf28..2b83e02d45a 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_innodb.result +++ b/mysql-test/suite/sys_vars/r/sysvars_innodb.result @@ -654,18 +654,6 @@ NUMERIC_BLOCK_SIZE NULL ENUM_VALUE_LIST OFF,ON READ_ONLY NO COMMAND_LINE_ARGUMENT REQUIRED -VARIABLE_NAME INNODB_DEBUG_SYNC -SESSION_VALUE NULL -DEFAULT_VALUE -VARIABLE_SCOPE GLOBAL -VARIABLE_TYPE VARCHAR -VARIABLE_COMMENT debug_sync for innodb purge threads. Use it to set up sync points for all purge threads at once. The commands will be applied sequentially at the beginning of purging the next undo record. -NUMERIC_MIN_VALUE NULL -NUMERIC_MAX_VALUE NULL -NUMERIC_BLOCK_SIZE NULL -ENUM_VALUE_LIST NULL -READ_ONLY NO -COMMAND_LINE_ARGUMENT NONE VARIABLE_NAME INNODB_DEFAULT_ENCRYPTION_KEY_ID SESSION_VALUE 1 DEFAULT_VALUE 1 diff --git a/storage/innobase/fil/fil0pagecompress.cc b/storage/innobase/fil/fil0pagecompress.cc index 89b8efc4e9b..2c0e4222660 100644 --- a/storage/innobase/fil/fil0pagecompress.cc +++ b/storage/innobase/fil/fil0pagecompress.cc @@ -27,7 +27,6 @@ Updated 14/02/2015 #include "fil0fil.h" #include "fil0pagecompress.h" -#include #include #include "mem0mem.h" diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 68011147bc5..b917bdab19e 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -19523,33 +19523,6 @@ innodb_log_checksums_update( thd, *static_cast(save)); } -#ifdef UNIV_DEBUG -static -void -innobase_debug_sync_callback(srv_slot_t *slot, const void *value) -{ - const char *value_str = *static_cast(value); - size_t len = strlen(value_str) + 1; - - - // One allocatoin for list node object and value. - void *buf = ut_malloc_nokey(sizeof(srv_slot_t::debug_sync_t) + len); - srv_slot_t::debug_sync_t *sync = new(buf) srv_slot_t::debug_sync_t(); - strcpy(reinterpret_cast(&sync[1]), value_str); - - rw_lock_x_lock(&slot->debug_sync_lock); - UT_LIST_ADD_LAST(slot->debug_sync, sync); - rw_lock_x_unlock(&slot->debug_sync_lock); -} -static -void -innobase_debug_sync_set(THD *thd, st_mysql_sys_var*, void *, const void *value) -{ - srv_for_each_thread(SRV_WORKER, innobase_debug_sync_callback, value); - srv_for_each_thread(SRV_PURGE, innobase_debug_sync_callback, value); -} -#endif - static SHOW_VAR innodb_status_variables_export[]= { {"Innodb", (char*) &show_innodb_vars, SHOW_FUNC}, {NullS, NullS, SHOW_LONG} @@ -21162,16 +21135,6 @@ static MYSQL_SYSVAR_BOOL(debug_force_scrubbing, 0, "Perform extra scrubbing to increase test exposure", NULL, NULL, FALSE); - -char *innobase_debug_sync; -static MYSQL_SYSVAR_STR(debug_sync, innobase_debug_sync, - PLUGIN_VAR_NOCMDARG, - "debug_sync for innodb purge threads. " - "Use it to set up sync points for all purge threads " - "at once. The commands will be applied sequentially at " - "the beginning of purging the next undo record.", - NULL, - innobase_debug_sync_set, NULL); #endif /* UNIV_DEBUG */ static MYSQL_SYSVAR_BOOL(instrument_semaphores, innodb_instrument_semaphores, @@ -21395,7 +21358,6 @@ static struct st_mysql_sys_var* innobase_system_variables[]= { MYSQL_SYSVAR(background_scrub_data_check_interval), #ifdef UNIV_DEBUG MYSQL_SYSVAR(debug_force_scrubbing), - MYSQL_SYSVAR(debug_sync), #endif MYSQL_SYSVAR(instrument_semaphores), MYSQL_SYSVAR(buf_dump_status_frequency), diff --git a/storage/innobase/include/srv0srv.h b/storage/innobase/include/srv0srv.h index 767e24e9265..5214953f308 100644 --- a/storage/innobase/include/srv0srv.h +++ b/storage/innobase/include/srv0srv.h @@ -1147,13 +1147,6 @@ struct srv_slot_t{ to do */ que_thr_t* thr; /*!< suspended query thread (only used for user threads) */ -#ifdef UNIV_DEBUG - struct debug_sync_t { - UT_LIST_NODE_T(debug_sync_t) debug_sync_list; - }; - UT_LIST_BASE_NODE_T(debug_sync_t) debug_sync; - rw_lock_t debug_sync_lock; -#endif }; #ifdef UNIV_DEBUG diff --git a/storage/innobase/row/row0purge.cc b/storage/innobase/row/row0purge.cc index 5f2a854ba92..f4fd617f154 100644 --- a/storage/innobase/row/row0purge.cc +++ b/storage/innobase/row/row0purge.cc @@ -46,7 +46,6 @@ Created 3/14/1997 Heikki Tuuri #include "handler.h" #include "ha_innodb.h" #include "fil0fil.h" -#include "debug_sync.h" /************************************************************************* IMPORTANT NOTE: Any operation that generates redo MUST check that there @@ -1209,26 +1208,6 @@ row_purge_step( node->start(); -#ifdef UNIV_DEBUG - srv_slot_t *slot = thr->thread_slot; - ut_ad(slot); - - rw_lock_x_lock(&slot->debug_sync_lock); - while (UT_LIST_GET_LEN(slot->debug_sync)) { - srv_slot_t::debug_sync_t *sync = - UT_LIST_GET_FIRST(slot->debug_sync); - const char* sync_str = reinterpret_cast(&sync[1]); - bool result = debug_sync_set_action(current_thd, - sync_str, - strlen(sync_str)); - ut_a(!result); - - UT_LIST_REMOVE(slot->debug_sync, sync); - ut_free(sync); - } - rw_lock_x_unlock(&slot->debug_sync_lock); -#endif - if (!(node->undo_recs == NULL || ib_vector_is_empty(node->undo_recs))) { trx_purge_rec_t*purge_rec; diff --git a/storage/innobase/row/row0vers.cc b/storage/innobase/row/row0vers.cc index b148f7f54da..3627c659231 100644 --- a/storage/innobase/row/row0vers.cc +++ b/storage/innobase/row/row0vers.cc @@ -468,7 +468,6 @@ row_vers_build_clust_v_col( vcol_info->set_used(); maria_table = vcol_info->table(); } - DEBUG_SYNC(current_thd, "ib_clust_v_col_before_row_allocated"); innobase_allocate_row_for_vcol(thd, index, &local_heap, diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc index 44c07693e6a..6eb67a1b2d4 100644 --- a/storage/innobase/srv/srv0srv.cc +++ b/storage/innobase/srv/srv0srv.cc @@ -2600,13 +2600,6 @@ DECLARE_THREAD(srv_worker_thread)( slot = srv_reserve_slot(SRV_WORKER); -#ifdef UNIV_DEBUG - UT_LIST_INIT(slot->debug_sync, - &srv_slot_t::debug_sync_t::debug_sync_list); - rw_lock_create(PFS_NOT_INSTRUMENTED, &slot->debug_sync_lock, - SYNC_NO_ORDER_CHECK); -#endif - ut_a(srv_n_purge_threads > 1); ut_a(ulong(my_atomic_loadlint(&srv_sys.n_threads_active[SRV_WORKER])) < srv_n_purge_threads); @@ -2854,12 +2847,6 @@ DECLARE_THREAD(srv_purge_coordinator_thread)( slot = srv_reserve_slot(SRV_PURGE); -#ifdef UNIV_DEBUG - UT_LIST_INIT(slot->debug_sync, - &srv_slot_t::debug_sync_t::debug_sync_list); - rw_lock_create(PFS_NOT_INSTRUMENTED, &slot->debug_sync_lock, - SYNC_NO_ORDER_CHECK); -#endif ulint rseg_history_len = trx_sys->rseg_history_len; do { From ee5152fc4bc0f164d546ce87c1d36d1a5f78591b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 14 May 2020 17:41:37 +0300 Subject: [PATCH 06/10] MDEV-22070 MSAN use-of-uninitialized-value in encryption.innodb-redo-badkey On a checksum failure of a ROW_FORMAT=COMPRESSED page, buf_LRU_free_one_page() would invoke buf_LRU_block_remove_hashed() which will read the uncompressed page frame, although it would not be initialized. With bad enough luck, fil_page_get_type(page) could return an unrecognized value and cause the server to abort. buf_page_io_complete(): On the corruption of a ROW_FORMAT=COMPRESSED page, zerofill the uncompressed page frame. --- storage/innobase/buf/buf0buf.cc | 10 +++++++--- storage/xtradb/buf/buf0buf.cc | 10 +++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 136d46b7027..0fa569e0254 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -2,7 +2,7 @@ Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. -Copyright (c) 2013, 2019, MariaDB Corporation. +Copyright (c) 2013, 2020, MariaDB Corporation. Portions of this file contain modifications contributed and copyrighted by Google, Inc. Those modifications are gratefully acknowledged and are described @@ -4931,9 +4931,8 @@ buf_page_io_complete(buf_page_t* bpage, bool evict) err = buf_page_check_corrupt(bpage, space); -database_corrupted: - if (err != DB_SUCCESS) { +database_corrupted: /* Not a real corruption if it was triggered by error injection */ DBUG_EXECUTE_IF("buf_page_import_corrupt_failure", @@ -4948,6 +4947,11 @@ database_corrupted: goto page_not_corrupt; ); + if (uncompressed && bpage->zip.data) { + memset(reinterpret_cast(bpage) + ->frame, 0, srv_page_size); + } + if (err == DB_PAGE_CORRUPTED) { ib_logf(IB_LOG_LEVEL_ERROR, "Database page corruption on disk" diff --git a/storage/xtradb/buf/buf0buf.cc b/storage/xtradb/buf/buf0buf.cc index 398e1e84994..5e92c101110 100644 --- a/storage/xtradb/buf/buf0buf.cc +++ b/storage/xtradb/buf/buf0buf.cc @@ -2,7 +2,7 @@ Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. -Copyright (c) 2013, 2019, MariaDB Corporation. +Copyright (c) 2013, 2020, MariaDB Corporation. Portions of this file contain modifications contributed and copyrighted by Google, Inc. Those modifications are gratefully acknowledged and are described @@ -4936,9 +4936,8 @@ buf_page_io_complete(buf_page_t* bpage) err = buf_page_check_corrupt(bpage, space); } -database_corrupted: - if (err != DB_SUCCESS) { +database_corrupted: /* Not a real corruption if it was triggered by error injection */ DBUG_EXECUTE_IF("buf_page_import_corrupt_failure", @@ -4953,6 +4952,11 @@ database_corrupted: goto page_not_corrupt; ); + if (uncompressed && bpage->zip.data) { + memset(reinterpret_cast(bpage) + ->frame, 0, srv_page_size); + } + if (err == DB_PAGE_CORRUPTED) { ib_logf(IB_LOG_LEVEL_ERROR, "Database page corruption on disk" From 277aa85c9b42e2a7b778d196b307e45711ccc895 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 15 May 2020 10:44:05 +0300 Subject: [PATCH 07/10] Fixed bugs found by valgrind Other things: - Removed innodb_encryption_tables.test from valgrind as it takes a REALLY long time --- mysql-test/suite/encryption/t/innodb_encryption_tables.test | 2 ++ sql/field.cc | 2 +- sql/log_event.cc | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/encryption/t/innodb_encryption_tables.test b/mysql-test/suite/encryption/t/innodb_encryption_tables.test index bc762faf12e..d03bc890ba4 100644 --- a/mysql-test/suite/encryption/t/innodb_encryption_tables.test +++ b/mysql-test/suite/encryption/t/innodb_encryption_tables.test @@ -1,6 +1,8 @@ -- source include/have_innodb.inc -- source include/have_example_key_management_plugin.inc -- source include/not_embedded.inc +# We can't run this test under valgrind as it 'takes forever' +-- source include/not_valgrind.inc create table innodb_normal(c1 bigint not null, b char(200)) engine=innodb; create table innodb_compact(c1 bigint not null, b char(200)) engine=innodb row_format=compact; diff --git a/sql/field.cc b/sql/field.cc index f96755f5f51..0e8dd26445f 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1438,7 +1438,7 @@ void Field::error_generated_column_function_is_not_allowed(THD *thd, QT_ITEM_IDENT_SKIP_TABLE_NAMES)); my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(error ? 0 : ME_JUST_WARNING), - tmp.c_ptr(), vcol_info->get_vcol_type_name(), + tmp.c_ptr_safe(), vcol_info->get_vcol_type_name(), const_cast(field_name)); } diff --git a/sql/log_event.cc b/sql/log_event.cc index 7341add598f..f9e4365ff94 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -10228,6 +10228,8 @@ const char *sql_ex_info::init(const char *buf, const char *buf_end, } else { + if (buf_end - buf < 7) + return 0; // Wrong data field_term_len= enclosed_len= line_term_len= line_start_len= escaped_len=1; field_term = buf++; // Use first byte in string enclosed= buf++; From af784385b4a2b286000fa2c658e34283fe7bba60 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 15 May 2020 14:20:43 +0300 Subject: [PATCH 08/10] Fix for using uninitialized memory MDEV-22073 MSAN use-of-uninitialized-value in collect_statistics_for_table() Other things: innodb.analyze_table was changed to mainly test statistic collection. Was discussed with Marko. --- include/my_valgrind.h | 26 +++++++++++---- .../suite/innodb/r/analyze_table.result | 19 +++-------- mysql-test/suite/innodb/t/analyze_table.test | 33 ++++--------------- sql/field.cc | 6 ++++ sql/field.h | 9 +++++ sql/sql_statistics.cc | 1 + 6 files changed, 48 insertions(+), 46 deletions(-) diff --git a/include/my_valgrind.h b/include/my_valgrind.h index 04116e556ce..3cd4210dc3b 100644 --- a/include/my_valgrind.h +++ b/include/my_valgrind.h @@ -33,6 +33,7 @@ #if defined(HAVE_VALGRIND_MEMCHECK_H) && defined(HAVE_valgrind) # include # define MEM_UNDEFINED(a,len) VALGRIND_MAKE_MEM_UNDEFINED(a,len) +# define MEM_MAKE_DEFINED(a,len) VALGRIND_MAKE_MEM_DEFINED(a,len) # define MEM_NOACCESS(a,len) VALGRIND_MAKE_MEM_NOACCESS(a,len) # define MEM_CHECK_ADDRESSABLE(a,len) VALGRIND_CHECK_MEM_IS_ADDRESSABLE(a,len) # define MEM_CHECK_DEFINED(a,len) VALGRIND_CHECK_MEM_IS_DEFINED(a,len) @@ -42,12 +43,22 @@ /* How to do manual poisoning: https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ # define MEM_UNDEFINED(a,len) ASAN_UNPOISON_MEMORY_REGION(a,len) +# define MEM_MAKE_DEFINED(a,len) ((void) 0) # define MEM_NOACCESS(a,len) ASAN_POISON_MEMORY_REGION(a,len) # define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) # define MEM_CHECK_DEFINED(a,len) ((void) 0) # define REDZONE_SIZE 8 +#elif __has_feature(memory_sanitizer) +# include +# define MEM_UNDEFINED(a,len) __msan_allocated_memory(a,len) +# define MEM_MAKE_DEFINED(a,len) __msan_unpoison(a,len) +# define MEM_NOACCESS(a,len) ((void) 0) +# define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) +# define MEM_CHECK_DEFINED(a,len) __msan_check_mem_is_initialized(a,len) +# define REDZONE_SIZE 8 #else # define MEM_UNDEFINED(a,len) ((void) (a), (void) (len)) +# define MEM_MAKE_DEFINED(a,len) ((void) 0) # define MEM_NOACCESS(a,len) ((void) 0) # define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) # define MEM_CHECK_DEFINED(a,len) ((void) 0) @@ -55,12 +66,15 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ #endif /* HAVE_VALGRIND_MEMCHECK_H */ #ifndef DBUG_OFF -/* NOTE: Do not invoke TRASH_FILL directly! Use TRASH_ALLOC or TRASH_FREE. - -The MEM_UNDEFINED() call before memset() is for canceling the effect -of any previous MEM_NOACCESS(). We must invoke MEM_UNDEFINED() after -writing the dummy pattern, unless MEM_NOACCESS() is going to be invoked. -On AddressSanitizer, the MEM_UNDEFINED() in TRASH_ALLOC() has no effect. */ +/* + TRASH_FILL() has to call MEM_UNDEFINED() to cancel any effect of TRASH_FREE(). + This can happen in the case one does + TRASH_ALLOC(A,B) ; TRASH_FREE(A,B) ; TRASH_ALLOC(A,B) + to reuse the same memory in an internal memory allocator like MEM_ROOT. + For my_malloc() and safemalloc() the extra MEM_UNDEFINED is bit of an + overkill. + TRASH_FILL() is an internal function and should not be used externally. +*/ #define TRASH_FILL(A,B,C) do { const size_t trash_tmp= (B); MEM_UNDEFINED(A, trash_tmp); memset(A, C, trash_tmp); } while (0) #else #define TRASH_FILL(A,B,C) do { MEM_UNDEFINED((A), (B)); } while (0) diff --git a/mysql-test/suite/innodb/r/analyze_table.result b/mysql-test/suite/innodb/r/analyze_table.result index a5c25289ad1..830130821da 100644 --- a/mysql-test/suite/innodb/r/analyze_table.result +++ b/mysql-test/suite/innodb/r/analyze_table.result @@ -1,25 +1,16 @@ -CREATE PROCEDURE populate_t1() -BEGIN -DECLARE i int DEFAULT 1; -START TRANSACTION; -WHILE (i <= 1000000) DO -INSERT INTO t1 VALUES (i, i, CONCAT('a', i)); -SET i = i + 1; -END WHILE; -COMMIT; -END| +set use_stat_tables='preferably'; CREATE TABLE t1( class INT, id INT, title VARCHAR(100) ) ENGINE=InnoDB; +insert into t1 select seq, seq, concat('a', seq) from seq_1_to_500; SELECT COUNT(*) FROM t1; COUNT(*) -1000000 -SET GLOBAL innodb_stats_persistent_sample_pages=2000; +500 +set @@max_heap_table_size=16384; ANALYZE TABLE t1; Table Op Msg_type Msg_text +test.t1 analyze status Engine-independent statistics collected test.t1 analyze status OK DROP TABLE t1; -DROP PROCEDURE populate_t1; -SET GLOBAL innodb_stats_persistent_sample_pages=default; diff --git a/mysql-test/suite/innodb/t/analyze_table.test b/mysql-test/suite/innodb/t/analyze_table.test index e9db3668f02..538eed04ba4 100644 --- a/mysql-test/suite/innodb/t/analyze_table.test +++ b/mysql-test/suite/innodb/t/analyze_table.test @@ -1,23 +1,11 @@ -# -# BUG#22385442 - INNODB: DIFFICULT TO FIND FREE BLOCKS IN THE BUFFER POOL -# - --source include/have_innodb.inc ---source include/big_test.inc +--source include/have_sequence.inc -DELIMITER |; -CREATE PROCEDURE populate_t1() -BEGIN - DECLARE i int DEFAULT 1; +# +# MDEV-22073 MSAN use-of-uninitialized-value in collect_statistics_for_table() +# - START TRANSACTION; - WHILE (i <= 1000000) DO - INSERT INTO t1 VALUES (i, i, CONCAT('a', i)); - SET i = i + 1; - END WHILE; - COMMIT; -END| -DELIMITER ;| +set use_stat_tables='preferably'; CREATE TABLE t1( class INT, @@ -25,18 +13,11 @@ CREATE TABLE t1( title VARCHAR(100) ) ENGINE=InnoDB; --- disable_query_log -CALL populate_t1(); --- enable_query_log +insert into t1 select seq, seq, concat('a', seq) from seq_1_to_500; SELECT COUNT(*) FROM t1; -SET GLOBAL innodb_stats_persistent_sample_pages=2000; - +set @@max_heap_table_size=16384; ANALYZE TABLE t1; DROP TABLE t1; - -DROP PROCEDURE populate_t1; - -SET GLOBAL innodb_stats_persistent_sample_pages=default; diff --git a/sql/field.cc b/sql/field.cc index 0e8dd26445f..9ab76aa37a4 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -7772,6 +7772,12 @@ my_decimal *Field_varstring::val_decimal(my_decimal *decimal_value) } +void Field_varstring::mark_unused_memory_as_defined() +{ + uint used_length __attribute__((unused)) = get_length(); + MEM_MAKE_DEFINED(get_data() + used_length, field_length - used_length); +} + int Field_varstring::cmp_max(const uchar *a_ptr, const uchar *b_ptr, uint max_len) diff --git a/sql/field.h b/sql/field.h index fa27b0d10a6..ea57a4f07cc 100644 --- a/sql/field.h +++ b/sql/field.h @@ -850,6 +850,14 @@ public: enum_check_fields check_level); int store(const LEX_STRING *ls, CHARSET_INFO *cs) { return store(ls->str, (uint32) ls->length, cs); } + /* + Mark unused memory in the field as defined. Mainly used to ensure + that if we write full field to disk (for example in + Count_distinct_field::add(), we don't write unitalized data to + disk which would confuse valgrind or MSAN. + */ + virtual void mark_unused_memory_as_defined() {} + virtual double val_real(void)=0; virtual longlong val_int(void)=0; /* @@ -3242,6 +3250,7 @@ public: int store(const char *to,uint length,CHARSET_INFO *charset); int store(longlong nr, bool unsigned_val); int store(double nr) { return Field_str::store(nr); } /* QQ: To be deleted */ + void mark_unused_memory_as_defined(); double val_real(void); longlong val_int(void); String *val_str(String*,String *); diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index f8723a4f8ee..0f7a1d42fd6 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -1671,6 +1671,7 @@ public: */ virtual bool add() { + table_field->mark_unused_memory_as_defined(); return tree->unique_add(table_field->ptr); } From 1cac6d48282459d8855c1cec1eeb4bc0e3db5f89 Mon Sep 17 00:00:00 2001 From: Eugene Kosov Date: Thu, 14 May 2020 15:24:47 +0300 Subject: [PATCH 09/10] span cleanup --- include/span.h | 62 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/include/span.h b/include/span.h index 0ed0158088c..0e8516933c6 100644 --- a/include/span.h +++ b/include/span.h @@ -24,11 +24,33 @@ this program; if not, write to the Free Software Foundation, Inc., namespace st_ { +namespace detail +{ + +template struct remove_cv +{ + typedef T type; +}; +template struct remove_cv +{ + typedef T type; +}; +template struct remove_cv +{ + typedef T type; +}; +template struct remove_cv +{ + typedef T type; +}; + +} // namespace detail + template class span { public: typedef ElementType element_type; - typedef ElementType value_type; + typedef typename detail::remove_cv::type value_type; typedef size_t size_type; typedef ptrdiff_t difference_type; typedef element_type *pointer; @@ -38,7 +60,6 @@ public: typedef pointer iterator; typedef const_pointer const_iterator; typedef std::reverse_iterator reverse_iterator; - typedef std::reverse_iterator const_reverse_iterator; span() : data_(NULL), size_(0) {} @@ -64,73 +85,72 @@ public: span &operator=(const span &other) { - data_= other.data_; - size_= other.size_; + data_= other.data(); + size_= other.size(); return *this; } template span first() const { assert(!empty()); - return span(data_, 1); + return span(data(), 1); } template span last() const { assert(!empty()); - return span(data_ + size() - 1, 1); + return span(data() + size() - 1, 1); } span first(size_type count) const { assert(!empty()); - return span(data_, 1); + return span(data(), 1); } span last(size_type count) const { assert(!empty()); - return span(data_ + size() - 1, 1); + return span(data() + size() - 1, 1); } span subspan(size_type offset, size_type count) const { assert(!empty()); assert(size() >= offset + count); - return span(data_ + offset, count); + return span(data() + offset, count); } size_type size() const { return size_; } - size_type size_bytes() const { return size_ * sizeof(ElementType); } - bool empty() const __attribute__((warn_unused_result)) { return size_ == 0; } + size_type size_bytes() const { return size() * sizeof(ElementType); } + bool empty() const __attribute__((warn_unused_result)) + { + return size() == 0; + } reference operator[](size_type idx) const { assert(size() > idx); - return data_[idx]; + return data()[idx]; } reference front() const { assert(!empty()); - return data_[0]; + return data()[0]; } reference back() const { assert(!empty()); - return data_[size() - 1]; - } - pointer data() const - { - assert(!empty()); - return data_; + return data()[size() - 1]; } + pointer data() const { return data_; } iterator begin() const { return data_; } iterator end() const { return data_ + size_; } reverse_iterator rbegin() const { - return std::reverse_iterator(std::advance(end(), -1)); + return std::reverse_iterator(end()); } reverse_iterator rend() const { - return std::reverse_iterator(std::advance(begin(), -1)); + return std::reverse_iterator(begin()); } private: From ff66d65a096ec02dda1ab449d84a40361551085c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 15 May 2020 17:09:20 +0300 Subject: [PATCH 10/10] Amend af784385b4a2b286000fa2c658e34283fe7bba60: Avoid vtable overhead When neither MSAN nor Valgrind are enabled, declare Field::mark_unused_memory_as_defined() as an empty inline function, instead of declaring it as a virtual function. --- include/my_valgrind.h | 4 +++- sql/field.cc | 7 +++++-- sql/field.h | 10 ++++++++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/my_valgrind.h b/include/my_valgrind.h index 3cd4210dc3b..dc0c9bcfad4 100644 --- a/include/my_valgrind.h +++ b/include/my_valgrind.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2010, 2019, MariaDB Corporation. +/* Copyright (C) 2010, 2020, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -32,6 +32,7 @@ #if defined(HAVE_VALGRIND_MEMCHECK_H) && defined(HAVE_valgrind) # include +# define HAVE_valgrind_or_MSAN # define MEM_UNDEFINED(a,len) VALGRIND_MAKE_MEM_UNDEFINED(a,len) # define MEM_MAKE_DEFINED(a,len) VALGRIND_MAKE_MEM_DEFINED(a,len) # define MEM_NOACCESS(a,len) VALGRIND_MAKE_MEM_NOACCESS(a,len) @@ -50,6 +51,7 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ # define REDZONE_SIZE 8 #elif __has_feature(memory_sanitizer) # include +# define HAVE_valgrind_or_MSAN # define MEM_UNDEFINED(a,len) __msan_allocated_memory(a,len) # define MEM_MAKE_DEFINED(a,len) __msan_unpoison(a,len) # define MEM_NOACCESS(a,len) ((void) 0) diff --git a/sql/field.cc b/sql/field.cc index 9ab76aa37a4..d1bd5860f06 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1,6 +1,6 @@ /* Copyright (c) 2000, 2017, Oracle and/or its affiliates. - Copyright (c) 2008, 2017, MariaDB + Copyright (c) 2008, 2020, MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -7772,11 +7772,14 @@ my_decimal *Field_varstring::val_decimal(my_decimal *decimal_value) } + +#ifdef HAVE_valgrind_or_MSAN void Field_varstring::mark_unused_memory_as_defined() { - uint used_length __attribute__((unused)) = get_length(); + uint used_length= get_length(); MEM_MAKE_DEFINED(get_data() + used_length, field_length - used_length); } +#endif int Field_varstring::cmp_max(const uchar *a_ptr, const uchar *b_ptr, diff --git a/sql/field.h b/sql/field.h index ea57a4f07cc..3007e666ba7 100644 --- a/sql/field.h +++ b/sql/field.h @@ -1,7 +1,7 @@ #ifndef FIELD_INCLUDED #define FIELD_INCLUDED /* Copyright (c) 2000, 2015, Oracle and/or its affiliates. - Copyright (c) 2008, 2017, MariaDB Corporation. + Copyright (c) 2008, 2020, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -850,13 +850,17 @@ public: enum_check_fields check_level); int store(const LEX_STRING *ls, CHARSET_INFO *cs) { return store(ls->str, (uint32) ls->length, cs); } - /* +#ifdef HAVE_valgrind_or_MSAN + /** Mark unused memory in the field as defined. Mainly used to ensure that if we write full field to disk (for example in Count_distinct_field::add(), we don't write unitalized data to disk which would confuse valgrind or MSAN. */ virtual void mark_unused_memory_as_defined() {} +#else + void mark_unused_memory_as_defined() {} +#endif virtual double val_real(void)=0; virtual longlong val_int(void)=0; @@ -3250,7 +3254,9 @@ public: int store(const char *to,uint length,CHARSET_INFO *charset); int store(longlong nr, bool unsigned_val); int store(double nr) { return Field_str::store(nr); } /* QQ: To be deleted */ +#ifdef HAVE_valgrind_or_MSAN void mark_unused_memory_as_defined(); +#endif /* HAVE_valgrind_or_MSAN */ double val_real(void); longlong val_int(void); String *val_str(String*,String *);