Re: [Maria-developers] MDEV-14603 signal 11 with short stacktrace

27 Dec 2017

Hi, Alexander!

On Dec 26, Alexander Barkov wrote:
...
Hi Sergei,
can you please review a patch for MDEV-14603?
I agree with the fix.

But I don't like that there are many things to backup/restore
(Statement, arena, free_list, and now change_list), they're all
saved/restored in different places - it's easy to miss something when
making changes.

Would it be possible to move all that saving/restoring into dedicated
helpers and use them in all three places (prepare, execute, execute
immediate)?
...

diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result
index 6857ebc..a56920d 100644
--- a/mysql-test/r/ps.result
+++ b/mysql-test/r/ps.result
@@ -5074,3 +5074,45 @@ t1	CREATE TABLE `t1` (
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1
 DROP TABLE t1;
 DROP PROCEDURE p1;
+#
+# MDEV-14603 signal 11 with short stacktrace
+#
+SET NAMES utf8;
+CREATE TABLE t1(i INT);
+CREATE PROCEDURE p1(tn VARCHAR(32))
+EXECUTE IMMEDIATE CONCAT('ANALYZE TABLE ',tn);
+CALL p1('t1');
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	Table is already up to date
+DROP PROCEDURE p1;
+DROP TABLE t1;
+SET NAMES utf8;
+CREATE PROCEDURE p1()
+EXECUTE IMMEDIATE CONCAT('SELECT ',CONVERT(RAND() USING latin1));
+CALL p1();
+DROP PROCEDURE p1;
+SET NAMES utf8;
+CREATE PROCEDURE p1()
+BEGIN
+PREPARE stmt FROM CONCAT('SELECT ',CONVERT(RAND() USING latin1));
+EXECUTE stmt;
+DEALLOCATE PREPARE stmt;
+END;
+$$
+CALL p1();
+DROP PROCEDURE p1;
+SET NAMES utf8;
+CREATE PROCEDURE p1(a VARCHAR(10) CHARACTER SET utf8)
+EXECUTE IMMEDIATE 'SELECT ?' USING CONCAT(a, CONVERT(RAND() USING latin1));
+CALL p1('x');
+DROP PROCEDURE p1;
+SET NAMES utf8;
+CREATE PROCEDURE p1(a VARCHAR(10) CHARACTER SET utf8)
+BEGIN
+PREPARE stmt FROM 'SELECT ?';
+EXECUTE stmt USING CONCAT(a, CONVERT(RAND() USING latin1));
+DEALLOCATE PREPARE stmt;
+END;
+$$
+CALL p1('x');
+DROP PROCEDURE p1;
diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test
index a7683b5..640f479 100644
--- a/mysql-test/t/ps.test
+++ b/mysql-test/t/ps.test
@@ -4529,3 +4529,62 @@ CREATE TABLE t1 AS SELECT @a AS a, @b AS b;
 SHOW CREATE TABLE t1;
 DROP TABLE t1;
 DROP PROCEDURE p1;
+
+
+--echo #
+--echo # MDEV-14603 signal 11 with short stacktrace
+--echo #
+
+SET NAMES utf8;
+CREATE TABLE t1(i INT);
+CREATE PROCEDURE p1(tn VARCHAR(32))
+  EXECUTE IMMEDIATE CONCAT('ANALYZE TABLE ',tn);
+CALL p1('t1');
+DROP PROCEDURE p1;
+DROP TABLE t1;
+
+SET NAMES utf8;
+CREATE PROCEDURE p1()
+  EXECUTE IMMEDIATE CONCAT('SELECT ',CONVERT(RAND() USING latin1));
+--disable_result_log
+CALL p1();
+--enable_result_log
+DROP PROCEDURE p1;
+
+SET NAMES utf8;
+DELIMITER $$;
+CREATE PROCEDURE p1()
+BEGIN
+  PREPARE stmt FROM CONCAT('SELECT ',CONVERT(RAND() USING latin1));
+  EXECUTE stmt;
+  DEALLOCATE PREPARE stmt;
+END;
+$$
+DELIMITER ;$$
+--disable_result_log
+CALL p1();
+--enable_result_log
+DROP PROCEDURE p1;
+
+SET NAMES utf8;
+CREATE PROCEDURE p1(a VARCHAR(10) CHARACTER SET utf8)
+  EXECUTE IMMEDIATE 'SELECT ?' USING CONCAT(a, CONVERT(RAND() USING latin1));
+--disable_result_log
+CALL p1('x');
+--enable_result_log
+DROP PROCEDURE p1;
+
+SET NAMES utf8;
+DELIMITER $$;
+CREATE PROCEDURE p1(a VARCHAR(10) CHARACTER SET utf8)
+BEGIN
+  PREPARE stmt FROM 'SELECT ?';
+  EXECUTE stmt USING CONCAT(a, CONVERT(RAND() USING latin1));
+  DEALLOCATE PREPARE stmt;
+END;
+$$
+DELIMITER ;$$
+--disable_result_log
+CALL p1('x');
+--enable_result_log
+DROP PROCEDURE p1;
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index 19c714c..d63c42c 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -2830,6 +2830,25 @@ void mysql_sql_stmt_prepare(THD *thd)
     DBUG_VOID_RETURN;
   }
