From 57c526ffb852fb027e25fdc77173d45bdc60b8a2 Mon Sep 17 00:00:00 2001 From: Monty Date: Sun, 26 Feb 2023 18:33:10 +0200 Subject: [PATCH 1/6] Added detection of memory overwrite with multi_malloc This patch also fixes some bugs detected by valgrind after this patch: - Not enough copy_func elements was allocated by Create_tmp_table() which causes an memory overwrite in Create_tmp_table::add_fields() I added an ASSERT() to be able to detect this also without valgrind. The bug was that TMP_TABLE_PARAM::copy_fields was not correctly set when calling create_tmp_table(). - Aria::empty_bits is not allocated if there is no varchar/char/blob fields in the table. Fixed code to take this into account. This cannot cause any issues as this is just a memory access into other Aria memory and the content of the memory would not be used. - Aria::last_key_buff was not allocated big enough. This may have caused issues with rtrees and ma_extra(HA_EXTRA_REMEMBER_POS) as they would use the same memory area. - Aria and MyISAM didn't take extended key parts into account, which caused problems when copying rec_per_key from engine to sql level. - Mark asan builds with 'asan' in version strihng to detect these in not_valgrind_build.inc. This is needed to not have main.sp-no-valgrind fail with asan. --- BUILD/compile-pentium64-asan-max | 2 +- mysql-test/include/not_valgrind_build.inc | 4 +-- .../t/innodb_log_checkpoint_now_basic.test | 2 ++ mysys/mulalloc.c | 19 +++++++++++ mysys/my_alloc.c | 12 +++++++ sql/opt_subselect.cc | 5 +-- sql/select_handler.cc | 15 ++++---- sql/sql_class.cc | 1 + sql/sql_class.h | 1 + sql/sql_expression_cache.cc | 2 +- sql/sql_select.cc | 34 ++++++++++++------- sql/sql_show.cc | 28 +++++++++------ sql/sql_union.cc | 4 ++- sql/temporary_tables.cc | 6 ++-- storage/maria/ha_maria.cc | 18 +++++++--- storage/maria/ma_blockrec.c | 25 +++++++++----- storage/maria/ma_open.c | 2 +- storage/myisam/ha_myisam.cc | 14 ++++++-- 18 files changed, 136 insertions(+), 58 deletions(-) diff --git a/BUILD/compile-pentium64-asan-max b/BUILD/compile-pentium64-asan-max index 37acc9f74f320..c70c173c6801e 100755 --- a/BUILD/compile-pentium64-asan-max +++ b/BUILD/compile-pentium64-asan-max @@ -17,7 +17,7 @@ path=`dirname $0` . "$path/SETUP.sh" -extra_flags="$pentium64_cflags $debug_cflags -lasan -O -g -fsanitize=address -USAFEMALLOC -UFORCE_INIT_OF_VARS -Wno-uninitialized -Wno-maybe-uninitialized" +extra_flags="$pentium64_cflags $debug_cflags -lasan -O -g -fsanitize=address -USAFEMALLOC -UFORCE_INIT_OF_VARS -Wno-uninitialized -Wno-maybe-uninitialized -DMYSQL_SERVER_SUFFIX=-asan-max" extra_configs="$pentium_configs $debug_configs $valgrind_configs $max_configs $disable_asan_plugins" export LDFLAGS="-ldl" diff --git a/mysql-test/include/not_valgrind_build.inc b/mysql-test/include/not_valgrind_build.inc index 2b60f11bfc703..b62a1bc953b45 100644 --- a/mysql-test/include/not_valgrind_build.inc +++ b/mysql-test/include/not_valgrind_build.inc @@ -1,4 +1,4 @@ -if (`select version() like '%valgrind%'`) +if (`select version() like '%valgrind%' || version() like '%asan%'`) { - skip Does not run with binaries built with valgrind; + skip Does not run with binaries built with valgrind or asan; } diff --git a/mysql-test/suite/sys_vars/t/innodb_log_checkpoint_now_basic.test b/mysql-test/suite/sys_vars/t/innodb_log_checkpoint_now_basic.test index 331803fff86ec..f59ebf649c31c 100644 --- a/mysql-test/suite/sys_vars/t/innodb_log_checkpoint_now_basic.test +++ b/mysql-test/suite/sys_vars/t/innodb_log_checkpoint_now_basic.test @@ -1,5 +1,7 @@ --source include/have_innodb.inc --source include/have_debug.inc +# Valgrind builds may block on this one +--source include/not_valgrind.inc SET @start_global_value = @@global.innodb_log_checkpoint_now; SELECT @start_global_value; diff --git a/mysys/mulalloc.c b/mysys/mulalloc.c index 357f9315f2b59..51f8d61b574a5 100644 --- a/mysys/mulalloc.c +++ b/mysys/mulalloc.c @@ -17,6 +17,11 @@ #include "mysys_priv.h" #include +#ifndef DBUG_OFF +/* Put a protected barrier after every element when using my_multi_malloc() */ +#define ALLOC_BARRIER +#endif + /* Malloc many pointers at the same time Only ptr1 can be free'd, and doing this will free all @@ -45,6 +50,9 @@ void* my_multi_malloc(PSI_memory_key key, myf myFlags, ...) { length=va_arg(args,uint); tot_length+=ALIGN_SIZE(length); +#ifdef ALLOC_BARRIER + tot_length+= ALIGN_SIZE(1); +#endif } va_end(args); @@ -58,6 +66,10 @@ void* my_multi_malloc(PSI_memory_key key, myf myFlags, ...) *ptr=res; length=va_arg(args,uint); res+=ALIGN_SIZE(length); +#ifdef ALLOC_BARRIER + TRASH_FREE(res, ALIGN_SIZE(1)); + res+= ALIGN_SIZE(1); +#endif } va_end(args); DBUG_RETURN((void*) start); @@ -89,6 +101,9 @@ void *my_multi_malloc_large(PSI_memory_key key, myf myFlags, ...) { length=va_arg(args,ulonglong); tot_length+=ALIGN_SIZE(length); +#ifdef ALLOC_BARRIER + tot_length+= ALIGN_SIZE(1); +#endif } va_end(args); @@ -102,6 +117,10 @@ void *my_multi_malloc_large(PSI_memory_key key, myf myFlags, ...) *ptr=res; length=va_arg(args,ulonglong); res+=ALIGN_SIZE(length); +#ifdef ALLOC_BARRIER + TRASH_FREE(res, ALIGN_SIZE(1)); + res+= ALIGN_SIZE(1); +#endif } va_end(args); DBUG_RETURN((void*) start); diff --git a/mysys/my_alloc.c b/mysys/my_alloc.c index c3205eac6f00b..aa0182c755e7a 100644 --- a/mysys/my_alloc.c +++ b/mysys/my_alloc.c @@ -23,6 +23,11 @@ #undef EXTRA_DEBUG #define EXTRA_DEBUG +#ifndef DBUG_OFF +/* Put a protected barrier after every element when using multi_alloc_root() */ +#define ALLOC_BARRIER +#endif + /* data packed in MEM_ROOT -> min_malloc */ #define MALLOC_FLAG(A) ((A & 1) ? MY_THREAD_SPECIFIC : 0) @@ -311,6 +316,9 @@ void *multi_alloc_root(MEM_ROOT *root, ...) { length= va_arg(args, uint); tot_length+= ALIGN_SIZE(length); +#ifdef ALLOC_BARRIER + tot_length+= ALIGN_SIZE(1); +#endif } va_end(args); @@ -324,6 +332,10 @@ void *multi_alloc_root(MEM_ROOT *root, ...) *ptr= res; length= va_arg(args, uint); res+= ALIGN_SIZE(length); +#ifdef ALLOC_BARRIER + TRASH_FREE(res, ALIGN_SIZE(1)); + res+= ALIGN_SIZE(1); +#endif } va_end(args); DBUG_RETURN((void*) start); diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc index d516dc02b9075..3cc4d621f0724 100644 --- a/sql/opt_subselect.cc +++ b/sql/opt_subselect.cc @@ -4174,6 +4174,7 @@ bool setup_sj_materialization_part1(JOIN_TAB *sjm_tab) } sjm->sjm_table_param.field_count= subq_select->item_list.elements; + sjm->sjm_table_param.func_count= sjm->sjm_table_param.field_count; sjm->sjm_table_param.force_not_null_cols= TRUE; if (!(sjm->table= create_tmp_table(thd, &sjm->sjm_table_param, @@ -5789,14 +5790,14 @@ TABLE *create_dummy_tmp_table(THD *thd) DBUG_ENTER("create_dummy_tmp_table"); TABLE *table; TMP_TABLE_PARAM sjm_table_param; - sjm_table_param.init(); - sjm_table_param.field_count= 1; List sjm_table_cols; const LEX_CSTRING dummy_name= { STRING_WITH_LEN("dummy") }; Item *column_item= new (thd->mem_root) Item_int(thd, 1); if (!column_item) DBUG_RETURN(NULL); + sjm_table_param.init(); + sjm_table_param.field_count= sjm_table_param.func_count= 1; sjm_table_cols.push_back(column_item, thd->mem_root); if (!(table= create_tmp_table(thd, &sjm_table_param, sjm_table_cols, (ORDER*) 0, diff --git a/sql/select_handler.cc b/sql/select_handler.cc index 795ed8eb641a6..b0b8e58623de7 100644 --- a/sql/select_handler.cc +++ b/sql/select_handler.cc @@ -51,18 +51,19 @@ select_handler::~select_handler() TABLE *select_handler::create_tmp_table(THD *thd, SELECT_LEX *select) { - DBUG_ENTER("select_handler::create_tmp_table"); List types; TMP_TABLE_PARAM tmp_table_param; + TABLE *table; + DBUG_ENTER("select_handler::create_tmp_table"); + if (select->master_unit()->join_union_item_types(thd, types, 1)) DBUG_RETURN(NULL); tmp_table_param.init(); - tmp_table_param.field_count= types.elements; - - TABLE *table= ::create_tmp_table(thd, &tmp_table_param, types, - (ORDER *) 0, false, 0, - TMP_TABLE_ALL_COLUMNS, 1, - &empty_clex_str, true, false); + tmp_table_param.field_count= tmp_table_param.func_count= types.elements; + table= ::create_tmp_table(thd, &tmp_table_param, types, + (ORDER *) 0, false, 0, + TMP_TABLE_ALL_COLUMNS, 1, + &empty_clex_str, true, false); DBUG_RETURN(table); } diff --git a/sql/sql_class.cc b/sql/sql_class.cc index b453279550880..95a777c75cf1a 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4310,6 +4310,7 @@ create_result_table(THD *thd_arg, List *column_types, { DBUG_ASSERT(table == 0); tmp_table_param.field_count= column_types->elements; + tmp_table_param.func_count= tmp_table_param.field_count; tmp_table_param.bit_fields_as_long= bit_fields_as_long; if (! (table= create_tmp_table(thd_arg, &tmp_table_param, *column_types, diff --git a/sql/sql_class.h b/sql/sql_class.h index 16751434718e7..8c51312ce2a34 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -5986,6 +5986,7 @@ class TMP_TABLE_PARAM :public Sql_alloc @see opt_sum_query, count_field_types */ uint sum_func_count; + uint copy_func_count; // Allocated copy fields uint hidden_field_count; uint group_parts,group_length,group_null_parts; diff --git a/sql/sql_expression_cache.cc b/sql/sql_expression_cache.cc index bc44ecd79fa0f..69d85f3343d46 100644 --- a/sql/sql_expression_cache.cc +++ b/sql/sql_expression_cache.cc @@ -114,7 +114,7 @@ void Expression_cache_tmptable::init() cache_table_param.init(); /* dependent items and result */ - cache_table_param.field_count= items.elements; + cache_table_param.field_count= cache_table_param.func_count= items.elements; /* postpone table creation to index description */ cache_table_param.skip_create_table= 1; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index c28f104804f81..0cb6050759dd5 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -3325,7 +3325,9 @@ bool JOIN::make_aggr_tables_info() if (gbh) { - if (!(pushdown_query= new (thd->mem_root) Pushdown_query(select_lex, gbh))) + TABLE *table; + if (!(pushdown_query= new (thd->mem_root) Pushdown_query(select_lex, + gbh))) DBUG_RETURN(1); /* We must store rows in the tmp table if we need to do an ORDER BY @@ -3345,13 +3347,13 @@ bool JOIN::make_aggr_tables_info() if (!(curr_tab->tmp_table_param= new TMP_TABLE_PARAM(tmp_table_param))) DBUG_RETURN(1); - TABLE* table= create_tmp_table(thd, curr_tab->tmp_table_param, - all_fields, - NULL, query.distinct, - TRUE, select_options, HA_POS_ERROR, - &empty_clex_str, !need_tmp, - query.order_by || query.group_by); - if (!table) + curr_tab->tmp_table_param->func_count= all_fields.elements; + if (!(table= create_tmp_table(thd, curr_tab->tmp_table_param, + all_fields, + NULL, query.distinct, + TRUE, select_options, HA_POS_ERROR, + &empty_clex_str, !need_tmp, + query.order_by || query.group_by))) DBUG_RETURN(1); if (!(curr_tab->aggr= new (thd->mem_root) AGGR_OP(curr_tab))) @@ -18805,6 +18807,7 @@ TABLE *Create_tmp_table::start(THD *thd, */ if (param->precomputed_group_by) copy_func_count+= param->sum_func_count; + param->copy_func_count= copy_func_count; init_sql_alloc(key_memory_TABLE, &own_root, TABLE_ALLOC_BLOCK_SIZE, 0, MYF(MY_THREAD_SPECIFIC)); @@ -19010,8 +19013,9 @@ bool Create_tmp_table::add_fields(THD *thd, We here distinguish between UNION and multi-table-updates by the fact that in the later case group is set to the row pointer. - The test for item->marker == 4 is ensure we don't create a group-by - key over a bit field as heap tables can't handle that. + The test for item->marker == MARKER_NULL_KEY is ensure we + don't create a group-by key over a bit field as heap tables + can't handle that. */ DBUG_ASSERT(!param->schema_table); Field *new_field= @@ -19076,6 +19080,7 @@ bool Create_tmp_table::add_fields(THD *thd, new_field->flags|= FIELD_PART_OF_TMP_UNIQUE; } } + DBUG_ASSERT(fieldnr == m_field_count[other] + m_field_count[distinct]); DBUG_ASSERT(m_blob_count == m_blobs_count[other] + m_blobs_count[distinct]); share->fields= fieldnr; @@ -19083,7 +19088,9 @@ bool Create_tmp_table::add_fields(THD *thd, table->field[fieldnr]= 0; // End marker share->blob_field[m_blob_count]= 0; // End marker copy_func[0]= 0; // End marker + DBUG_ASSERT((copy_func - param->items_to_copy) <= param->copy_func_count); param->func_count= (uint) (copy_func - param->items_to_copy); + share->column_bitmap_size= bitmap_buffer_size(share->fields); thd->mem_root= mem_root_save; @@ -26461,6 +26468,11 @@ bool JOIN::rollup_init() Item **ref_array; tmp_table_param.quick_group= 0; // Can't create groups in tmp table + /* + Each group can potentially be replaced with Item_func_rollup_const() which + needs a copy_func placeholder. + */ + tmp_table_param.func_count+= send_group_parts; rollup.state= ROLLUP::STATE_INITED; /* @@ -26485,7 +26497,6 @@ bool JOIN::rollup_init() ref_array= (Item**) (rollup.ref_pointer_arrays+send_group_parts); - /* Prepare space for field list for the different levels These will be filled up in rollup_make_fields() @@ -26650,7 +26661,6 @@ bool JOIN::rollup_make_fields(List &fields_arg, List &sel_fields, /* Point to first hidden field */ uint ref_array_ix= fields_arg.elements-1; - /* Remember where the sum functions ends for the previous level */ sum_funcs_end[pos+1]= *func; diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 1c291d9fcb363..6405a919698ba 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -8216,28 +8216,34 @@ TABLE *create_schema_table(THD *thd, TABLE_LIST *table_list) TABLE *table; ST_SCHEMA_TABLE *schema_table= table_list->schema_table; ST_FIELD_INFO *fields= schema_table->fields_info; - bool need_all_fieds= table_list->schema_table_reformed || // SHOW command + bool need_all_fields= table_list->schema_table_reformed || // SHOW command thd->lex->only_view_structure(); // need table structure + bool keep_row_order; + TMP_TABLE_PARAM *tmp_table_param; + SELECT_LEX *select_lex; + my_bitmap_map *bitmaps; DBUG_ENTER("create_schema_table"); for (; !fields->end_marker(); fields++) field_count++; - TMP_TABLE_PARAM *tmp_table_param = new (thd->mem_root) TMP_TABLE_PARAM; + tmp_table_param = new (thd->mem_root) TMP_TABLE_PARAM; tmp_table_param->init(); tmp_table_param->table_charset= system_charset_info; tmp_table_param->field_count= field_count; tmp_table_param->schema_table= 1; - SELECT_LEX *select_lex= table_list->select_lex; - bool keep_row_order= is_show_command(thd); - if (!(table= create_tmp_table_for_schema(thd, tmp_table_param, *schema_table, - (select_lex->options | thd->variables.option_bits | TMP_TABLE_ALL_COLUMNS), - table_list->alias, !need_all_fieds, keep_row_order))) + select_lex= table_list->select_lex; + keep_row_order= is_show_command(thd); + if (!(table= + create_tmp_table_for_schema(thd, tmp_table_param, *schema_table, + (select_lex->options | + thd->variables.option_bits | + TMP_TABLE_ALL_COLUMNS), + table_list->alias, !need_all_fields, + keep_row_order))) DBUG_RETURN(0); - my_bitmap_map* bitmaps= - (my_bitmap_map*) thd->alloc(bitmap_buffer_size(field_count)); - my_bitmap_init(&table->def_read_set, (my_bitmap_map*) bitmaps, field_count, - FALSE); + bitmaps= (my_bitmap_map*) thd->alloc(bitmap_buffer_size(field_count)); + my_bitmap_init(&table->def_read_set, bitmaps, field_count, FALSE); table->read_set= &table->def_read_set; bitmap_clear_all(table->read_set); table_list->schema_table_param= tmp_table_param; diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 150900a4c77f5..f996b4b093aa7 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -344,6 +344,7 @@ select_unit::create_result_table(THD *thd_arg, List *column_types, DBUG_ASSERT(table == 0); tmp_table_param.init(); tmp_table_param.field_count= column_types->elements; + tmp_table_param.func_count= tmp_table_param.field_count; tmp_table_param.bit_fields_as_long= bit_fields_as_long; tmp_table_param.hidden_field_count= hidden; @@ -384,7 +385,8 @@ select_union_recursive::create_result_table(THD *thd_arg, return true; incr_table_param.init(); - incr_table_param.field_count= column_types->elements; + incr_table_param.field_count= incr_table_param.func_count= + column_types->elements; incr_table_param.bit_fields_as_long= bit_fields_as_long; if (! (incr_table= create_tmp_table(thd_arg, &incr_table_param, *column_types, (ORDER*) 0, false, 1, diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc index 65aa6c3c28b42..8236d157b7330 100644 --- a/sql/temporary_tables.cc +++ b/sql/temporary_tables.cc @@ -912,9 +912,8 @@ bool THD::has_temporary_tables() uint THD::create_tmp_table_def_key(char *key, const char *db, const char *table_name) { - DBUG_ENTER("THD::create_tmp_table_def_key"); - uint key_length; + DBUG_ENTER("THD::create_tmp_table_def_key"); key_length= tdc_create_key(key, db, table_name); int4store(key + key_length, variables.server_id); @@ -1163,11 +1162,10 @@ TABLE *THD::open_temporary_table(TMP_TABLE_SHARE *share, */ bool THD::find_and_use_tmp_table(const TABLE_LIST *tl, TABLE **out_table) { - DBUG_ENTER("THD::find_and_use_tmp_table"); - char key[MAX_DBKEY_LENGTH]; uint key_length; bool result; + DBUG_ENTER("THD::find_and_use_tmp_table"); key_length= create_tmp_table_def_key(key, tl->get_db_name(), tl->get_table_name()); diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index 000b8527c0697..8d02ae31524b0 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -2630,12 +2630,22 @@ int ha_maria::info(uint flag) share->db_record_offset= maria_info.record_offset; if (share->key_parts) { - ulong *to= table->key_info[0].rec_per_key, *end; double *from= maria_info.rec_per_key; - for (end= to+ share->key_parts ; to < end ; to++, from++) - *to= (ulong) (*from + 0.5); + KEY *key, *key_end; + for (key= table->key_info, key_end= key + share->keys; + key < key_end ; key++) + { + ulong *to= key->rec_per_key; + /* Some temporary tables does not allocate rec_per_key */ + if (to) + { + for (ulong *end= to+ key->user_defined_key_parts ; + to < end ; + to++, from++) + *to= (ulong) (*from + 0.5); + } + } } - /* Set data_file_name and index_file_name to point at the symlink value if table is symlinked (Ie; Real name is not same as generated name) diff --git a/storage/maria/ma_blockrec.c b/storage/maria/ma_blockrec.c index e628c5ba5f3b1..eec4628497b6d 100644 --- a/storage/maria/ma_blockrec.c +++ b/storage/maria/ma_blockrec.c @@ -2774,7 +2774,8 @@ static my_bool write_block_record(MARIA_HA *info, const uchar *field_pos; ulong length; if ((record[column->null_pos] & column->null_bit) || - (row->empty_bits[column->empty_pos] & column->empty_bit)) + (column->empty_bit && + (row->empty_bits[column->empty_pos] & column->empty_bit))) continue; field_pos= record + column->offset; @@ -4872,7 +4873,8 @@ int _ma_read_block_record2(MARIA_HA *info, uchar *record, uchar *field_pos= record + column->offset; /* First check if field is present in record */ if ((record[column->null_pos] & column->null_bit) || - (cur_row->empty_bits[column->empty_pos] & column->empty_bit)) + (column->empty_bit && + (cur_row->empty_bits[column->empty_pos] & column->empty_bit))) { bfill(record + column->offset, column->fill_length, type == FIELD_SKIP_ENDSPACE ? ' ' : 0); @@ -4951,8 +4953,9 @@ int _ma_read_block_record2(MARIA_HA *info, uchar *record, { uint size_length; if ((record[blob_field->null_pos] & blob_field->null_bit) || - (cur_row->empty_bits[blob_field->empty_pos] & - blob_field->empty_bit)) + (blob_field->empty_bit & + (cur_row->empty_bits[blob_field->empty_pos] & + blob_field->empty_bit))) continue; size_length= blob_field->length - portable_sizeof_char_ptr; blob_lengths+= _ma_calc_blob_length(size_length, length_data); @@ -5806,7 +5809,8 @@ static size_t fill_insert_undo_parts(MARIA_HA *info, const uchar *record, const uchar *column_pos; size_t column_length; if ((record[column->null_pos] & column->null_bit) || - cur_row->empty_bits[column->empty_pos] & column->empty_bit) + (column->empty_bit && + cur_row->empty_bits[column->empty_pos] & column->empty_bit)) continue; column_pos= record+ column->offset; @@ -5987,7 +5991,8 @@ static size_t fill_update_undo_parts(MARIA_HA *info, const uchar *oldrec, */ continue; } - if (old_row->empty_bits[column->empty_pos] & column->empty_bit) + if (column->empty_bit && + (old_row->empty_bits[column->empty_pos] & column->empty_bit)) { if (new_row->empty_bits[column->empty_pos] & column->empty_bit) continue; /* Both are empty; skip */ @@ -6003,8 +6008,9 @@ static size_t fill_update_undo_parts(MARIA_HA *info, const uchar *oldrec, log the original value */ new_column_is_empty= ((newrec[column->null_pos] & column->null_bit) || - (new_row->empty_bits[column->empty_pos] & - column->empty_bit)); + (column->empty_bit && + (new_row->empty_bits[column->empty_pos] & + column->empty_bit))); old_column_pos= oldrec + column->offset; new_column_pos= newrec + column->offset; @@ -7177,7 +7183,8 @@ my_bool _ma_apply_undo_row_delete(MARIA_HA *info, LSN undo_lsn, column++, null_field_lengths++) { if ((record[column->null_pos] & column->null_bit) || - row.empty_bits[column->empty_pos] & column->empty_bit) + (column->empty_bit && + row.empty_bits[column->empty_pos] & column->empty_bit)) { if (column->type != FIELD_BLOB) *null_field_lengths= 0; diff --git a/storage/maria/ma_open.c b/storage/maria/ma_open.c index e3385a73f84c4..2f5cb525950e2 100644 --- a/storage/maria/ma_open.c +++ b/storage/maria/ma_open.c @@ -121,7 +121,7 @@ static MARIA_HA *maria_clone_internal(MARIA_SHARE *share, &info.blobs,sizeof(MARIA_BLOB)*share->base.blobs, &info.buff,(share->base.max_key_block_length*2+ share->base.max_key_length), - &info.lastkey_buff,share->base.max_key_length*2+1, + &info.lastkey_buff,share->base.max_key_length*3, &info.first_mbr_key, share->base.max_key_length, &info.maria_rtree_recursion_state, share->have_rtree ? 1024 : 0, diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index 78ef653e9abb4..1b16943a97c45 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -2143,9 +2143,17 @@ int ha_myisam::info(uint flag) share->keys_for_keyread.intersect(share->keys_in_use); share->db_record_offset= misam_info.record_offset; if (share->key_parts) - memcpy((char*) table->key_info[0].rec_per_key, - (char*) misam_info.rec_per_key, - sizeof(table->key_info[0].rec_per_key[0])*share->key_parts); + { + ulong *from= misam_info.rec_per_key; + KEY *key, *key_end; + for (key= table->key_info, key_end= key + share->keys; + key < key_end ; key++) + { + memcpy(key->rec_per_key, from, + key->user_defined_key_parts * sizeof(*from)); + from+= key->user_defined_key_parts; + } + } if (table_share->tmp_table == NO_TMP_TABLE) mysql_mutex_unlock(&table_share->LOCK_share); } From c14a39431b211017e6809bb79c4079b38ffc3dff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 28 Feb 2023 15:39:23 +0200 Subject: [PATCH 2/6] MDEV-30753 Possible corruption due to trx_purge_free_segment() Starting with commit 0de3be8cfdfc26f5c236eaefe12d03c7b4af22c8 (MDEV-30671), the field TRX_UNDO_NEEDS_PURGE lost its previous meaning. The following scenario is possible: (1) InnoDB is killed at a point of time corresponding to the durable execution of some fseg_free_step_not_header() but not trx_purge_remove_log_hdr(). (2) After restart, the affected pages are allocated for something else. (3) Purge will attempt to access the newly reallocated pages when looking for some old undo log records. trx_purge_free_segment(): Invoke trx_purge_remove_log_hdr() as the first thing, to be safe. If the server is killed, some pages will never be freed. That is the lesser evil. Also, before each mtr.start(), invoke log_free_check() to prevent ib_logfile0 overrun. --- storage/innobase/include/log0log.inl | 1 + storage/innobase/trx/trx0purge.cc | 97 +++++++++++----------------- 2 files changed, 39 insertions(+), 59 deletions(-) diff --git a/storage/innobase/include/log0log.inl b/storage/innobase/include/log0log.inl index d503e3ffec9a2..0ff8c2523d797 100644 --- a/storage/innobase/include/log0log.inl +++ b/storage/innobase/include/log0log.inl @@ -306,6 +306,7 @@ log_free_check(void) #ifdef UNIV_DEBUG static const latch_level_t latches[] = { + SYNC_REDO_RSEG, /* trx_purge_free_segment() */ SYNC_DICT, /* dict_sys.mutex during commit_try_rebuild() */ SYNC_DICT_OPERATION, /* dict_sys.latch X-latch during diff --git a/storage/innobase/trx/trx0purge.cc b/storage/innobase/trx/trx0purge.cc index f273903ef936a..3843810848067 100644 --- a/storage/innobase/trx/trx0purge.cc +++ b/storage/innobase/trx/trx0purge.cc @@ -345,66 +345,45 @@ static void trx_purge_remove_log_hdr(buf_block_t *rseg, buf_block_t* log, static void trx_purge_free_segment(mtr_t &mtr, trx_rseg_t* rseg, fil_addr_t hdr_addr) { - mtr.commit(); - mtr.start(); - ut_ad(mutex_own(&rseg->mutex)); - - buf_block_t* rseg_hdr = trx_rsegf_get(rseg->space, rseg->page_no, &mtr); - buf_block_t* block = trx_undo_page_get( - page_id_t(rseg->space->id, hdr_addr.page), &mtr); - - /* Mark the last undo log totally purged, so that if the - system crashes, the tail of the undo log will not get accessed - again. The list of pages in the undo log tail gets - inconsistent during the freeing of the segment, and therefore - purge should not try to access them again. */ - mtr.write<2,mtr_t::MAYBE_NOP>(*block, block->frame + hdr_addr.boffset - + TRX_UNDO_NEEDS_PURGE, 0U); - - while (!fseg_free_step_not_header( - TRX_UNDO_SEG_HDR + TRX_UNDO_FSEG_HEADER - + block->frame, &mtr)) { - mtr.commit(); - mtr.start(); - - rseg_hdr = trx_rsegf_get(rseg->space, rseg->page_no, &mtr); - - block = trx_undo_page_get( - page_id_t(rseg->space->id, hdr_addr.page), &mtr); - } - - /* The page list may now be inconsistent, but the length field - stored in the list base node tells us how big it was before we - started the freeing. */ - - const uint32_t seg_size = flst_get_len( - TRX_UNDO_SEG_HDR + TRX_UNDO_PAGE_LIST + block->frame); - - /* We may free the undo log segment header page; it must be freed - within the same mtr as the undo log header is removed from the - history list: otherwise, in case of a database crash, the segment - could become inaccessible garbage in the file space. */ - - trx_purge_remove_log_hdr(rseg_hdr, block, hdr_addr.boffset, &mtr); - - do { - - /* Here we assume that a file segment with just the header - page can be freed in a few steps, so that the buffer pool - is not flooded with bufferfixed pages: see the note in - fsp0fsp.cc. */ - - } while (!fseg_free_step(TRX_UNDO_SEG_HDR + TRX_UNDO_FSEG_HEADER - + block->frame, &mtr)); - - byte* hist = TRX_RSEG + TRX_RSEG_HISTORY_SIZE + rseg_hdr->frame; - ut_ad(mach_read_from_4(hist) >= seg_size); - - mtr.write<4>(*rseg_hdr, hist, mach_read_from_4(hist) - seg_size); - - ut_ad(rseg->curr_size >= seg_size); + mtr.commit(); + log_free_check(); + mtr.start(); + ut_ad(mutex_own(&rseg->mutex)); + + buf_block_t *rseg_hdr= trx_rsegf_get(rseg->space, rseg->page_no, &mtr); + buf_block_t *block= + trx_undo_page_get(page_id_t(rseg->space->id, hdr_addr.page), &mtr); + const uint32_t seg_size= + flst_get_len(TRX_UNDO_SEG_HDR + TRX_UNDO_PAGE_LIST + block->frame); + ut_ad(rseg->curr_size >= seg_size); + rseg->curr_size-= seg_size; + + trx_purge_remove_log_hdr(rseg_hdr, block, hdr_addr.boffset, &mtr); + byte *hist= TRX_RSEG + TRX_RSEG_HISTORY_SIZE + rseg_hdr->frame; + ut_ad(mach_read_from_4(hist) >= seg_size); + mtr.write<4>(*rseg_hdr, hist, mach_read_from_4(hist) - seg_size); + + while (!fseg_free_step_not_header(TRX_UNDO_SEG_HDR + TRX_UNDO_FSEG_HEADER + + block->frame, &mtr)) + { + block->fix(); + mtr.commit(); + /* NOTE: If the server is killed after the log that was produced + up to this point was written, and before the log from the mtr.commit() + in our caller is written, then the pages belonging to the + undo log will become unaccessible garbage. + + This does not matters when using multiple innodb_undo_tablespaces; + innodb_undo_log_truncate=ON will be able to reclaim the space. */ + log_free_check(); + mtr.start(); + ut_ad(rw_lock_s_lock_nowait(block->debug_latch, __FILE__, __LINE__)); + rw_lock_x_lock(&block->lock); + mtr_memo_push(&mtr, block, MTR_MEMO_PAGE_X_FIX); + } - rseg->curr_size -= seg_size; + while (!fseg_free_step(TRX_UNDO_SEG_HDR + TRX_UNDO_FSEG_HEADER + + block->frame, &mtr)); } /** Remove unnecessary history data from a rollback segment. From 6d923362bd968569acac60b73fbcfbc338841608 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 28 Feb 2023 20:03:38 +0100 Subject: [PATCH 3/6] CONC-637 Build fails when specifying -DPLUGIN_AUTH_GSSAPI_CLIENT=OFF --- libmariadb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmariadb b/libmariadb index d204e83104222..4e2408c1cc298 160000 --- a/libmariadb +++ b/libmariadb @@ -1 +1 @@ -Subproject commit d204e83104222844251b221e9be7eb3dd9f8d63d +Subproject commit 4e2408c1cc298ada91b30683501c0c94a6621562 From 550b8d76b3be360b1c7984a2531e094cd4c37f8c Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Tue, 28 Feb 2023 20:19:06 +0530 Subject: [PATCH 4/6] MDEV-30752 Assertion `!index->is_ibuf()' failed around cmp_dtuple_rec_with_match_bytes - InnoDB shouldn't use the adaptive hash index for change buffer indexes. --- storage/innobase/btr/btr0cur.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index ec1b72244b4ed..8d19064a658df 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -1011,9 +1011,10 @@ dberr_t btr_cur_t::search_leaf(const dtuple_t *tuple, page_cur_mode_t mode, # ifdef UNIV_SEARCH_PERF_STAT info->n_searches++; # endif + bool ahi_enabled= btr_search_enabled && !index()->is_ibuf(); /* We do a dirty read of btr_search_enabled below, and btr_search_guess_on_hash() will have to check it again. */ - if (!btr_search_enabled); + if (!ahi_enabled); else if (btr_search_guess_on_hash(index(), info, tuple, mode, latch_mode, this, mtr)) { @@ -1335,7 +1336,7 @@ dberr_t btr_cur_t::search_leaf(const dtuple_t *tuple, page_cur_mode_t mode, reached_latched_leaf: #ifdef BTR_CUR_HASH_ADAPT - if (btr_search_enabled && !(tuple->info_bits & REC_INFO_MIN_REC_FLAG)) + if (ahi_enabled && !(tuple->info_bits & REC_INFO_MIN_REC_FLAG)) { if (page_cur_search_with_match_bytes(tuple, mode, &up_match, &up_bytes, From 49e2b50d594dfee462505b8fab2d8ba875dcf6e0 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Tue, 21 Feb 2023 16:25:17 +0530 Subject: [PATCH 5/6] MDEV-30341 Reset check_foreigns, check_unique_secondary variables - InnoDB fails to reset the check_foreigns and check_unique_secondary in trx_t::free(), trx_t::commit_cleanup(). This lead to bulk insert in internal innodb fts table operation. --- storage/innobase/include/trx0trx.h | 2 ++ storage/innobase/trx/trx0trx.cc | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h index ad941b8969163..21e4516f35a00 100644 --- a/storage/innobase/include/trx0trx.h +++ b/storage/innobase/include/trx0trx.h @@ -1070,6 +1070,8 @@ struct trx_t : ilist_node<> ut_ad(!dict_operation); ut_ad(!apply_online_log); ut_ad(!is_not_inheriting_locks()); + ut_ad(check_foreigns); + ut_ad(check_unique_secondary); } /** This has to be invoked on SAVEPOINT or at the end of a statement. diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 1516968f3b860..a0c781f62875d 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -385,9 +385,10 @@ void trx_t::free() dict_operation= false; trx_sys.deregister_trx(this); + check_unique_secondary= true; + check_foreigns= true; assert_freed(); trx_sys.rw_trx_hash.put_pins(this); - mysql_thd= nullptr; // FIXME: We need to avoid this heap free/alloc for each commit. @@ -1378,6 +1379,8 @@ void trx_t::commit_cleanup() state= TRX_STATE_NOT_STARTED; mod_tables.clear(); + check_foreigns= true; + check_unique_secondary= true; assert_freed(); trx_init(this); mutex.wr_unlock(); From 948fb3c27d1e0754d99a96389aaf1cf85370d839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 6 Mar 2023 12:01:15 +0200 Subject: [PATCH 6/6] Fix GCC 5.3.1 -Wsign-compare This fixes up commit 57c526ffb852fb027e25fdc77173d45bdc60b8a2 --- sql/sql_select.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 0cb6050759dd5..8441ec685dc1b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -19088,8 +19088,8 @@ bool Create_tmp_table::add_fields(THD *thd, table->field[fieldnr]= 0; // End marker share->blob_field[m_blob_count]= 0; // End marker copy_func[0]= 0; // End marker - DBUG_ASSERT((copy_func - param->items_to_copy) <= param->copy_func_count); param->func_count= (uint) (copy_func - param->items_to_copy); + DBUG_ASSERT(param->func_count <= param->copy_func_count); share->column_bitmap_size= bitmap_buffer_size(share->fields);