Details
Description
sanja asked me if MemorySanitizer (MDEV-20377) is usable on clang-17. I had previously mostly used it on clang-15, so I tried a clang-16 build of 10.6 and accidentally found a bug that does not appear to be caught by older versions of the compiler:
MSAN_OPTIONS=abort_on_error=1:MSAN_OPTIONS=poison_in_dtor=0 LD_LIBRARY_PATH=~/libmsan-16 MSAN_SYMBOLIZER_PATH=~/bin/llvm-symbolizer-msan ./mtr main.pool_of_threads
|
|
10.6 53c6c823dc7cafefffdc93c79661cfb146ff8641 |
main.pool_of_threads [ fail ]
|
Test ended at 2024-02-16 16:01:09
|
|
|
CURRENT_TEST: main.pool_of_threads
|
mysqltest: In included file "./include/common-tests.inc":
|
included from /mariadb/10.6/mysql-test/main/pool_of_threads.test at line 17:
|
At line 1254: query 'select t2.fld3 from t2 where companynr = 58 and fld3 like "%imaginable%"' failed: <Unknown> (2013): Lost connection to server during query
|
…
|
Version: '10.6.18-MariaDB-debug-log' socket: '/dev/shm/10.6msan/mysql-test/var/tmp/mysqld.1.sock' port: 16000 Source distribution
|
==981332==WARNING: MemorySanitizer: use-of-uninitialized-value
|
#0 0x55fd598bfae1 in Item_func::not_null_tables() const /mariadb/10.6/sql/item_func.cc:624:3
|
#1 0x55fd597e82d6 in Item_cond::eval_not_null_tables(void*) /mariadb/10.6/sql/item_cmpfunc.cc:5187:38
|
#2 0x55fd597e7707 in Item_cond::fix_fields(THD*, Item**) /mariadb/10.6/sql/item_cmpfunc.cc:5078:10
|
#3 0x55fd5a24ce1b in make_cond_for_table_from_pred(THD*, Item*, Item*, unsigned long long, unsigned long long, int, bool, bool, bool) /mariadb/10.6/sql/sql_select.cc:23938:16
|
#4 0x55fd5a17e543 in make_cond_for_table(THD*, Item*, unsigned long long, unsigned long long, int, bool, bool) /mariadb/10.6/sql/sql_select.cc:23866:10
|
#5 0x55fd5a17e543 in make_join_select(JOIN*, SQL_SELECT*, Item*) /mariadb/10.6/sql/sql_select.cc:12543:16
|
#6 0x55fd5a1563f0 in JOIN::optimize_stage2() /mariadb/10.6/sql/sql_select.cc:2855:7
|
#7 0x55fd5a15f3e1 in JOIN::optimize_inner() /mariadb/10.6/sql/sql_select.cc:2590:9
|
#8 0x55fd5a153b70 in JOIN::optimize() /mariadb/10.6/sql/sql_select.cc:1888:10
|
#9 0x55fd5a13c232 in mysql_select(THD*, TABLE_LIST*, List<Item>&, Item*, unsigned int, st_order*, st_order*, Item*, st_order*, unsigned long long, select_result*, st_select_lex_unit*, st_select_lex*) /mariadb/10.6/sql/sql_select.cc:5127:19
|
#10 0x55fd5a13bc9d in handle_select(THD*, LEX*, select_result*, unsigned long) /mariadb/10.6/sql/sql_select.cc:559:10
|
#11 0x55fd5a078ff3 in execute_sqlcom_select(THD*, TABLE_LIST*) /mariadb/10.6/sql/sql_parse.cc:6376:12
|
#12 0x55fd5a0636ce in mysql_execute_command(THD*, bool) /mariadb/10.6/sql/sql_parse.cc:3980:12
|
#13 0x55fd5a053c60 in mysql_parse(THD*, char*, unsigned int, Parser_state*) /mariadb/10.6/sql/sql_parse.cc:8143:18
|
#14 0x55fd5a04cc0c in dispatch_command(enum_server_command, THD*, char*, unsigned int, bool) /mariadb/10.6/sql/sql_parse.cc:1896:7
|
#15 0x55fd5a054531 in do_command(THD*, bool) /mariadb/10.6/sql/sql_parse.cc:1409:17
|
#16 0x55fd5a7f7c26 in threadpool_process_request(THD*) /mariadb/10.6/sql/threadpool_common.cc:435:13
|
#17 0x55fd5a7f7c26 in tp_callback(TP_connection*) /mariadb/10.6/sql/threadpool_common.cc:249:12
|
#18 0x55fd5a7ffd45 in worker_main(void*) /mariadb/10.6/sql/threadpool_generic.cc:1556:5
|
#19 0x7f01e9dd045b in start_thread nptl/pthread_create.c:444:8
|
#20 0x7f01e9e50bbb in clone3 misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
|
|
|
Memory was marked as uninitialized
|
#0 0x55fd595d3db1 in __msan_allocated_memory (/dev/shm/10.6msan/sql/mariadbd+0xf26db1) (BuildId: a8f73f07e16a2953)
|
#1 0x55fd5b5e2d85 in my_malloc /mariadb/10.6/mysys/my_malloc.c:114:7
|
I checked this with rr, and indeed the memory is straight from TRASH_ALLOC():
|
10.6 53c6c823dc7cafefffdc93c79661cfb146ff8641 |
#3 0x000055c7785aed86 in my_malloc (key=<optimized out>, size=<optimized out>, size@entry=576, my_flags=<optimized out>) at /mariadb/10.6/mysys/my_malloc.c:114
|
#4 0x000055c7785925e0 in alloc_root (mem_root=0x72b000052d98, length=576) at /mariadb/10.6/mysys/my_alloc.c:189
|
#5 0x000055c7771bbcb1 in Item::operator new (size=552, mem_root=0x215000080b58) at /mariadb/10.6/sql/item.h:861
|
#6 eliminate_item_equal (thd=thd@entry=0x72b00004d018, cond=cond@entry=0x0, upper_levels=0x0, item_equal=<optimized out>) at /mariadb/10.6/sql/sql_select.cc:16889
|
#7 0x000055c777145c0f in substitute_for_best_equal_field (thd=0x72b00004d018, context_tab=context_tab@entry=0x1, cond=0x711000075470, cond_equal=0x711000075548, table_join_idx=0x71e0000213f8,
|
do_substitution=true) at /mariadb/10.6/sql/sql_select.cc:17069
|
#8 0x000055c777120aa1 in JOIN::optimize_stage2 (this=0x71a000030030) at /mariadb/10.6/sql/sql_select.cc:2710
|
The memory is being used here:
|
10.6 53c6c823dc7cafefffdc93c79661cfb146ff8641 |
#0 0x000055c776594018 in __msan_warning_with_origin_noreturn ()
|
#1 0x000055c77688bae2 in Item_func::not_null_tables (this=<optimized out>) at /mariadb/10.6/sql/item_func.cc:624
|
#2 0x000055c7767b42d7 in Item_cond::eval_not_null_tables (this=<optimized out>, opt_arg=<optimized out>) at /mariadb/10.6/sql/item_cmpfunc.cc:5187
|
#3 0x000055c7767b3708 in Item_cond::fix_fields (this=0x711000075970, thd=0x72b00004d018, ref=<optimized out>) at /mariadb/10.6/sql/item_cmpfunc.cc:5078
|
#4 0x000055c777218e1c in make_cond_for_table_from_pred (thd=0x72b00004d018, root_cond=0x711000075470, cond=0x711000075470, tables=13835058055282163713, used_table=used_table@entry=1,
|
join_tab_idx_arg=join_tab_idx_arg@entry=0, exclude_expensive_cond=false, retain_ref_cond=<optimized out>, is_top_and_level=<optimized out>) at /mariadb/10.6/sql/sql_select.cc:23938
|
#5 0x000055c77714a544 in make_cond_for_table (thd=<optimized out>, cond=<optimized out>, tables=<optimized out>, used_table=<optimized out>, join_tab_idx_arg=<optimized out>, exclude_expensive_cond=false,
|
retain_ref_cond=false) at /mariadb/10.6/sql/sql_select.cc:23866
|
#6 make_join_select (join=<optimized out>, join@entry=0x71a000030030, select=<optimized out>, cond=<optimized out>) at /mariadb/10.6/sql/sql_select.cc:12543
|
#7 0x000055c7771223f1 in JOIN::optimize_stage2 (this=0x71a000030030) at /mariadb/10.6/sql/sql_select.cc:2855
|
The construction of the object happens here:
|
eliminate_item_equal() |
eq_item= new (thd->mem_root) Item_func_eq(thd, |
field_item->remove_item_direct_ref(),
|
head_item->remove_item_direct_ref());
|
The following patch, which makes use of the C++11 data member default initializer feature (to have the initialization added to every constructor), fixes the bug:
diff --git a/sql/item_func.h b/sql/item_func.h
|
index 170fc943681..f3d3684f5b1 100644
|
--- a/sql/item_func.h
|
+++ b/sql/item_func.h
|
@@ -89,7 +89,7 @@ class Item_func :public Item_func_or_sum
|
static void wrong_param_count_error(const LEX_CSTRING &schema_name,
|
const LEX_CSTRING &func_name);
|
|
- table_map not_null_tables_cache;
|
+ table_map not_null_tables_cache= 0;
|
|
enum Functype { UNKNOWN_FUNC,EQ_FUNC,EQUAL_FUNC,NE_FUNC,LT_FUNC,LE_FUNC,
|
GE_FUNC,GT_FUNC,FT_FUNC, |
I think that along with that addition, some redundant initialization should be removed.
Attachments
Issue Links
- includes
-
MDEV-33665 main.pool_of_threads fails due to (spurious) uninitialized Item_func::not_null_tables_cache
-
- Closed
-
-
-
- In Review
-
- relates to
-
-
- Open
-
Activity
I see that MYSQL_TYPE_LONG always was 32 bits. Here is the fixed test:
diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c
|
index fde6148f4e0..b1e40ea5c88 100644
|
--- a/tests/mysql_client_test.c
|
+++ b/tests/mysql_client_test.c
|
@@ -3842,7 +3842,7 @@ static void test_bind_result_ext1()
|
short i_data;
|
uchar b_data;
|
int f_data;
|
- long bData;
|
+ int bData;
|
char d_data[20];
|
double szData;
|
MYSQL_BIND my_bind[8];
|
@@ -3938,7 +3938,7 @@ static void test_bind_result_ext1()
|
fprintf(stdout, "\n data (float) : %d(%lu)", f_data, length[4]);
|
fprintf(stdout, "\n data (double) : %s(%lu)", d_data, length[5]);
|
|
- fprintf(stdout, "\n data (bin) : %ld(%lu)", bData, length[6]);
|
+ fprintf(stdout, "\n data (bin) : %d(%lu)", bData, length[6]);
|
fprintf(stdout, "\n data (str) : %g(%lu)", szData, length[7]);
|
}
|
|
|
The other error related to foreign keys is a bogus one:
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
|
--- a/storage/innobase/handler/ha_innodb.cc
|
+++ b/storage/innobase/handler/ha_innodb.cc
|
@@ -12108,7 +12108,7 @@ create_table_info_t::create_foreign_keys()
|
dict_index_t* index = NULL;
|
fkerr_t index_error = FK_SUCCESS;
|
dict_index_t* err_index = NULL;
|
- ulint err_col;
|
+ ulint err_col = 0;
|
const bool tmp_table = m_flags2 & DICT_TF2_TEMPORARY;
|
const CHARSET_INFO* cs = thd_charset(m_thd);
|
const char* operation = "Create "; |
If dict_foreign_find_index() yields FK_INDEX_NOT_FOUND, the value of err_col will remain uninitialized, but it would not be used by foreign_push_index_error() either. MemorySanitizer starting with clang-16 does not allow uninitialized data to be passed by value or to be returned. A similar fix is needed for the test main.debug_sync when it is trying to report Missing synchronization point name:
diff --git a/sql/debug_sync.cc b/sql/debug_sync.cc
|
index eec95bee1f2..160851ae004 100644
|
--- a/sql/debug_sync.cc
|
+++ b/sql/debug_sync.cc
|
@@ -1008,7 +1008,7 @@ static bool debug_sync_eval_action(THD *thd, char *action_str, char *action_end)
|
st_debug_sync_action *action= NULL;
|
const char *errmsg;
|
char *ptr;
|
- char *token;
|
+ char *token= nullptr;
|
uint token_length= 0;
|
DBUG_ENTER("debug_sync_eval_action");
|
DBUG_ASSERT(thd); |
Another one is main.fulltext; for delimiters the w would not be initialized:
diff --git a/storage/myisam/ft_boolean_search.c b/storage/myisam/ft_boolean_search.c
|
index f69f1869383..3d95fffacaf 100644
|
--- a/storage/myisam/ft_boolean_search.c
|
+++ b/storage/myisam/ft_boolean_search.c
|
@@ -287,6 +287,8 @@ static int ftb_parse_query_internal(MYSQL_FTPARSER_PARAM *param,
|
uchar *end= (uchar*) query + len;
|
FT_WORD w;
|
|
+ w.pos= NULL;
|
+ w.len= 0;
|
info.prev= ' ';
|
info.quot= 0;
|
while (ft_get_word(cs, start, end, &w, &info)) |
The test innodb.page_compressed,lz4 fails because I do not have an instrumented lz4 compression library.
The test handler.heap and about 10 other tests are fixed by the following:
diff --git a/sql/sql_lex.h b/sql/sql_lex.h
|
index b6d91896142..3ab50d4aaa8 100644
|
--- a/sql/sql_lex.h
|
+++ b/sql/sql_lex.h
|
@@ -3260,7 +3260,7 @@ struct LEX: public Query_tables_list
|
Table_type table_type; /* Used for SHOW CREATE */
|
List<Key_part_spec> ref_list;
|
List<LEX_USER> users_list;
|
- List<Item> *insert_list,field_list,value_list,update_list;
|
+ List<Item> *insert_list= nullptr,field_list,value_list,update_list;
|
List<List_item> many_values;
|
List<set_var_base> var_list;
|
List<set_var_base> stmt_var_list; //SET_STATEMENT values |
The test main.subselect4 and 31 other tests are fixed by the following:
diff --git a/sql/multi_range_read.h b/sql/multi_range_read.h
|
index 57cfd21727f..d0fdb5a2c87 100644
|
--- a/sql/multi_range_read.h
|
+++ b/sql/multi_range_read.h
|
@@ -581,7 +581,7 @@ class DsMrr_impl
|
int dsmrr_explain_info(uint mrr_mode, char *str, size_t size);
|
private:
|
/* Buffer to store (key, range_id) pairs */
|
- Lifo_buffer *key_buffer;
|
+ Lifo_buffer *key_buffer= nullptr;
|
|
/*
|
The "owner" handler object (the one that is expected to "own" this object |
The function DsMrr_impl::dsmrr_init() would read a garbage key_buffer. This affects at least MyISAM, InnoDB and Aria.
The test main.mysql-interactive appears to fail because I do not have an instrumented ncurses library. The memory was marked uninitialized at allocation and the uninitialized use flagged in add_to_blob() a little later by __interceptor_strlen().
#3 0x000055c0e0cd8162 in __interceptor_malloc ()
|
#4 0x00007f741238d7f9 in _nc_first_db (state=<optimized out>, offset=<optimized out>) at ../../ncurses/tinfo/db_iterator.c:352
|
#5 0x00007f741239b20f in _nc_read_entry2 (name=0x7010000000a0 "dumb", filename=filename@entry=0x7fff875cb5e0 "dumb", tp=tp@entry=0x7160000002b0) at ../../ncurses/tinfo/read_entry.c:902
|
#6 0x00007f74123905b6 in _nc_setup_tinfo (tn=<optimized out>, tp=0x7160000002b0) at ../../ncurses/tinfo/lib_setup.c:626
|
#7 0x00007f7412390996 in _nc_setupterm (tname=<optimized out>, Filedes=1, errret=0x7fff875cc674, reuse=<optimized out>) at ../../ncurses/tinfo/lib_setup.c:916
|
#8 0x000055c0e0d3391d in put_info (str=0x55c0e09c4c74 "Welcome to the MariaDB monitor. Commands end with ; or \\g.", info_type=info_type@entry=INFO_INFO, error=error@entry=0, sqlstate=sqlstate@entry=0x0)
|
at /mariadb/10.5/client/mysql.cc:5092
|
#9 0x000055c0e0d3294c in main (argc=9, argv=0x70d000000070) at /mariadb/10.5/client/mysql.cc:1229
|
This test was recently added for MDEV-14448. I think that we can skip it on MSAN.
On merge to 10.6, there were no additonal failures. Hopefully it will stay like that when merging to further later-version branches.
I believe the fix provided for multi_range_read.h is legitimate, and here is why. So, we had this MSAN warning:
==243288==WARNING: MemorySanitizer: use-of-uninitialized-value
|
#0 0x5577ba464d89 in DsMrr_impl::dsmrr_init(handler*, st_range_seq_if*, void*, unsigned int, unsigned int, st_handler_buffer*) /home/oleg/server/sql/multi_range_read.cc:1208:31
|
#1 0x5577bbd61bd2 in ha_myisam::multi_range_read_init(st_range_seq_if*, void*, unsigned int, unsigned int, st_handler_buffer*) /home/oleg/server/storage/myisam/ha_myisam.cc:2639:17
|
#2 0x5577bac5ab4b in QUICK_RANGE_SELECT::reset() /home/oleg/server/sql/opt_range.cc:12703:16
|
#3 0x5577ba084442 in join_init_read_record(st_join_table*) /home/oleg/server/sql/sql_select.cc:22170:64
|
#4 0x5577ba0efae8 in sub_select(JOIN*, st_join_table*, bool) /home/oleg/server/sql/sql_select.cc:21219:12
|
#5 0x5577ba08e802 in do_select(JOIN*, Procedure*) /home/oleg/server/sql/sql_select.cc:20739:14
|
#6 0x5577ba08ba6e in JOIN::exec_inner() /home/oleg/server/sql/sql_select.cc:4625:50
|
#7 0x5577ba0895fa in JOIN::exec() /home/oleg/server/sql/sql_select.cc:4405:3
|
#8 0x5577ba0166ec in mysql_select(THD*, TABLE_LIST*, List<Item>&, Item*, unsigned int, st_order*, st_order*, Item*, st_order*, unsigned long long, select_result*, st_select_lex_unit*, st_select_lex*) /home/oleg/server/sql/sql_select.cc:4882:9
|
#9 0x5577ba015f18 in handle_select(THD*, LEX*, select_result*, unsigned long) /home/oleg/server/sql/sql_select.cc:449:10
|
We had an uninitialized key_buffer passed to the init() method:
|
multi_range_read.cc |
if ((res= index_strategy->init(secondary_file, seq_funcs, seq_init_param,
|
n_ranges, mode, &keypar, key_buffer,
|
&buf_manager)) ||
|
(res= disk_strategy->init(primary_file, index_strategy, mode,
|
&rowid_buffer, rowid_filter)))
|
init() is a pure virtual method that is overriden in Mrr_simple_index_reader and Mrr_ordered_index_reader classes. However, in the case of Mrr_simple_index_reader the parameter key_buffer is not used, so it is just passed to the method according to the calling convention. And in the case of Mrr_ordered_index_reader the key_buffer is used, and it is always initialized before the init() call (at least in all cases covered by the test suite). I couldn't come up with a case when an uninitialized key_buffer might have been passed to the method.
So initializing key_buffer to nullptr as this is done at
commit 09d991d01c47e22030879e5bf0c7a4893a598199
|
Author: Marko Mäkelä <marko.makela@mariadb.com>
|
Date: Mon Mar 18 16:01:29 2024 +0200
|
is valid to silence the memory sanitizer warnings.
The test test_bind_result_ext1 in main.mysql_client_test fails because only the least significant 32 bits of long bData are being initialized:
10.5 fb774eb1ebab7d397c10a516be364d395440d079
0x000055dbbce480f2 in convert_froma_string (r_param=r_param@entry=0x71c0000002b8, buffer=0x72400000201f "206\0036.7", len=<optimized out>) at /mariadb/10.5/libmariadb/libmariadb/ma_stmt_codec.c:536536 longstore(r_param->buffer, (int32)val);(rr) bt#0 0x000055dbbce480f2 in convert_froma_string (r_param=r_param@entry=0x71c0000002b8, buffer=0x72400000201f "206\0036.7", len=<optimized out>) at /mariadb/10.5/libmariadb/libmariadb/ma_stmt_codec.c:536#1 0x000055dbbce45c2c in ps_fetch_string (r_param=0x71c0000002b8, field=<optimized out>, row=0x7fff9b9cede0) at /mariadb/10.5/libmariadb/libmariadb/ma_stmt_codec.c:1197#2 0x000055dbbce2a01c in mthd_stmt_fetch_to_bind (stmt=<optimized out>, row=<optimized out>) at /mariadb/10.5/libmariadb/libmariadb/mariadb_stmt.c:446#3 0x000055dbbce327b6 in mysql_stmt_fetch (stmt=stmt@entry=0x718000000000) at /mariadb/10.5/libmariadb/libmariadb/mariadb_stmt.c:1503#4 0x000055dbbcb881d9 in wrap_mysql_stmt_fetch (stmt=0x718000000000) at /mariadb/10.5/tests/nonblock-wrappers.h:351#5 test_bind_result_ext1 () at /mariadb/10.5/tests/mysql_client_test.c:3927#6 0x000055dbbcb52204 in main (argc=0, argv=0x70e000000078) at /mariadb/10.5/tests/mysql_client_fw.c:1459This would appear to be a bug in Connector/C. long is 32 bits only on 32-bit platforms or LLP64 platforms, such as Microsoft Windows.