MDEV-34251 Conditional jump or move depends on uninitialised value in ha_handler_stats::has_stats

Fixed by checking handler_stats if it's active instead of
thd->variables.log_slow_verbosity & LOG_SLOW_VERBOSITY_ENGINE.

Reviewed-by: Sergei Petrunia <sergey@mariadb.com>

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

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

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
     {

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)
   {

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 &
+  if (unlikely(file->handler_stats) && file->handler_stats->active)
-               LOG_SLOW_VERBOSITY_ENGINE) &&
-      likely(file->handler_stats))
   {
     Exec_time_tracker *tracker;
     if ((tracker= file->get_time_tracker()))

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);
 }
 

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 */