From 1b15f430d6734bc3a3b1e4f5af6d52855ad7d33d Mon Sep 17 00:00:00 2001
From: unknown <petr@mysql.com>
Date: Thu, 8 Sep 2005 18:25:42 +0200
Subject: [PATCH] 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 | 19 +++++++++++++++++++
 mysql-test/t/sp.test   | 26 ++++++++++++++++++++++++++
 sql/item.cc            | 19 ++++++++++++++++++-
 sql/item.h             |  8 ++++++++
 sql/sp_head.cc         | 15 +++++++++++++--
 5 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result
index 4424f4e6ad4..c3238860b40 100644
--- a/mysql-test/r/sp.result
+++ b/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;
diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test
index d52ebbbbf67..1fff6bf8109 100644
--- a/mysql-test/t/sp.test
+++ b/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
 #
diff --git a/sql/item.cc b/sql/item.cc
index 56b03055968..e7da646ae73 100644
--- a/sql/item.cc
+++ b/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;
 }
 
 
diff --git a/sql/item.h b/sql/item.h
index f128c72413d..381ba98e193 100644
--- a/sql/item.h
+++ b/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).
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 14956138cbf..763d3ef3e0e 100644
--- a/sql/sp_head.cc
+++ b/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