diff --git a/mysql-test/main/log_slow.result b/mysql-test/main/log_slow.result index e254be8a316..168cf02d3a5 100644 --- a/mysql-test/main/log_slow.result +++ b/mysql-test/main/log_slow.result @@ -209,3 +209,14 @@ b`; SET timestamp=1234567890; select count(*) from mysql.global_priv where length(priv)>2 # End of 10.5 tests +# +# MDEV-34251 Conditional jump or move depends on uninitialised value in +# ha_handler_stats::has_stats +# +set @@global.log_slow_verbosity=""; +connect con1,localhost,root,,; +connection con1; +set long_query_time=0.0, log_slow_verbosity='engine'; +connection default; +disconnect con1; +# End of 10.6 tests diff --git a/mysql-test/main/log_slow.test b/mysql-test/main/log_slow.test index 887a81913a0..3b6488265c0 100644 --- a/mysql-test/main/log_slow.test +++ b/mysql-test/main/log_slow.test @@ -225,3 +225,19 @@ let SEARCH_OUTPUT=matches; source include/search_pattern_in_file.inc; --echo # End of 10.5 tests + +--echo # +--echo # MDEV-34251 Conditional jump or move depends on uninitialised value in +--echo # ha_handler_stats::has_stats +--echo # + +set @@global.log_slow_verbosity=""; +connect (con1,localhost,root,,); +connection con1; + +# valgrind or asan would notice if engine stats are accessed wrong. +set long_query_time=0.0, log_slow_verbosity='engine'; +connection default; +disconnect con1; + +--echo # End of 10.6 tests diff --git a/sql/ha_handler_stats.h b/sql/ha_handler_stats.h index 362a19dab0b..ddf284b48aa 100644 --- a/sql/ha_handler_stats.h +++ b/sql/ha_handler_stats.h @@ -44,6 +44,12 @@ public: /* Time spent in engine, in timer_tracker_frequency() units */ ulonglong engine_time; uint active; /* <> 0 if status has to be updated */ + + ha_handler_stats() + { + active= 0; + } + #define first_stat pages_accessed #define last_stat engine_time inline void reset() @@ -61,6 +67,8 @@ public: } inline bool has_stats() { + if (!active) + return 0; ulonglong *to= &first_stat; do { diff --git a/sql/handler.h b/sql/handler.h index 4f5065fe006..51b08cc943b 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -3216,7 +3216,7 @@ protected: ha_rows estimation_rows_to_insert; handler *lookup_handler; - /* Statistics for the query. Updated if handler_stats.in_use is set */ + /* Statistics for the query. Updated if handler_stats.active is set */ ha_handler_stats active_handler_stats; void set_handler_stats(); public: @@ -3456,7 +3456,6 @@ public: ("handler created F_UNLCK %d F_RDLCK %d F_WRLCK %d", F_UNLCK, F_RDLCK, F_WRLCK)); reset_statistics(); - active_handler_stats.active= 0; } virtual ~handler(void) { diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 06b43277daa..4c1a5796653 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1002,9 +1002,7 @@ void close_thread_table(THD *thd, TABLE **table_ptr) file->update_global_table_stats(); file->update_global_index_stats(); - if (unlikely(thd->variables.log_slow_verbosity & - LOG_SLOW_VERBOSITY_ENGINE) && - likely(file->handler_stats)) + if (unlikely(file->handler_stats) && file->handler_stats->active) { Exec_time_tracker *tracker; if ((tracker= file->get_time_tracker())) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index d81a39d2354..6e1743e78f8 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -5998,7 +5998,7 @@ void THD::store_slow_query_state(Sub_statement_state *backup) backup->tmp_tables_disk_used= tmp_tables_disk_used; backup->tmp_tables_size= tmp_tables_size; backup->tmp_tables_used= tmp_tables_used; - backup->handler_stats= handler_stats; + backup->handler_stats= handler_stats; } /* Reset variables related to slow query log */ @@ -6016,6 +6016,8 @@ void THD::reset_slow_query_state() tmp_tables_used= 0; if ((variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_ENGINE)) handler_stats.reset(); + else + handler_stats.active= 0; } /* @@ -6034,7 +6036,7 @@ void THD::add_slow_query_state(Sub_statement_state *backup) tmp_tables_disk_used+= backup->tmp_tables_disk_used; tmp_tables_size+= backup->tmp_tables_size; tmp_tables_used+= backup->tmp_tables_used; - if ((variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_ENGINE)) + if (handler_stats.active && backup->handler_stats.active) handler_stats.add(&backup->handler_stats); } diff --git a/sql/sql_class.h b/sql/sql_class.h index 30e98a7406f..8688f1ee35d 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -5783,10 +5783,18 @@ public: lex= backup_lex; } - bool should_collect_handler_stats() const + bool should_collect_handler_stats() { - return (variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_ENGINE) || - lex->analyze_stmt; + /* + We update handler_stats.active to ensure that we have the same + value across the whole statement. + This function is only called from TABLE::init() so the value will + be the same for the whole statement. + */ + handler_stats.active= + ((variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_ENGINE) || + lex->analyze_stmt); + return handler_stats.active; } /* Return true if we should create a note when an unusable key is found */