MDEV-26115: Crash when calling stored function in FOR loop argument

On handling SP statement `FOR IN lower_bound..func() DO` the instruction
sp_instr_set is allocated on sp_head's memory root, whereas an instance
of the class Item_func_sp pointed by the data member
   sp_instr_set::sp_result_field
is allocated on runtime memory root. In result, on finishing the first
execution of a stored routine the memory allocated for the instance of
the class Item_func_sp is released whereas the pointer
  sp_instr_set::sp_result_field
still references the deleted memory. Next time the same stored routine
is run dereferencing deallocated memory results in abnormal server
termination.

To fix the issue, allocate an instance of the class Item_func_sp on
sp_head memory root. Do this allocation only once, meaning the
  Item_func_sp::cleanup
doesn't do deletion an instance of the class Item_func_sp and
nullifying the data member sp_instr_set::sp_result_field.

Author: dmitryshulga

mysql-test/main/sp-bugs2.result

@@ -59,4 +59,39 @@ call p2('2');
 drop table t1;
 drop procedure p1;
 drop procedure p2;
+#
+# MDEV-26115: Crash when calling stored function in FOR loop argument
+#
+CREATE OR REPLACE FUNCTION cnt()
+RETURNS INTEGER NO SQL
+BEGIN
+RETURN 3;
+END;
+$
+CREATE OR REPLACE PROCEDURE p1()
+NO SQL
+BEGIN
+DECLARE i INTEGER;
+FOR i IN 1..cnt() DO
+SELECT 1;
+END FOR;
+END;
+$
+CALL p1();
+1
+1
+1
+1
+1
+1
+CALL p1();
+1
+1
+1
+1
+1
+1
+# Clean up
+DROP FUNCTION cnt;
+DROP PROCEDURE p1;
 # End of 10.11 tests

mysql-test/main/sp-bugs2.test

@@ -67,4 +67,34 @@ drop table t1;
 drop procedure p1;
 drop procedure p2;
 
+--echo #
+--echo # MDEV-26115: Crash when calling stored function in FOR loop argument
+--echo #
+--delimiter $
+CREATE OR REPLACE FUNCTION cnt()
+RETURNS INTEGER NO SQL
+BEGIN
+   RETURN 3;
+END;
+$
+
+CREATE OR REPLACE PROCEDURE p1()
+NO SQL
+BEGIN
+   DECLARE i INTEGER;
+   FOR i IN 1..cnt() DO
+      SELECT 1;
+   END FOR;
+END;
+$
+
+--delimiter ;
+
+CALL p1();
+CALL p1();
+
+--echo # Clean up
+DROP FUNCTION cnt;
+DROP PROCEDURE p1;
+
 --echo # End of 10.11 tests

sql/item.cc

@@ -2898,8 +2898,6 @@ Item_sp::func_name_cstring(THD *thd, bool is_package_function) const
 void
 Item_sp::cleanup()
 {
-  delete sp_result_field;
-  sp_result_field= NULL;
   m_sp= NULL;
   delete func_ctx;
   func_ctx= NULL;
@@ -3071,7 +3069,6 @@ Item_sp::init_result_field(THD *thd, uint max_length, uint maybe_null,
   DBUG_ENTER("Item_sp::init_result_field");
 
   DBUG_ASSERT(m_sp != NULL);
-  DBUG_ASSERT(sp_result_field == NULL);
 
   /*
      A Field needs to be attached to a Table.
@@ -3085,23 +3082,26 @@ Item_sp::init_result_field(THD *thd, uint max_length, uint maybe_null,
   dummy_table->s->table_name= empty_clex_str;
   dummy_table->maybe_null= maybe_null;
 
-  if (!(sp_result_field= m_sp->create_result_field(max_length, name,
-                                                   dummy_table)))
-   DBUG_RETURN(TRUE);
-
-  if (sp_result_field->pack_length() > sizeof(result_buf))
+  if (!sp_result_field)
   {
-    void *tmp;
-    if (!(tmp= thd->alloc(sp_result_field->pack_length())))
-      DBUG_RETURN(TRUE);
-    sp_result_field->move_field((uchar*) tmp);
-  }
-  else
-    sp_result_field->move_field(result_buf);
+    sp_result_field= m_sp->create_result_field(max_length, name,
+                                               dummy_table);
+    if (!sp_result_field)
+      DBUG_RETURN(true);
 
-  sp_result_field->null_ptr= (uchar *) null_value;
-  sp_result_field->null_bit= 1;
+    if (sp_result_field->pack_length() > sizeof(result_buf))
+    {
+      void *tmp;
+      if (!(tmp= thd->alloc(sp_result_field->pack_length())))
+        DBUG_RETURN(TRUE);
+      sp_result_field->move_field((uchar*) tmp);
+    }
+    else
+      sp_result_field->move_field(result_buf);
 
+    sp_result_field->null_ptr= (uchar *) null_value;
+    sp_result_field->null_bit= 1;
+  }
   DBUG_RETURN(FALSE);
 }
 

sql/item.h

@@ -5815,6 +5815,11 @@ class Item_sp
   Field *sp_result_field;
   Item_sp(THD *thd, Name_resolution_context *context_arg, sp_name *name_arg);
   Item_sp(THD *thd, Item_sp *item);
+  virtual ~Item_sp()
+  {
+    delete sp_result_field;
+    sp_result_field= NULL;
+  }
   LEX_CSTRING func_name_cstring(THD *thd, bool is_package_function) const;
   void cleanup();
   bool sp_check_access(THD *thd);

sql/item_func.cc

@@ -6837,12 +6837,29 @@ Item_func_sp::fix_fields(THD *thd, Item **ref)
     DBUG_RETURN(TRUE);
   }
 
+  Query_arena *arena, backup;
+  /*
+    Allocation an instance of Item_func_sp used for initialization of
+    sp_result_field taken place inside the method init_result_field() is done
+    on sp_head's mem_root since Item_sp also allocated on this memory root.
+
+    Switching to SP/PS memory root is done explicitly before calling the method
+    init_result_field() instead doing that inside init_result_field()
+    since for the case when rollup aggregate function is handled
+    (@see Item_sum_sp::copy_or_same, @see JOIN::rollup_make_fields)
+    the runtime arena used for operations, so switching to SP/PS arena for this
+    case would result in assertion failure on second execution of the same
+    prepared statement because the memory root be already marked as read only.
+  */
+  arena= thd->activate_stmt_arena_if_needed(&backup);
   /*
     We must call init_result_field before Item_func::fix_fields()
     to make m_sp and result_field members available to fix_length_and_dec(),
     which is called from Item_func::fix_fields().
   */
   res= init_result_field(thd, max_length, maybe_null(), &null_value, &name);
+  if (arena)
+    thd->restore_active_arena(arena, &backup);
 
   if (res)
     DBUG_RETURN(TRUE);

sql/item_sum.cc

@@ -1365,8 +1365,16 @@ Item_sum_sp::fix_fields(THD *thd, Item **ref)
     return TRUE;
   }
 
-  if (init_result_field(thd, max_length, maybe_null(), &null_value, &name))
-    return TRUE;
+  Query_arena *arena, backup;
+  arena= thd->activate_stmt_arena_if_needed(&backup);
+
+  bool ret= init_result_field(thd, max_length, maybe_null(),
+                              &null_value, &name);
+  if (arena)
+    thd->restore_active_arena(arena, &backup);
+
+  if(ret)
+    return true;
 
   for (uint i= 0 ; i < arg_count ; i++)
   {