+  /*
+    Make sure we call Prepared_statement::prepare() with an empty
+    THD::change_list. It can be non-empty as the above
+    LEX::get_dynamic_sql_string() calls fix_fields() for the Item
+    containing the PS source, e.g. on character set conversion:
+
+    SET NAMES utf8;
+    DELIMITER $$
+    CREATE PROCEDURE p1()
+    BEGIN
+      PREPARE stmt FROM CONCAT('SELECT ',CONVERT(RAND() USING latin1));
+      EXECUTE stmt;
+    END;
+    $$
+    DELIMITER ;
+    CALL p1();
+  */
+  Item_change_list save_change_list;
+  thd->change_list.move_elements_to(&save_change_list);
   if (stmt->prepare(query.str, (uint) query.length))
   {
     /* Statement map deletes the statement on erase */
@@ -2840,6 +2859,7 @@ void mysql_sql_stmt_prepare(THD *thd)
     SESSION_TRACKER_CHANGED(thd, SESSION_STATE_CHANGE_TRACKER, NULL);
     my_ok(thd, 0L, 0L, "Statement prepared");
   }
+  save_change_list.move_elements_to(&thd->change_list);
DBUG_VOID_RETURN;
 }
@@ -2871,7 +2891,30 @@ void mysql_sql_stmt_execute_immediate(THD *thd)
   // See comments on thd->free_list in mysql_sql_stmt_execute()
   Item *free_list_backup= thd->free_list;
   thd->free_list= NULL;
+  /*
+    Make sure that we're call Prepared_statement::execute_immediate()
+    with an empty THD::change_list. It can be non empty as the above
+    LEX::prepared_stmt_params_fix_fields() and LEX::get_dynamic_str_string()
+    call fix_fields() for the PS source and PS parameter Items and
+    can do Item tree changes, e.g. on character set conversion:
+
+    - Example #1: Item tree changes in get_dynamic_str_string()
+    SET NAMES utf8;
+    CREATE PROCEDURE p1()
+      EXECUTE IMMEDIATE CONCAT('SELECT ',CONVERT(RAND() USING latin1));
+    CALL p1();
+
+    - Example #2: Item tree changes in prepared_stmt_param_fix_fields():
+    SET NAMES utf8;
+    CREATE PROCEDURE p1(a VARCHAR(10) CHARACTER SET utf8)
+      EXECUTE IMMEDIATE 'SELECT ?' USING CONCAT(a, CONVERT(RAND() USING latin1));
+    CALL p1('x');
+  */
+  Item_change_list save_change_list;
+  thd->change_list.move_elements_to(&save_change_list);
   (void) stmt->execute_immediate(query.str, (uint) query.length);
+  thd->rollback_item_tree_changes();
+  save_change_list.move_elements_to(&thd->change_list);
   thd->free_items();
   thd->free_list= free_list_backup;
@@ -3269,7 +3312,29 @@ void mysql_sql_stmt_execute(THD *thd)
   */
   Item *free_list_backup= thd->free_list;
   thd->free_list= NULL; // Hide the external (e.g. "SET STATEMENT") Items
+  /*
+    Make sure we call Prepared_statement::execute_loop() with an empty
+    THD::change_list. It can be non-empty because the above
+    LEX::prepared_stmt_params_fix_fields() calls fix_fields() for
+    the PS parameter Items and can do some Item tree changes,
+    e.g. on character set conversion:
+
+    SET NAMES utf8;
+    DELIMITER $$
+    CREATE PROCEDURE p1(a VARCHAR(10) CHARACTER SET utf8)
+    BEGIN
+      PREPARE stmt FROM 'SELECT ?';
+      EXECUTE stmt USING CONCAT(a, CONVERT(RAND() USING latin1));
+    END;
+    $$
+    DELIMITER ;
+    CALL p1('x');
+  */
+  Item_change_list save_change_list;
+  thd->change_list.move_elements_to(&save_change_list);
   (void) stmt->execute_loop(&expanded_query, FALSE, NULL, NULL);
+  thd->rollback_item_tree_changes();
+  save_change_list.move_elements_to(&thd->change_list);
   thd->free_items();    // Free items created by execute_loop()
   /*
     Now restore the "external" (e.g. "SET STATEMENT") Item list.
Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org

    

Re: [Maria-developers] MDEV-14603 signal 11 with short stacktrace

Sergei Golubchik