MDEV-17869 AddressSanitizer: use-after-poison in Item_change_list::rollback_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

8 files changed, 86 insertions, 24 deletions

diff --git a/mysql-test/main/prepare.result b/mysql-test/main/prepare.result
index cfe6603dbbe..7c730bff0c5 100644
--- a/mysql-test/main/prepare.result
+++ b/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
+#
diff --git a/mysql-test/main/prepare.test b/mysql-test/main/prepare.test
index 4d1573eb0c8..bf37f6dc8d1 100644
--- a/mysql-test/main/prepare.test
+++ b/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 #
diff --git a/sql/item.h b/sql/item.h
index 9389250d6ec..89d47f0026b 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -1868,6 +1868,11 @@ public:
   }
 
   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 @@ public:
       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)
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index b6509fa8b25..278e11f4002 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -5032,7 +5032,8 @@ 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());
 
@@ -5040,7 +5041,8 @@ Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg)
   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);
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 05b1f95ff36..01834fe06d7 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -3018,12 +3018,30 @@ public:
   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);
diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
index f31a128832b..ac570be78aa 100644
--- a/sql/sql_lex.cc
+++ b/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 &&
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index d9922fddb8f..eb54484fa51 100644
--- a/sql/sql_select.cc
+++ b/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)
 */
diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc
index a25f6522bd9..d3d20ef7a9a 100644
--- a/sql/sql_tvc.cc
+++ b/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;
       }
     }
   }