MDEV-17869 AddressSanitizer: use-after-poison in Item_change_list::ro…

…llback_item_tree_changes

it's incorrect to use change_item_tree() to replace arguments
of top-level AND/OR, because they (arguments) are stored in a List,
so a pointer to an argument is in the list_node, and individual
list_node's of top-level AND/OR can be deleted in Item_cond::build_equal_items().
In that case rollback_item_tree_changes() will modify the deleted object.

Luckily, it's not needed to use change_item_tree() for top-level
AND/OR, because the whole top-level item is copied and preserved
in prep_where and prep_on, and restored from there.

So, just don't.

Additionally to the test case in the commit it fixes
* ASAN failure of main.opt_tvc --ps
* ASAN failure of main.having_cond_pushdown --ps

mysql-test/main/prepare.result

@@ -64,3 +64,19 @@ SQRT(?) is not null
 #
 # End of 10.3 tests
 #
+#
+# MDEV-17869 AddressSanitizer: use-after-poison in Item_change_list::rollback_item_tree_changes
+#
+create table t1 (pk int, v1 varchar(1));
+insert t1 values (1,'v'),(2,'v'),(3,'c');
+create table t2 (pk int, v1 varchar(1));
+insert t2 values (1,'x');
+create table t3 (pk int, i1 int, v1 varchar(1));
+insert t3 values (10,8,9);
+execute immediate 'select straight_join 1 from (t1 join t2 on (t1.v1 = t2.v1))
+where (3, 6) in (select tc.pk,  t3.i1 from (t3 join t1 as tc on (tc.v1 = t3.v1)) having tc.pk > 1 );';
+1
+drop table t1, t2, t3;
+#
+# End of 10.4 tests
+#

mysql-test/main/prepare.test

@@ -52,3 +52,20 @@ execute p1 using 17864960750176564435;
 --echo #
 --echo # End of 10.3 tests
 --echo #
+
+--echo #
+--echo # MDEV-17869 AddressSanitizer: use-after-poison in Item_change_list::rollback_item_tree_changes
+--echo #
+create table t1 (pk int, v1 varchar(1));
+insert t1 values (1,'v'),(2,'v'),(3,'c');
+create table t2 (pk int, v1 varchar(1));
+insert t2 values (1,'x');
+create table t3 (pk int, i1 int, v1 varchar(1));
+insert t3 values (10,8,9);
+execute immediate 'select straight_join 1 from (t1 join t2 on (t1.v1 = t2.v1))
+where (3, 6) in (select tc.pk,  t3.i1 from (t3 join t1 as tc on (tc.v1 = t3.v1)) having tc.pk > 1 );';
+drop table t1, t2, t3;
+
+--echo #
+--echo # End of 10.4 tests
+--echo #

sql/item.h

@@ -1868,6 +1868,11 @@ class Item: public Value_source,
   }
 
   virtual Item* transform(THD *thd, Item_transformer transformer, uchar *arg);
+  virtual Item* top_level_transform(THD *thd, Item_transformer transformer,
+                                    uchar *arg)
+  {
+    return transform(thd, transformer, arg);
+  }
 
   /*
     This function performs a generic "compilation" of the Item tree.
@@ -1892,6 +1897,11 @@ class Item: public Value_source,
       return ((this->*transformer) (thd, arg_t));
     return 0;
   }
+  virtual Item* top_level_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
+                                  Item_transformer transformer, uchar *arg_t)
+  {
+    return compile(thd, analyzer, arg_p, transformer, arg_t);
+  }
 
    virtual void traverse_cond(Cond_traverser traverser,
                               void *arg, traverse_order order)

sql/item_cmpfunc.cc

@@ -5032,15 +5032,17 @@ bool Item_cond::walk(Item_processor processor, bool walk_subquery, void *arg)
     Item returned as the result of transformation of the root node 
 */
 
-Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg)
+Item *Item_cond::do_transform(THD *thd, Item_transformer transformer, uchar *arg,
+                              bool toplevel)
 {
   DBUG_ASSERT(!thd->stmt_arena->is_stmt_prepare());
 
   List_iterator<Item> li(list);
   Item *item;
   while ((item= li++))
   {
-    Item *new_item= item->transform(thd, transformer, arg);
+    Item *new_item= toplevel ? item->top_level_transform(thd, transformer, arg)
+                             : item->transform(thd, transformer, arg);
     if (!new_item)
       return 0;
 
@@ -5050,7 +5052,9 @@ Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg)
       Otherwise we'll be allocating a lot of unnecessary memory for
       change records at each execution.
     */
-    if (new_item != item)
+    if (toplevel)
+      *li.ref()= new_item;
+    else if (new_item != item)
       thd->change_item_tree(li.ref(), new_item);
   }
   return Item_func::transform(thd, transformer, arg);
@@ -5081,8 +5085,8 @@ Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg)
     Item returned as the result of transformation of the root node 
 */
 
-Item *Item_cond::compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
-                         Item_transformer transformer, uchar *arg_t)
+Item *Item_cond::do_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
+                      Item_transformer transformer, uchar *arg_t, bool toplevel)
 {
   if (!(this->*analyzer)(arg_p))
     return 0;
@@ -5097,7 +5101,11 @@ Item *Item_cond::compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
     */   
     uchar *arg_v= *arg_p;
     Item *new_item= item->compile(thd, analyzer, &arg_v, transformer, arg_t);
-    if (new_item && new_item != item)
+    if (!new_item || new_item == item)
+      continue;
+    if (toplevel)
+      *li.ref()= new_item;
+    else
       thd->change_item_tree(li.ref(), new_item);
   }
   return Item_func::transform(thd, transformer, arg_t);

