[Commits] 46271662a71: MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value
revision-id: 46271662a71c4ba7826a7760059c9853586ed424 (mariadb-10.2.31-719-g46271662a71) parent(s): 20f6c403eb976a6dd25cb58d0ce17f6da2566253 author: Varun Gupta committer: Varun Gupta timestamp: 2021-01-28 18:09:58 +0530 message: MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value The parameter is_ror_scan was not initialized. It should be initialized whenever we try to call check_quick_select() for a SEL_ARG. --- mysql-test/r/range.result | 17 +++++++++++++++++ mysql-test/r/range_mrr_icp.result | 17 +++++++++++++++++ mysql-test/t/range.test | 14 ++++++++++++++ sql/opt_range.cc | 2 ++ 4 files changed, 50 insertions(+) diff --git a/mysql-test/r/range.result b/mysql-test/r/range.result index 1d07cb04c06..6a3850c0ed9 100644 --- a/mysql-test/r/range.result +++ b/mysql-test/r/range.result @@ -3184,5 +3184,22 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR id a b code num DROP TABLE t1, t2; # +# MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +# +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 2 +1 SIMPLE t1 ALL PRIMARY,v NULL NULL NULL 8 Range checked for each record (index map: 0x3) +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +pk i v a b +1 1 2 1 2 +2 2 4 2 4 +drop table t1, t2; +# # End of 10.2 tests # diff --git a/mysql-test/r/range_mrr_icp.result b/mysql-test/r/range_mrr_icp.result index f3203fea73d..24f42f34ce5 100644 --- a/mysql-test/r/range_mrr_icp.result +++ b/mysql-test/r/range_mrr_icp.result @@ -3196,6 +3196,23 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR id a b code num DROP TABLE t1, t2; # +# MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +# +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 2 +1 SIMPLE t1 ALL PRIMARY,v NULL NULL NULL 8 Range checked for each record (index map: 0x3) +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +pk i v a b +1 1 2 1 2 +2 2 4 2 4 +drop table t1, t2; +# # End of 10.2 tests # set optimizer_switch=@mrr_icp_extra_tmp; diff --git a/mysql-test/t/range.test b/mysql-test/t/range.test index 2f55889afec..890377ed977 100644 --- a/mysql-test/t/range.test +++ b/mysql-test/t/range.test @@ -2217,6 +2217,20 @@ SELECT * FROM t1 JOIN t2 ON (t2.code = t1.b) WHERE t1.a NOT IN ('baz', 'qux') OR DROP TABLE t1, t2; + +--echo # +--echo # MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value +--echo # + +create table t1 (pk int, i int, v int, primary key (pk), key(v)); +insert into t1 (pk,i,v) values (1,1,2),(2,2,4),(3,3,6),(4,4,8),(5,5,10),(6,6,12),(7,7,14),(8,8,16); +create table t2 (a int, b int); +insert into t2 values (1,2),(2,4); +EXPLAIN +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +select * from t1 inner join t2 on ( t2.b = t1.v or t2.a = t1.pk); +drop table t1, t2; + --echo # --echo # End of 10.2 tests --echo # diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 7785c768fbc..b98d773aa3a 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -2457,6 +2457,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, param.remove_false_where_parts= remove_false_parts_of_where; param.force_default_mrr= ordered_output; param.possible_keys.clear_all(); + param.is_ror_scan= FALSE; thd->no_errors=1; // Don't warn about NULL init_sql_alloc(&alloc, thd->variables.range_alloc_block_size, 0, @@ -6819,6 +6820,7 @@ static TRP_RANGE *get_key_scans_params(PARAM *param, SEL_TREE *tree, double found_read_time; uint mrr_flags, buf_size; INDEX_SCAN_INFO *index_scan; + param->is_ror_scan= FALSE; uint keynr= param->real_keynr[idx]; if (key->type == SEL_ARG::MAYBE_KEY || key->maybe_flag)
Hi Varun, On Thu, Jan 28, 2021 at 06:14:08PM +0530, varun wrote:
revision-id: 46271662a71c4ba7826a7760059c9853586ed424 (mariadb-10.2.31-719-g46271662a71) parent(s): 20f6c403eb976a6dd25cb58d0ce17f6da2566253 author: Varun Gupta committer: Varun Gupta timestamp: 2021-01-28 18:09:58 +0530 message:
MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value
The parameter is_ror_scan was not initialized. It should be initialized whenever we try to call check_quick_select() for a SEL_ARG.
...
diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 7785c768fbc..b98d773aa3a 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -2457,6 +2457,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, param.remove_false_where_parts= remove_false_parts_of_where; param.force_default_mrr= ordered_output; param.possible_keys.clear_all(); + param.is_ror_scan= FALSE;
thd->no_errors=1; // Don't warn about NULL init_sql_alloc(&alloc, thd->variables.range_alloc_block_size, 0, @@ -6819,6 +6820,7 @@ static TRP_RANGE *get_key_scans_params(PARAM *param, SEL_TREE *tree, double found_read_time; uint mrr_flags, buf_size; INDEX_SCAN_INFO *index_scan; + param->is_ror_scan= FALSE; uint keynr= param->real_keynr[idx]; if (key->type == SEL_ARG::MAYBE_KEY || key->maybe_flag)
check_quick_select actually does initialize PARAM::is_ror_scan, except for "degenerate" cases that are handled at the start of the function. So, I think an easier fix would be to just have check_quick_select to set is_ror_scan for the degenerate cases, too. I've also removed another redundant initialization, and it seems things work. Please find the patch below: diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 7785c768fbc..f2c55b2b5a5 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -3065,7 +3065,6 @@ bool calculate_cond_selectivity_for_table(THD *thd, TABLE *table, Item **cond) param.mem_root= &alloc; param.old_root= thd->mem_root; param.table= table; - param.is_ror_scan= FALSE; param.remove_false_where_parts= true; if (create_key_parts_for_pseudo_indexes(¶m, used_fields)) @@ -10385,6 +10384,7 @@ ha_rows check_quick_select(PARAM *param, uint idx, bool index_only, uint keynr= param->real_keynr[idx]; DBUG_ENTER("check_quick_select"); + param->is_ror_scan= FALSE; /* Handle cases when we don't have a valid non-empty list of range */ if (!tree) DBUG_RETURN(HA_POS_ERROR); BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net
On Thu, Jan 28, 2021 at 07:42:07PM +0300, Sergey Petrunia wrote:
Hi Varun,
@@ -6819,6 +6820,7 @@ static TRP_RANGE *get_key_scans_params(PARAM *param, SEL_TREE *tree, double found_read_time; uint mrr_flags, buf_size; INDEX_SCAN_INFO *index_scan; + param->is_ror_scan= FALSE; uint keynr= param->real_keynr[idx]; if (key->type == SEL_ARG::MAYBE_KEY || key->maybe_flag)
check_quick_select actually does initialize PARAM::is_ror_scan, except for "degenerate" cases that are handled at the start of the function.
So, I think an easier fix would be to just have check_quick_select to set is_ror_scan for the degenerate cases, too.
I've also removed another redundant initialization, and it seems things work. Please find the patch below:
diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 7785c768fbc..f2c55b2b5a5 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -3065,7 +3065,6 @@ bool calculate_cond_selectivity_for_table(THD *thd, TABLE *table, Item **cond) param.mem_root= &alloc; param.old_root= thd->mem_root; param.table= table; - param.is_ror_scan= FALSE; param.remove_false_where_parts= true;
if (create_key_parts_for_pseudo_indexes(¶m, used_fields))
Ok, this one should be removed because it causes a failure like this: ==10660== Conditional jump or move depends on uninitialised value(s) ==10660== at 0xAC760A: sel_arg_range_seq_next(void*, st_key_multi_range*) (opt_range_mrr.cc:316) ==10660== by 0xACCD96: records_in_column_ranges(PARAM*, unsigned int, SEL_ARG*) (opt_range.cc:2864) ==10660== by 0xACD821: calculate_cond_selectivity_for_table(THD*, TABLE*, Item**) (opt_range.cc:3117) ==10660== by 0x742C69: make_join_statistics(JOIN*, List<TABLE_LIST>&, st_dynamic_array*) (sql_select.cc:4510) ==10660== by 0x738887: JOIN::optimize_inner() (sql_select.cc:1587) ==10660== by 0x736D09: JOIN::optimize() (sql_select.cc:1117) ==10660== by 0x74039B: mysql_select(THD*, TABLE_LIST*, unsigned int, List<Item>&, Item*, unsigned int, st_order*, st_order*, Item*, st_order*, unsigned long long, select_result*, st_select_lex_unit*, st_select_lex*) (sql_select.cc:3822) ==10660== by 0x777D5F: mysql_explain_union(THD*, st_select_lex_unit*, select_result*) (sql_select.cc:25334)
@@ -10385,6 +10384,7 @@ ha_rows check_quick_select(PARAM *param, uint idx, bool index_only, uint keynr= param->real_keynr[idx]; DBUG_ENTER("check_quick_select");
+ param->is_ror_scan= FALSE; /* Handle cases when we don't have a valid non-empty list of range */ if (!tree) DBUG_RETURN(HA_POS_ERROR);
BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net
participants (2)
-
Sergey Petrunia
-
varun