revision-id: a180dac22deedf6cae7ef026255102af37737d67 (mariadb-10.2.31-64-ga180dac) parent(s): a0ce62f804dc1a0073688bea46dc2e6728cf55e3 author: Igor Babaev committer: Igor Babaev timestamp: 2020-03-19 21:52:52 -0700 message: MDEV-17177 Crash in Item_func_in::cleanup() for SELECT executed via prepared statement The method Item_func_in::build_clone() that builds a clone item for an Item_func_in item first calls a generic method Item_func::build_item() that builds the the clones for the arguments of the Item_func_in item to be cloned, creates a copy of the Item_func_in object and attaches the clones for the arguments to this copy. Then the method Item_func_in::build_clone() makes the copy fully independent on the copied object in order to guarantee a proper destruction of the clone. The fact is the copy of the Item_func_in object is registered as any other item object and should be destructed as any other item object. If the method Item_func::build_item fails to build a clone of an argument then it returns 0. In this case no copy of the Item_func_in object should be created. Otherwise the finalizing actions for this copy would not be performed and the copy would remain in a state that would prevent its proper destruction. The code of Item_func_in::build_clone() before this patch created the copy of the Item_func_in object before cloning the argument items. If this cloning failed the server crashed when trying to destruct the copy item. The code of Item_row::build_clone() was changed similarly to the code of Item_func::build_clone though this code could not cause any problems. --- mysql-test/r/derived_cond_pushdown.result | 12 +++++++++++ mysql-test/t/derived_cond_pushdown.test | 16 ++++++++++++++ sql/item.cc | 35 +++++++++++++++++-------------- sql/item_row.cc | 11 ++++++---- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/mysql-test/r/derived_cond_pushdown.result b/mysql-test/r/derived_cond_pushdown.result index 1010ed6..e2c2463 100644 --- a/mysql-test/r/derived_cond_pushdown.result +++ b/mysql-test/r/derived_cond_pushdown.result @@ -10581,4 +10581,16 @@ a b max_c a b dayname(v1.b) DROP VIEW v1; DROP TABLE t1, t2; SET optimizer_switch=DEFAULT; +# +# MDEV-17177: an attempt to push down IN predicate when one of +# the arguments is too complex to be cloned +# +CREATE TABLE t1 (a VARCHAR(8)); +INSERT INTO t1 VALUES ('abc'),('def'); +CREATE VIEW v1 AS SELECT * FROM t1 GROUP BY a; +SELECT * FROM v1 WHERE IF( a REGEXP 'def', 'foo', a ) IN ('abc', 'foobar'); +a +abc +DROP VIEW v1; +DROP TABLE t1; # End of 10.2 tests diff --git a/mysql-test/t/derived_cond_pushdown.test b/mysql-test/t/derived_cond_pushdown.test index ec15791..a7df65f 100644 --- a/mysql-test/t/derived_cond_pushdown.test +++ b/mysql-test/t/derived_cond_pushdown.test @@ -2168,4 +2168,20 @@ DROP TABLE t1, t2; SET optimizer_switch=DEFAULT; + + +--echo # +--echo # MDEV-17177: an attempt to push down IN predicate when one of +--echo # the arguments is too complex to be cloned +--echo # + +CREATE TABLE t1 (a VARCHAR(8)); +INSERT INTO t1 VALUES ('abc'),('def'); +CREATE VIEW v1 AS SELECT * FROM t1 GROUP BY a; + +SELECT * FROM v1 WHERE IF( a REGEXP 'def', 'foo', a ) IN ('abc', 'foobar'); + +DROP VIEW v1; +DROP TABLE t1; + --echo # End of 10.2 tests diff --git a/sql/item.cc b/sql/item.cc index 4f8433c..ab7340a 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -2389,37 +2389,40 @@ bool Item_func_or_sum::agg_item_set_converter(const DTCollation &coll, @param mem_root part of the memory for the clone @details - This method gets copy of the current item and also - build clones for its referencies. For the referencies - build_copy is called again. + This method fisrt builds clones of the arguments. If it is successful with + buiding the clones then it constructs a copy of this Item_func_or_sum object + and attaches to it the built clones of the arguments. @retval - clone of the item - 0 if an error occured + clone of the item on success + 0 on a failure */ Item* Item_func_or_sum::build_clone(THD *thd, MEM_ROOT *mem_root) { - Item_func_or_sum *copy= (Item_func_or_sum *) get_copy(thd, mem_root); - if (!copy) - return 0; + Item *copy_tmp_args[2]= {0,0}; + Item **copy_args= copy_tmp_args; if (arg_count > 2) { - copy->args= - (Item**) alloc_root(mem_root, sizeof(Item*) * arg_count); - if (!copy->args) + if (!(copy_args= (Item**) alloc_root(mem_root, sizeof(Item*) * arg_count))) return 0; } - else if (arg_count > 0) - copy->args= copy->tmp_arg; - - for (uint i= 0; i < arg_count; i++) { Item *arg_clone= args[i]->build_clone(thd, mem_root); if (!arg_clone) return 0; - copy->args[i]= arg_clone; + copy_args[i]= arg_clone; + } + Item_func_or_sum *copy= (Item_func_or_sum *) get_copy(thd, mem_root); + if (!copy) + return 0; + if (arg_count > 2) + copy->args= copy_args; + else if (arg_count > 0) + { + copy->args= copy->tmp_arg; + memcpy(copy->args, copy_args, sizeof(Item *) * arg_count); } return copy; } diff --git a/sql/item_row.cc b/sql/item_row.cc index 000253e..6ae049d 100644 --- a/sql/item_row.cc +++ b/sql/item_row.cc @@ -166,17 +166,20 @@ void Item_row::bring_value() Item* Item_row::build_clone(THD *thd, MEM_ROOT *mem_root) { - Item_row *copy= (Item_row *) get_copy(thd, mem_root); - if (!copy) + Item **copy_args= 0; + if (!(copy_args= (Item**) alloc_root(mem_root, sizeof(Item*) * arg_count))) return 0; - copy->args= (Item**) alloc_root(mem_root, sizeof(Item*) * arg_count); for (uint i= 0; i < arg_count; i++) { Item *arg_clone= args[i]->build_clone(thd, mem_root); if (!arg_clone) return 0; - copy->args[i]= arg_clone; + copy_args[i]= arg_clone; } + Item_row *copy= (Item_row *) get_copy(thd, mem_root); + if (!copy) + return 0; + copy->args= copy_args; return copy; }