From 8925b4a935115aef51cfff22564ee2b0d5fb5fce Mon Sep 17 00:00:00 2001 From: Sergey Petrunya Date: Thu, 16 Oct 2014 17:57:13 +0400 Subject: MDEV-6878: Use of uninitialized saved_primary_key in Mrr_ordered_index_reader::resume_read() - Make Mrr_ordered_index_reader::resume_read() restore index position only if it was saved before with Mrr_ordered_index_reader::interrupt_read(). --- sql/multi_range_read.cc | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'sql/multi_range_read.cc') diff --git a/sql/multi_range_read.cc b/sql/multi_range_read.cc index b63db9ecea2..4665957528f 100644 --- a/sql/multi_range_read.cc +++ b/sql/multi_range_read.cc @@ -426,6 +426,7 @@ bool Mrr_ordered_index_reader::set_interruption_temp_buffer(uint rowid_length, *space_start += key_len; have_saved_rowid= FALSE; + read_was_interrupted= FALSE; return FALSE; } @@ -434,6 +435,7 @@ void Mrr_ordered_index_reader::set_no_interruption_temp_buffer() support_scan_interruptions= FALSE; saved_key_tuple= saved_rowid= saved_primary_key= NULL; /* safety */ have_saved_rowid= FALSE; + read_was_interrupted= FALSE; } void Mrr_ordered_index_reader::interrupt_read() @@ -451,6 +453,7 @@ void Mrr_ordered_index_reader::interrupt_read() &table->key_info[table->s->primary_key], table->key_info[table->s->primary_key].key_length); } + read_was_interrupted= TRUE; /* Save the last rowid */ memcpy(saved_rowid, file->ref, file->ref_length); @@ -468,14 +471,17 @@ void Mrr_ordered_index_reader::position() void Mrr_ordered_index_reader::resume_read() { TABLE *table= file->get_table(); - KEY *used_index= &table->key_info[file->active_index]; - key_restore(table->record[0], saved_key_tuple, - used_index, used_index->key_length); - if (saved_primary_key) + if (read_was_interrupted) { - key_restore(table->record[0], saved_primary_key, - &table->key_info[table->s->primary_key], - table->key_info[table->s->primary_key].key_length); + KEY *used_index= &table->key_info[file->active_index]; + key_restore(table->record[0], saved_key_tuple, + used_index, used_index->key_length); + if (saved_primary_key) + { + key_restore(table->record[0], saved_primary_key, + &table->key_info[table->s->primary_key], + table->key_info[table->s->primary_key].key_length); + } } } @@ -557,8 +563,7 @@ int Mrr_ordered_index_reader::init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, is_mrr_assoc= !MY_TEST(mode & HA_MRR_NO_ASSOCIATION); mrr_funcs= *seq_funcs; source_exhausted= FALSE; - if (support_scan_interruptions) - bzero(saved_key_tuple, key_info->key_length); + read_was_interrupted= false; have_saved_rowid= FALSE; return 0; } -- cgit v1.2.1 From 9cb002b359ca8281cd30b711690dd589a254db4e Mon Sep 17 00:00:00 2001 From: Sergey Petrunya Date: Wed, 29 Oct 2014 01:37:58 +0300 Subject: MDEV-6878: Use of uninitialized saved_primary_key in Mrr_ordered_index_reader::resume_read() (Backport to 5.3) (variant #2, with fixed coding style) - Make Mrr_ordered_index_reader::resume_read() restore index position only if it was saved before with Mrr_ordered_index_reader::interrupt_read(). --- sql/multi_range_read.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'sql/multi_range_read.cc') diff --git a/sql/multi_range_read.cc b/sql/multi_range_read.cc index 727ebc29032..6dfd8bec980 100644 --- a/sql/multi_range_read.cc +++ b/sql/multi_range_read.cc @@ -410,6 +410,7 @@ bool Mrr_ordered_index_reader::set_interruption_temp_buffer(uint rowid_length, *space_start += key_len; have_saved_rowid= FALSE; + read_was_interrupted= FALSE; return FALSE; } @@ -418,6 +419,7 @@ void Mrr_ordered_index_reader::set_no_interruption_temp_buffer() support_scan_interruptions= FALSE; saved_key_tuple= saved_rowid= saved_primary_key= NULL; /* safety */ have_saved_rowid= FALSE; + read_was_interrupted= FALSE; } void Mrr_ordered_index_reader::interrupt_read() @@ -435,6 +437,7 @@ void Mrr_ordered_index_reader::interrupt_read() &table->key_info[table->s->primary_key], table->key_info[table->s->primary_key].key_length); } + read_was_interrupted= TRUE; /* Save the last rowid */ memcpy(saved_rowid, file->ref, file->ref_length); @@ -452,6 +455,10 @@ void Mrr_ordered_index_reader::position() void Mrr_ordered_index_reader::resume_read() { TABLE *table= file->get_table(); + + if (!read_was_interrupted) + return; + KEY *used_index= &table->key_info[file->active_index]; key_restore(table->record[0], saved_key_tuple, used_index, used_index->key_length); @@ -541,8 +548,7 @@ int Mrr_ordered_index_reader::init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, is_mrr_assoc= !test(mode & HA_MRR_NO_ASSOCIATION); mrr_funcs= *seq_funcs; source_exhausted= FALSE; - if (support_scan_interruptions) - bzero(saved_key_tuple, key_info->key_length); + read_was_interrupted= false; have_saved_rowid= FALSE; return 0; } -- cgit v1.2.1 From 06c7f493e35dd1d798f44b6920c4b7957d42c4de Mon Sep 17 00:00:00 2001 From: Sergey Petrunya Date: Thu, 13 Nov 2014 13:56:35 +0300 Subject: MDEV-7068: MRR accessing uninitialised bytes, test case failure main.innodb_mrr - Don't call index_reader->interrupt_read() if the index reader has returned all rows that matched its keys. --- sql/multi_range_read.cc | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'sql/multi_range_read.cc') diff --git a/sql/multi_range_read.cc b/sql/multi_range_read.cc index bbc287bf239..2479bc3258c 100644 --- a/sql/multi_range_read.cc +++ b/sql/multi_range_read.cc @@ -684,8 +684,19 @@ int Mrr_ordered_rndpos_reader::refill_from_index_reader() rowid_buffer->write_ptr2= (uchar*)&range_info; rowid_buffer->write(); } - - index_reader->interrupt_read(); + + /* + When index_reader_needs_refill=TRUE, this means we've got all of index + tuples for lookups keys that index_reader had. We are not in the middle + of an index read, so there is no need to call interrupt_read. + + Actually, we must not call interrupt_read(), because it could be that we + haven't read a single row (because all index lookups returned + HA_ERR_KEY_NOT_FOUND). In this case, interrupt_read() will cause [harmless] + valgrind warnings when trying to save garbage from table->record[0]. + */ + if (!index_reader_needs_refill) + index_reader->interrupt_read(); /* Sort the buffer contents by rowid */ rowid_buffer->sort((qsort2_cmp)rowid_cmp_reverse, (void*)file); -- cgit v1.2.1