maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #11074
Re: MDEV-14603 signal 11 with short stacktrace
Hi Sergei,
On 01/20/2018 10:35 PM, Sergei Golubchik wrote:
Hi, Alexander!
Thanks! Looks more robust, indeed.
Isn't there a need to save/restore the arena?
You call thd->free_items(); but unless mem_root is reset this won't free
the memory, so I'd expect thd->free_list and thd->mem_root to be always
saved/restored in sync. That is, as an arena.
As agreed on slack, let's return to the first version of the patch.
I only added a helper class Item_change_list_savepoint,
to avoid duplicate code.
Thanks!
On Jan 18, Alexander Barkov wrote:
On 12/27/2017 08:33 PM, Sergei Golubchik wrote:
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)?
Thanks for a good idea.
Done. Please find attached.
Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx
diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result
index 1958cafca1e..96d4b3d2f70 100644
--- a/mysql-test/r/ps.result
+++ b/mysql-test/r/ps.result
@@ -5121,3 +5121,45 @@ END;
$$
ERROR 22007: Incorrect datetime value: '10' for column 'a' at row 1
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 327a94cdace..565f831efdd 100644
--- a/mysql-test/t/ps.test
+++ b/mysql-test/t/ps.test
@@ -4578,3 +4578,60 @@ END;
$$
DELIMITER ;$$
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');
+DROP PROCEDURE p1;
diff --git a/sql/sql_class.h b/sql/sql_class.h
index bcd43cd62cd..5a409ec8268 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -1296,6 +1296,25 @@ class Item_change_list
};
+class Item_change_list_savepoint: public Item_change_list
+{
+public:
+ Item_change_list_savepoint(Item_change_list *list)
+ {
+ list->move_elements_to(this);
+ }
+ void rollback(Item_change_list *list)
+ {
+ list->rollback_item_tree_changes();
+ move_elements_to(list);
+ }
+ ~Item_change_list_savepoint()
+ {
+ DBUG_ASSERT(is_empty());
+ }
+};
+
+
/**
Type of locked tables mode.
See comment for THD::locked_tables_mode for complete description.
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index 4e847fb9ff3..16c386e5e8f 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -2823,6 +2823,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 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_savepoint change_list_savepoint(thd);
+
if (stmt->prepare(query.str, (uint) query.length))
{
/* Statement map deletes the statement on erase */
@@ -2833,6 +2852,7 @@ void mysql_sql_stmt_prepare(THD *thd)
SESSION_TRACKER_CHANGED(thd, SESSION_STATE_CHANGE_TRACKER, NULL);
my_ok(thd, 0L, 0L, "Statement prepared");
}
+ change_list_savepoint.rollback(thd);
DBUG_VOID_RETURN;
}
@@ -2864,7 +2884,28 @@ 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 we 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_savepoint change_list_savepoint(thd);
(void) stmt->execute_immediate(query.str, (uint) query.length);
+ change_list_savepoint.rollback(thd);
thd->free_items();
thd->free_list= free_list_backup;
@@ -3262,7 +3303,27 @@ 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_savepoint change_list_savepoint(thd);
(void) stmt->execute_loop(&expanded_query, FALSE, NULL, NULL);
+ change_list_savepoint.rollback(thd);
thd->free_items(); // Free items created by execute_loop()
/*
Now restore the "external" (e.g. "SET STATEMENT") Item list.
Follow ups
References