sql/item_cmpfunc.h

@@ -3018,12 +3018,30 @@ class Item_cond :public Item_bool_func
   bool top_level() { return abort_on_null; }
   void copy_andor_arguments(THD *thd, Item_cond *item);
   bool walk(Item_processor processor, bool walk_subquery, void *arg);
-  Item *transform(THD *thd, Item_transformer transformer, uchar *arg);
+  Item *do_transform(THD *thd, Item_transformer transformer, uchar *arg, bool toplevel);
+  Item *transform(THD *thd, Item_transformer transformer, uchar *arg)
+  {
+    return do_transform(thd, transformer, arg, 0);
+  }
+  Item *top_level_transform(THD *thd, Item_transformer transformer, uchar *arg)
+  {
+    return do_transform(thd, transformer, arg, 1);
+  }
   void traverse_cond(Cond_traverser, void *arg, traverse_order order);
   void neg_arguments(THD *thd);
   Item* propagate_equal_fields(THD *, const Context &, COND_EQUAL *);
+  Item *do_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
+                   Item_transformer transformer, uchar *arg_t, bool toplevel);
   Item *compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
-                Item_transformer transformer, uchar *arg_t);
+                Item_transformer transformer, uchar *arg_t)
+  {
+    return do_compile(thd, analyzer, arg_p, transformer, arg_t, 0);
+  }
+  Item* top_level_compile(THD *thd, Item_analyzer analyzer, uchar **arg_p,
+                          Item_transformer transformer, uchar *arg_t)
+  {
+    return do_compile(thd, analyzer, arg_p, transformer, arg_t, 1);
+  }
   bool eval_not_null_tables(void *opt_arg);
   Item *build_clone(THD *thd);
   bool excl_dep_on_table(table_map tab_map);

sql/sql_lex.cc

@@ -10042,9 +10042,8 @@ st_select_lex::build_pushable_cond_for_having_pushdown(THD *thd, Item *cond)
   */
   if (cond->get_extraction_flag() == FULL_EXTRACTION_FL)
   {
-    Item *result= cond->transform(thd,
-                                  &Item::multiple_equality_transformer,
-                                  (uchar *)this);
+    Item *result= cond->top_level_transform(thd,
+                        &Item::multiple_equality_transformer, (uchar *)this);
     if (!result)
       return true;
     if (result->type() == Item::COND_ITEM &&

sql/sql_select.cc

@@ -28295,11 +28295,11 @@ void JOIN::cache_const_exprs()
     return;
 
   if (conds)
-    conds->compile(thd, &Item::cache_const_expr_analyzer, &analyzer_arg,
+    conds->top_level_compile(thd, &Item::cache_const_expr_analyzer, &analyzer_arg,
                               &Item::cache_const_expr_transformer, &cache_flag);
   cache_flag= FALSE;
   if (having)
-    having->compile(thd, &Item::cache_const_expr_analyzer,
+    having->top_level_compile(thd, &Item::cache_const_expr_analyzer,
                 &analyzer_arg, &Item::cache_const_expr_transformer, &cache_flag);
 
   for (JOIN_TAB *tab= first_depth_first_tab(this); tab;
@@ -28308,7 +28308,7 @@ void JOIN::cache_const_exprs()
     if (*tab->on_expr_ref)
     {
       cache_flag= FALSE;
-      (*tab->on_expr_ref)->compile(thd, &Item::cache_const_expr_analyzer,
+      (*tab->on_expr_ref)->top_level_compile(thd, &Item::cache_const_expr_analyzer,
                 &analyzer_arg, &Item::cache_const_expr_transformer, &cache_flag);
     }
   }
@@ -29365,7 +29365,6 @@ select_handler *SELECT_LEX::find_select_handler(THD *thd)
 }
 
 
-
 /**
   @} (end of group Query_Optimizer)
 */

sql/sql_tvc.cc

@@ -1121,12 +1121,10 @@ bool JOIN::transform_in_predicates_into_in_subq(THD *thd)
   {
     select_lex->parsing_place= IN_WHERE;
     conds=
-      conds->transform(thd,
-		       &Item::in_predicate_to_in_subs_transformer,
-                       (uchar*) 0);
+      conds->top_level_transform(thd,
+                                 &Item::in_predicate_to_in_subs_transformer, 0);
     if (!conds)
       DBUG_RETURN(true);
-    select_lex->prep_where= conds ? conds->copy_andor_structure(thd) : 0;
     select_lex->where= conds;
   }
 
@@ -1141,13 +1139,10 @@ bool JOIN::transform_in_predicates_into_in_subq(THD *thd)
       if (table->on_expr)
       {
         table->on_expr=
-          table->on_expr->transform(thd,
-		                    &Item::in_predicate_to_in_subs_transformer,
-                                    (uchar*) 0);
+          table->on_expr->top_level_transform(thd,
+                              &Item::in_predicate_to_in_subs_transformer, 0);
 	if (!table->on_expr)
 	  DBUG_RETURN(true);
-	table->prep_on_expr= table->on_expr ?
-                             table->on_expr->copy_andor_structure(thd) : 0;
       }
     }
   }