Fixed usage of not initialized memory in LIKE ... ESCAPE

This was noticed wben running "mtr --valgrind main.precedence"

The problem was that Item_func_like::escape could be left unitialized
when used with views combined with UNIONS like in:

create or replace view v1 as select 2 LIKE 1 ESCAPE 3 IN (SELECT 0 UNION SELECT 1), 2 LIKE 1 ESCAPE (3 IN (SELECT 0 UNION SELECT 1)), (2 LIKE 1 ESCAPE 3) IN (SELECT 0 UNION SELECT 1);

The above query causes in fix_escape_item()
escape_item->const_during_execution() to be true
and
escape_item->const_item() to be false

in which case 'escape' is never calculated.

The fix is to make the main logic of fix_escape_item() out to a
separate function and call that function once in Item.

Other things:
- Reorganized fields in Item_func_like class to make it more compact

2 files changed, 91 insertions, 65 deletions

diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index bfd415344ef..47e9695e507 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -5613,6 +5613,61 @@ void Item_func_like::print(String *str, enum_query_type query_type)
 }
 
 
+static bool fix_escape_item_now(THD *thd, Item *escape_item, String *tmp_str,
+                               bool escape_used_in_parsing, CHARSET_INFO *cmp_cs,
+                               int *escape)
+{
+  String *escape_str= escape_item->val_str(tmp_str);
+  if (escape_str)
+  {
+    const char *escape_str_ptr= escape_str->ptr();
+    if (escape_used_in_parsing &&
+        ((((thd->variables.sql_mode & MODE_NO_BACKSLASH_ESCAPES) &&
+           escape_str->numchars() != 1) ||
+          escape_str->numchars() > 1)))
+    {
+      my_error(ER_WRONG_ARGUMENTS,MYF(0),"ESCAPE");
+      return TRUE;
+    }
+
+    if (cmp_cs->use_mb())
+    {
+      CHARSET_INFO *cs= escape_str->charset();
+      my_wc_t wc;
+      int rc= cs->mb_wc(&wc,
+                        (const uchar*) escape_str_ptr,
+                        (const uchar*) escape_str_ptr +
+                        escape_str->length());
+      *escape= (int) (rc > 0 ? wc : '\\');
+    }
+    else
+    {
+      /*
+        In the case of 8bit character set, we pass native
+        code instead of Unicode code as "escape" argument.
+        Convert to "cs" if charset of escape differs.
+      */
+      uint32 unused;
+      if (escape_str->needs_conversion(escape_str->length(),
+                                       escape_str->charset(),cmp_cs,&unused))
+      {
+        char ch;
+        uint errors;
+        uint32 cnvlen= copy_and_convert(&ch, 1, cmp_cs, escape_str_ptr,
+                                        escape_str->length(),
+                                        escape_str->charset(), &errors);
+        *escape= cnvlen ? ch : '\\';
+      }
+      else
+        *escape= escape_str_ptr ? *escape_str_ptr : '\\';
+    }
+  }
+  else
+    *escape= '\\';
+  return FALSE;
+}
+
+
 longlong Item_func_like::val_int()
 {
   DBUG_ASSERT(fixed == 1);
@@ -5631,6 +5686,17 @@ longlong Item_func_like::val_int()
   null_value=0;
   if (canDoTurboBM)
     return turboBM_matches(res->ptr(), res->length()) ? !negated : negated;
+  if (unlikely(!escape_item_evaluated))
+  {
+    if (fix_escape_item_now(current_thd, escape_item, &cmp_value1,
+                            escape_used_in_parsing,
+                            cmp_collation.collation, &escape))
+    {
+      null_value= 1;
+      return 0;
+    }
+    escape_item_evaluated= 1;
+  }
   return cmp_collation.collation->wildcmp(
 		    res->ptr(),res->ptr()+res->length(),
 		    res2->ptr(),res2->ptr()+res2->length(),
@@ -5711,58 +5777,13 @@ bool fix_escape_item(THD *thd, Item *escape_item, String *tmp_str,
   if (escape_item->const_item())
   {
     /* If we are on execution stage */
-    String *escape_str= escape_item->val_str(tmp_str);
-    if (escape_str)
-    {
-      const char *escape_str_ptr= escape_str->ptr();
-      if (escape_used_in_parsing && (
-             (((thd->variables.sql_mode & MODE_NO_BACKSLASH_ESCAPES) &&
-                escape_str->numchars() != 1) ||
-               escape_str->numchars() > 1)))
-      {
-        my_error(ER_WRONG_ARGUMENTS,MYF(0),"ESCAPE");
-        return TRUE;
-      }
-
-      if (cmp_cs->use_mb())
-      {
-        CHARSET_INFO *cs= escape_str->charset();
-        my_wc_t wc;
-        int rc= cs->mb_wc(&wc,
-                          (const uchar*) escape_str_ptr,
-                          (const uchar*) escape_str_ptr +
-                          escape_str->length());
-        *escape= (int) (rc > 0 ? wc : '\\');
-      }
-      else
-      {
-        /*
-          In the case of 8bit character set, we pass native
-          code instead of Unicode code as "escape" argument.
-          Convert to "cs" if charset of escape differs.
-        */
-        uint32 unused;
-        if (escape_str->needs_conversion(escape_str->length(),
-                                         escape_str->charset(),cmp_cs,&unused))
-        {
-          char ch;
-          uint errors;
-          uint32 cnvlen= copy_and_convert(&ch, 1, cmp_cs, escape_str_ptr,
-                                          escape_str->length(),
-                                          escape_str->charset(), &errors);
-          *escape= cnvlen ? ch : '\\';
-        }
-        else
-          *escape= escape_str_ptr ? *escape_str_ptr : '\\';
-      }
-    }
-    else
-      *escape= '\\';
+    return fix_escape_item_now(thd, escape_item, tmp_str, escape_used_in_parsing,
+                               cmp_cs, escape);
   }
-
   return FALSE;
 }
 
+
 bool Item_func_like::fix_fields(THD *thd, Item **ref)
 {
   DBUG_ASSERT(fixed == 0);
@@ -5772,8 +5793,10 @@ bool Item_func_like::fix_fields(THD *thd, Item **ref)
                       cmp_collation.collation, &escape))
     return TRUE;
 
+  escape_item_evaluated= 0;
   if (escape_item->const_item())
   {
+    escape_item_evaluated= 1;
     /*
       We could also do boyer-more for non-const items, but as we would have to
       recompute the tables for each row it's not worth it.
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index fa715badfc7..1808884647e 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -2672,14 +2672,24 @@ public:
 
 class Item_func_like :public Item_bool_func2
 {
-  // Turbo Boyer-Moore data
-  bool        canDoTurboBM;	// pattern is '%abcd%' case
   const char* pattern;
-  int         pattern_len;
+  Item *escape_item;
+  DTCollation cmp_collation;
+  String cmp_value1, cmp_value2;
 
+  // Turbo Boyer-Moore data
   // TurboBM buffers, *this is owner
-  int* bmGs; //   good suffix shift table, size is pattern_len + 1
-  int* bmBc; // bad character shift table, size is alphabet_size
+  int *bmGs; //   good suffix shift table, size is pattern_len + 1
+  int *bmBc; // bad character shift table, size is alphabet_size
+  int  pattern_len;
+public:
+  int escape;
+  bool negated;
+private:
+  bool canDoTurboBM;	// pattern is '%abcd%' case
+  bool escape_item_evaluated;
+  bool escape_used_in_parsing;
+  bool use_sampling;
 
   void turboBM_compute_suffixes(int* suff);
   void turboBM_compute_good_suffix_shifts(int* suff);
@@ -2687,13 +2697,6 @@ class Item_func_like :public Item_bool_func2
   bool turboBM_matches(const char* text, int text_len) const;
   enum { alphabet_size = 256 };
 
-  Item *escape_item;
-
-  bool escape_used_in_parsing;
-  bool use_sampling;
-
-  DTCollation cmp_collation;
-  String cmp_value1, cmp_value2;
   bool with_sargable_pattern() const;
 protected:
   SEL_TREE *get_func_mm_tree(RANGE_OPT_PARAM *param,
@@ -2706,13 +2709,13 @@ protected:
                        KEY_PART *key_part,
                        Item_func::Functype type, Item *value);
 public:
-  int escape;
-  bool negated;
 
   Item_func_like(THD *thd, Item *a, Item *b, Item *escape_arg, bool escape_used):
-    Item_bool_func2(thd, a, b), canDoTurboBM(FALSE), pattern(0), pattern_len(0),
-    bmGs(0), bmBc(0), escape_item(escape_arg),
-    escape_used_in_parsing(escape_used), use_sampling(0), negated(0) {}
+    Item_bool_func2(thd, a, b), pattern(0), escape_item(escape_arg),
+      bmGs(0), bmBc(0), pattern_len(0),  negated(0), canDoTurboBM(FALSE),
+      escape_item_evaluated(0), escape_used_in_parsing(escape_used),
+      use_sampling(0)
+  {}
 
   bool get_negated() const { return negated; } // Used by ColumnStore