BUG#36314928 - Assertion failure !s.uses_buffer_owned_by(this) in sql_string.cc

BUG#36389091 - `!s.uses_buffer_owned_by(this)' in String::append at sql-common/sql_string.cc
BUG#36397496 - HLL query gives Assertion `!str || str != m_ptr' failed

Problem
=======
In udf_handler::get_and_convert_string, we use udf_handler::buffers
to hold pointers returned by val_str(...) calls. This causes an
issue with certain functions (such as Item_func_concat), which
expect the argument passed in val_str(...) to be non-overlapping
to their local buffers.
Sequence of steps leading to the issue:
1. First call of udf_handler::get_and_convert_string: We call
Item_func_concat::val_str with buffers[0].m_ptr: 0x0.
2. Item_func_concat::val_str returns the address of its local
&tmp_value as res. res->m_ptr: ADDR1
3. We copy *res to buffers[0]. buffers[0].m_ptr: ADDR1
4. Second call of udf_handler::get_and_convert_string: We call
Item_func_concat::val_str with buffers[0].m_ptr: ADDR1
5. Item_func_concat::val_str sees that buffers[0].m_ptr and
its local tmp_value.m_ptr are the same ADDR1 -> assert fails.

Solution
========
- Introduce udf_handler::arg_buffers to hold data returned by
val_str(...) in get_and_convert_string. The current way of
modifying *res itself is risky, since it could be changing
Item::str_value object, which has other implications
(BUG#36297142)
- Instead of using udf_handler::buffers to hold pointers returned,
do a deep copy to udf_handler::arg_buffers.

Change-Id: I50c2a36cd609dc28e70cabe6c730b15bac6ef8d6

Author: kaankara

sql/item_func.cc

@@ -4593,6 +4593,7 @@ void udf_handler::clean_buffers() {
   if (buffers == nullptr) return;
   for (uint i = 0; i < f_args.arg_count; i++) {
     buffers[i].mem_free();
+    arg_buffers[i].mem_free();
   }
 }
 
@@ -4703,6 +4704,8 @@ bool udf_handler::fix_fields(THD *thd, Item_result_field *func, uint arg_count,
     // if (!(buffers = new String[arg_count]) ||
     if (!(buffers = pointer_cast<String *>(
               (*THR_MALLOC)->Alloc(sizeof(String) * arg_count))) ||
+        !(arg_buffers = pointer_cast<String *>(
+              (*THR_MALLOC)->Alloc(sizeof(String) * arg_count))) ||
         !(f_args.args =
               (char **)(*THR_MALLOC)->Alloc(arg_count * sizeof(char *))) ||
         !(f_args.lengths =
@@ -4723,6 +4726,7 @@ bool udf_handler::fix_fields(THD *thd, Item_result_field *func, uint arg_count,
   }
   for (uint i = 0; i < arg_count; i++) {
     (void)::new (buffers + i) String;
+    (void)::new (arg_buffers + i) String;
     m_args_extension.charset_info[i] = nullptr;
   }
 
@@ -5044,29 +5048,17 @@ bool udf_handler::get_and_convert_string(uint index) {
   String *res = args[index]->val_str(&buffers[index]);
 
   if (!args[index]->null_value) {
-    auto check_charset = m_args_extension.charset_info[index];
-    if (check_charset != nullptr && res->charset() != check_charset) {
-      /* m_args_extension.charset_info[index] is a legitimate charset */
-      String temp;
-      uint dummy;
-      if (temp.copy(res->ptr(), res->length(), res->charset(),
-                    m_args_extension.charset_info[index], &dummy)) {
-        return true;
-      }
-      *res = std::move(temp);
-    } else if (res != &buffers[index]) {
-      /* The res returned above is &Item::str_value
-       * We may call c_ptr_safe() below, reallocating the buffer of
-       * Item::str_value If we set the str_value m_ptr directly somewhere, the
-       * allocated buffer at c_ptr_safe() will be freed, before the UDF is able
-       * to use it. So, instead of changing Item::str_value, copy its contents
-       * to udf_handler::buffers and use that instead.
-       */
-      buffers[index] = *res;
-      res = &buffers[index];
-    }
-    f_args.args[index] = res->c_ptr_safe();
-    f_args.lengths[index] = res->length();
+    uint errors = 0;
+    if (arg_buffers[index].copy(res->ptr(), res->length(), res->charset(),
+                                m_args_extension.charset_info[index],
+                                &errors)) {
+      return true;
+    }
+    if (errors) {
+      return true;
+    }
+    f_args.args[index] = arg_buffers[index].c_ptr_safe();
+    f_args.lengths[index] = arg_buffers[index].length();
   } else {
     f_args.lengths[index] = 0;
   }

sql/sql_udf.h

@@ -84,6 +84,7 @@ class udf_handler {
  protected:
   udf_func *u_d;
   String *buffers{nullptr};
+  String *arg_buffers{nullptr};
   UDF_ARGS f_args;
   UDF_INIT initid;
   char *num_buffer{nullptr};