MDEV-18970: uninited var can be read in gtid_delete_pending()

Problem:
========
gcc 8 -O2 seems to indicate a real error for this code:

direct_pos= table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION;

the warning: /mariadb/10.4/sql/rpl_gtid.cc:980:7:
warning: 'direct_pos' may be used uninitialized in this function [-Wmaybe-uninitialized]

Analysis:
=========
'direct_pos' is a variable which holds 'table_flags'. If this flag is set it means
that a record within a table can be directly located by using its position. If
this flag is set to '0' means there is no direct access is available, hence
index scan must be initiated to locate the record.  This direct_pos is used to
locate a row within mysql.gtid_slave_pos table for deletion.

Prior to the initialization of 'direct_pos' following steps take place.

1. mysql.gtid_slave_pos table is opened and 'table_opened' flag is set to true.
2. State check for mysql.gtid_slave_pos table is initiated.

If there is a failure during step2 code will be redirected to the error handling
part. This error handling code will access uninitialized value of 'direct_pos'.
This results in above mentioned warning.

Another issue found during analysis is the error handling code uses '!direct_pos'
to identify if the index is initialized or not. This is incorrect.

The index initialization code is shown below.

  if (!direct_pos && (err= table->file->ha_index_init(0, 0)))
    {
      table->file->print_error(err, MYF(0));
      goto end;
    }

In case there is a failure during ha_index_init code will be redirected to end
part which tries to close the uninitialized index. It will result in an assert

10.4/sql/handler.h:3186: int handler::ha_index_end(): Assertion `inited==INDEX'
failed.

Fix:
===
Introduce a new variable named 'index_inited'. Set this variable upon successful
initialization of index initialization otherwise by default it is false. Use
this variable during error handling.

1 files changed, 16 insertions, 4 deletions

diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc
index 17f474c2acf..2ba9dd87a8f 100644
--- a/sql/rpl_gtid.cc
+++ b/sql/rpl_gtid.cc
@@ -877,6 +877,7 @@ rpl_slave_state::gtid_delete_pending(THD *thd,
     handler::Table_flags direct_pos;
     list_element *cur, **cur_ptr_ptr;
     bool table_opened= false;
+    bool index_inited= false;
     void *hton= (*list_ptr)->hton;
 
     thd->reset_for_next_command();
@@ -915,10 +916,14 @@ rpl_slave_state::gtid_delete_pending(THD *thd,
     bitmap_set_bit(table->read_set, table->field[0]->field_index);
     bitmap_set_bit(table->read_set, table->field[1]->field_index);
 
-    if (!direct_pos && (err= table->file->ha_index_init(0, 0)))
+    if (!direct_pos)
     {
-      table->file->print_error(err, MYF(0));
-      goto end;
+      if ((err= table->file->ha_index_init(0, 0)))
+      {
+        table->file->print_error(err, MYF(0));
+        goto end;
+      }
+      index_inited= true;
     }
 
     cur = *list_ptr;
@@ -977,7 +982,14 @@ rpl_slave_state::gtid_delete_pending(THD *thd,
 end:
     if (table_opened)
     {
-      if (!direct_pos)
+      DBUG_ASSERT(direct_pos || index_inited || err);
+      /*
+        Index may not be initialized if there was a failure during
+        'ha_index_init'. Hence check if index initialization is successful and
+        then invoke ha_index_end(). Ending an index which is not initialized
+        will lead to assert.
+      */
+      if (index_inited)
         table->file->ha_index_end();
 
       if (err || (err= ha_commit_trans(thd, FALSE)))