Bug #47412: Valgrind warnings / user can read uninitalized memory using

SP variables

A function call may end without throwing an error or without setting 
the return value. This can happen when e.g. an error occurs while 
calculating the return value.

Fixed by setting the value to NULL when error occurs during evaluation
of an expression.

3 files changed, 125 insertions, 14 deletions

diff --git a/mysql-test/r/sp-bugs.result b/mysql-test/r/sp-bugs.result
new file mode 100644
index 00000000000..14c5311bbe5
--- /dev/null
+++ b/mysql-test/r/sp-bugs.result
@@ -0,0 +1,47 @@
+#
+# Bug #47412: Valgrind warnings / user can read uninitalized memory
+# using SP variables
+#
+CREATE SCHEMA testdb;
+USE testdb;
+CREATE FUNCTION f2 () RETURNS INTEGER
+BEGIN
+DECLARE CONTINUE HANDLER FOR SQLSTATE '42000' SET @aux = 1;
+RETURN f_not_exists () ;
+END|
+CREATE PROCEDURE p3 ( arg1 VARCHAR(32) )
+BEGIN
+CALL p_not_exists ( );
+END|
+# should not return valgrind warnings
+CALL p3 ( f2 () );
+ERROR 42000: PROCEDURE testdb.p_not_exists does not exist
+DROP SCHEMA testdb;
+CREATE SCHEMA testdb;
+USE testdb;
+CREATE FUNCTION f2 () RETURNS INTEGER
+BEGIN
+DECLARE CONTINUE HANDLER FOR SQLSTATE '42000' SET @aux = 1;
+RETURN f_not_exists () ;
+END|
+CREATE PROCEDURE p3 ( arg2 INTEGER )
+BEGIN
+CALL p_not_exists ( );
+END|
+# should not return valgrind warnings
+CALL p3 ( f2 () );
+ERROR 42000: PROCEDURE testdb.p_not_exists does not exist
+DROP SCHEMA testdb;
+CREATE SCHEMA testdb;
+USE testdb;
+CREATE FUNCTION f2 () RETURNS INTEGER
+BEGIN
+DECLARE CONTINUE HANDLER FOR SQLSTATE '42000' SET @aux = 1;
+RETURN f_not_exists () ;
+END|
+# should not return valgrind warnings
+SELECT f2 ();
+f2 ()
+NULL
+DROP SCHEMA testdb;
+End of 5.1 tests
diff --git a/mysql-test/t/sp-bugs.test b/mysql-test/t/sp-bugs.test
new file mode 100644
index 00000000000..7b94e65a5e9
--- /dev/null
+++ b/mysql-test/t/sp-bugs.test
@@ -0,0 +1,61 @@
+# Test file for stored procedure bugfixes
+
+--echo #
+--echo # Bug #47412: Valgrind warnings / user can read uninitalized memory
+--echo # using SP variables
+--echo #
+
+CREATE SCHEMA testdb;
+USE testdb;
+DELIMITER |;
+CREATE FUNCTION f2 () RETURNS INTEGER
+BEGIN
+   DECLARE CONTINUE HANDLER FOR SQLSTATE '42000' SET @aux = 1;
+   RETURN f_not_exists () ;
+END|
+CREATE PROCEDURE p3 ( arg1 VARCHAR(32) )
+BEGIN
+   CALL p_not_exists ( );
+END|
+DELIMITER ;|
+--echo # should not return valgrind warnings
+--error ER_SP_DOES_NOT_EXIST
+CALL p3 ( f2 () );
+
+DROP SCHEMA testdb;
+
+CREATE SCHEMA testdb;
+USE testdb;
+DELIMITER |;
+CREATE FUNCTION f2 () RETURNS INTEGER
+BEGIN
+   DECLARE CONTINUE HANDLER FOR SQLSTATE '42000' SET @aux = 1;
+   RETURN f_not_exists () ;
+END|
+CREATE PROCEDURE p3 ( arg2 INTEGER )
+BEGIN
+   CALL p_not_exists ( );
+END|
+DELIMITER ;|
+--echo # should not return valgrind warnings
+--error ER_SP_DOES_NOT_EXIST
+CALL p3 ( f2 () );
+
+DROP SCHEMA testdb;
+
+CREATE SCHEMA testdb;
+USE testdb;
+DELIMITER |;
+CREATE FUNCTION f2 () RETURNS INTEGER
+BEGIN
+   DECLARE CONTINUE HANDLER FOR SQLSTATE '42000' SET @aux = 1;
+   RETURN f_not_exists () ;
+END|
+DELIMITER ;|
+--echo # should not return valgrind warnings
+SELECT f2 ();
+
+DROP SCHEMA testdb;
+
+
+--echo End of 5.1 tests
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 78b3df525f5..f0c858cc50a 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -334,16 +334,18 @@ bool
 sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr)
 {
   Item *expr_item;
+  enum_check_fields save_count_cuted_fields= thd->count_cuted_fields;
+  bool save_abort_on_warning= thd->abort_on_warning;
+  bool save_stmt_modified_non_trans_table= 
+    thd->transaction.stmt.modified_non_trans_table;
 
   DBUG_ENTER("sp_eval_expr");
 
   if (!*expr_item_ptr)
-    DBUG_RETURN(TRUE);
+    goto error;
 
   if (!(expr_item= sp_prepare_func_item(thd, expr_item_ptr)))
-    DBUG_RETURN(TRUE);
-
-  bool err_status= FALSE;
+    goto error;
 
   /*
     Set THD flags to emit warnings/errors in case of overflow/type errors
@@ -352,10 +354,6 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr)
     Save original values and restore them after save.
   */
   
-  enum_check_fields save_count_cuted_fields= thd->count_cuted_fields;
-  bool save_abort_on_warning= thd->abort_on_warning;
-  bool save_stmt_modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table;
-
   thd->count_cuted_fields= CHECK_FIELD_ERROR_FOR_NULL;
   thd->abort_on_warning=
     thd->variables.sql_mode &
@@ -370,13 +368,18 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr)
   thd->abort_on_warning= save_abort_on_warning;
   thd->transaction.stmt.modified_non_trans_table= save_stmt_modified_non_trans_table;
 
-  if (thd->is_error())
-  {
-    /* Return error status if something went wrong. */
-    err_status= TRUE;
-  }
+  if (!thd->is_error())
+    DBUG_RETURN(FALSE);
 
-  DBUG_RETURN(err_status);
+error:
+  /*
+    In case of error during evaluation, leave the result field set to NULL.
+    Sic: we can't do it in the beginning of the function because the 
+    result field might be needed for its own re-evaluation, e.g. case of 
+    set x = x + 1;
+  */
+  result_field->set_null();
+  DBUG_RETURN (TRUE);
 }