From a7f4882a43d8db061c2241438635fb13fc1c0d37 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 19 Oct 2005 14:54:54 +0200 Subject: [PATCH 1/2] Fixed BUG#13941: replace() string fuction behaves badly inside stored procedure For some functions returning strings (like "replace" and "ifnull" - where val_str() is returning a pointer into one of the parameters) - we ended up with a dangling pointer after the new operator destroyed the reuse item in the eval function. A working, if not very elegant, solution is to simply copy the string in such cases. mysql-test/r/sp.result: New test case for BUG#13941. mysql-test/t/sp.test: New test case for BUG#13941. sql/sp_head.cc: Copy the string when evaluating some string functions (e.g. "replace" and "ifnull") to avoid using a dangling pointer. --- mysql-test/r/sp.result | 28 +++++++++++++++++++++++++++ mysql-test/t/sp.test | 43 ++++++++++++++++++++++++++++++++++++++++++ sql/sp_head.cc | 15 +++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index e95ee4441ce..7fdbeff9a86 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -3504,4 +3504,32 @@ drop procedure bug7049_5| drop procedure bug7049_6| drop function bug7049_1| drop function bug7049_2| +drop function if exists bug13941| +drop procedure if exists bug13941| +create function bug13941(p_input_str text) +returns text +begin +declare p_output_str text; +set p_output_str = p_input_str; +set p_output_str = replace(p_output_str, 'xyzzy', 'plugh'); +set p_output_str = replace(p_output_str, 'test', 'prova'); +set p_output_str = replace(p_output_str, 'this', 'questo'); +set p_output_str = replace(p_output_str, ' a ', 'una '); +set p_output_str = replace(p_output_str, 'is', ''); +return p_output_str; +end| +create procedure bug13941(out sout varchar(128)) +begin +set sout = 'Local'; +set sout = ifnull(sout, 'DEF'); +end| +select bug13941('this is a test')| +bug13941('this is a test') +questo una prova +call bug13941(@a)| +select @a| +@a +Local +drop function bug13941| +drop procedure bug13941| drop table t1,t2; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 4b9987c2803..18607c7f13c 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -4390,6 +4390,49 @@ drop function bug7049_1| drop function bug7049_2| +# +# BUG#13941: replace() string fuction behaves badly inside stored procedure +# (BUG#13914: IFNULL is returning garbage in stored procedure) +# +--disable_warnings +drop function if exists bug13941| +drop procedure if exists bug13941| +--enable_warnings + +create function bug13941(p_input_str text) + returns text +begin + declare p_output_str text; + + set p_output_str = p_input_str; + + set p_output_str = replace(p_output_str, 'xyzzy', 'plugh'); + set p_output_str = replace(p_output_str, 'test', 'prova'); + set p_output_str = replace(p_output_str, 'this', 'questo'); + set p_output_str = replace(p_output_str, ' a ', 'una '); + set p_output_str = replace(p_output_str, 'is', ''); + + return p_output_str; +end| + +create procedure bug13941(out sout varchar(128)) +begin + set sout = 'Local'; + set sout = ifnull(sout, 'DEF'); +end| + +# Note: The bug showed different behaviour in different types of builds, +# giving garbage results in some, and seemingly working in others. +# Running with valgrind (or purify) is the safe way to check that it's +# really working correctly. +select bug13941('this is a test')| +call bug13941(@a)| +select @a| + +drop function bug13941| +drop procedure bug13941| + + # # BUG#NNNN: New bug synopsis # diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 063c388702d..65e6a239242 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -293,6 +293,21 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type, if (it == reuse) DBUG_RETURN(it); + /* + For some functions, 's' is now pointing to an argument of the + function, which might be a local variable that it to be reused. + In this case, new(reuse, &rsize) below will call the destructor + and 's' ends up pointing to freed memory. + A somewhat ugly fix is to simply copy the string to our local one + (which is unused by most functions anyway), but only if 's' is + pointing somewhere else than to 'tmp' or 'it->str_value'. + */ + if (reuse && s != &tmp && s != &it->str_value) + { + tmp.copy(s->c_ptr(), s->length(), it->collation.collation); + s= &tmp; + } + CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize) Item_string(it->collation.collation), use_callers_arena, &backup_arena); From d9b0a62e936007d14a651bfa85d892eb8db93dde Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 21 Oct 2005 13:08:00 +0200 Subject: [PATCH 2/2] Post-review fix. sql/sp_head.cc: Post-review fix; changed string copying method (+ fixed comment typo and indention). --- sql/sp_head.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 65e6a239242..abc66ce0b21 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -295,7 +295,7 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type, /* For some functions, 's' is now pointing to an argument of the - function, which might be a local variable that it to be reused. + function, which might be a local variable that is to be reused. In this case, new(reuse, &rsize) below will call the destructor and 's' ends up pointing to freed memory. A somewhat ugly fix is to simply copy the string to our local one @@ -304,7 +304,8 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type, */ if (reuse && s != &tmp && s != &it->str_value) { - tmp.copy(s->c_ptr(), s->length(), it->collation.collation); + if (tmp.copy((const String)(*s))) + DBUG_RETURN(NULL); s= &tmp; } @@ -338,7 +339,7 @@ sp_eval_func_item(THD *thd, Item **it_addr, enum enum_field_types type, return_null_item: CREATE_ON_CALLERS_ARENA(it= new(reuse, &rsize) Item_null(), - use_callers_arena, &backup_arena); + use_callers_arena, &backup_arena); end: it->rsize= rsize;