fix for Bug #12849 Stored Procedure: Crash on procedure call with CHAR type 'INOUT' parameter

(recommit with the right Bug#)


mysql-test/r/sp.result:
  result file modified to reflect new test
mysql-test/t/sp.test:
  added test for the bug
sql/item.cc:
  protect Item_splocal value from modification by CONCAT() et al
sql/item.h:
  added a buffer to save Item_splocal string pointer
sql/sp_head.cc:
  don't employ reuse mechanism to save var into itself

mysql-test/r/sp.result

@@ -3193,4 +3193,23 @@ set f1= concat( 'hello', f1 );
 return f1;
 end|
 drop function bug9048|
+drop procedure if exists bug12849_1|
+create procedure bug12849_1(inout x char) select x into x|
+set @var='a'|
+call bug12849_1(@var)|
+select @var|
+@var
+a
+drop procedure bug12849_1|
+drop procedure if exists bug12849_2|
+create procedure bug12849_2(inout foo varchar(15))
+begin
+select concat(foo, foo) INTO foo;
+end|
+set @var='abcd'|
+call bug12849_2(@var)|
+select @var|
+@var
+abcdabcd
+drop procedure bug12849_2|
 drop table t1,t2;

mysql-test/t/sp.test

@@ -4043,6 +4043,32 @@ begin
 end|
 drop function bug9048|
 
+#
+# Bug #12849 Stored Procedure: Crash on procedure call with CHAR type
+# 'INOUT' parameter
+#
+
+--disable_warnings
+drop procedure if exists bug12849_1|
+--enable_warnings
+create procedure bug12849_1(inout x char) select x into x|
+set @var='a'|
+call bug12849_1(@var)|
+select @var|
+drop procedure bug12849_1|
+
+--disable_warnings
+drop procedure if exists bug12849_2|
+--enable_warnings
+create procedure bug12849_2(inout foo varchar(15))
+begin
+select concat(foo, foo) INTO foo;
+end|
+set @var='abcd'|
+call bug12849_2(@var)|
+select @var|
+drop procedure bug12849_2|
+
 #
 # BUG#NNNN: New bug synopsis
 #

sql/item.cc

@@ -818,8 +818,25 @@ String *Item_splocal::val_str(String *sp)
   DBUG_ASSERT(fixed);
   Item *it= this_item();
   String *ret= it->val_str(sp);
+  /*
+    This way we mark returned value of val_str as const,
+    so that various functions (e.g. CONCAT) won't try to
+    modify the value of the Item. Analogous mechanism is
+    implemented for Item_param.
+    Without this trick Item_splocal could be changed as a
+    side-effect of expression computation. Here is an example
+    of what happens without it: suppose x is varchar local
+    variable in a SP with initial value 'ab' Then
+      select concat(x,'c');
+    would change x's value to 'abc', as Item_func_concat::val_str()
+    would use x's internal buffer to compute the result.
+    This is intended behaviour of Item_func_concat. Comments to
+    Item_param class contain some more details on the topic.
+  */
+  str_value_ptr.set(ret->ptr(), ret->length(),
+                    ret->charset());
   null_value= it->null_value;
-  return ret;
+  return &str_value_ptr;
 }
 
 

sql/item.h

@@ -715,9 +715,17 @@ public:
 class Item_splocal : public Item
 {
   uint m_offset;
+
 public:
   LEX_STRING m_name;
 
+  /*
+    Buffer, pointing to the string value of the item. We need it to
+    protect internal buffer from changes. See comment to analogous
+    member in Item_param for more details.
+  */
+  String str_value_ptr;
+
   /* 
     Position of this reference to SP variable in the statement (the
     statement itself is in sp_instr_stmt::m_query).

sql/sp_head.cc

@@ -275,8 +275,19 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type,
     }
     DBUG_PRINT("info",("STRING_RESULT: %*s",
                        s->length(), s->c_ptr_quick()));
-    CHARSET_INFO *itcs= it->collation.collation;
-    CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize) Item_string(itcs),
+    /*
+      Reuse mechanism in sp_eval_func_item() is only employed for assignments
+      to local variables and OUT/INOUT SP parameters repsesented by
+      Item_splocal. Usually we have some expression, which needs
+      to be calculated and stored into the local variable. However in the
+      case if "it" equals to "reuse", there is no "calculation" step. So,
+      no reason to employ reuse mechanism to save variable into itself.
+    */
+    if (it == reuse)
+      DBUG_RETURN(it);
+
+    CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize)
+                            Item_string(it->collation.collation),
                             use_callers_arena, &backup_arena);
     /*
       We have to use special constructor and allocate string