From e176066a9e02cf2bdedf9e79fb150029dde430f6 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Fri, 4 Apr 2025 11:16:31 +0700 Subject: [PATCH] MDEV-36462: Crash on `DECLARE spvar1 ROW TYPE OF cursor1` after a table recreation After a cursor's statement is re-parsed by the reason of metadata changes for tables the statement depends on, following memory allocations taken place on cursor execution is performed on a memory root already marked as read only despite the fact that a new memory root has been allocated for this goal. To fix the issue, bind the cursor lex with a new memory root created for re-parsing cursor's statement, clean up items stored on cursor's free list and nullify the data member sp_lex_cursor::free_list to avoid dangling pointer problem. --- mysql-test/main/sp-cursor.result | 63 ++++++++++++++++++++++++++++++++ mysql-test/main/sp-cursor.test | 56 ++++++++++++++++++++++++++++ sql/sp_instr.cc | 29 +++++++++++++-- sql/sp_instr.h | 3 +- 4 files changed, 147 insertions(+), 4 deletions(-) diff --git a/mysql-test/main/sp-cursor.result b/mysql-test/main/sp-cursor.result index 7afd4c33aef..9cc2c3ea1a8 100644 --- a/mysql-test/main/sp-cursor.result +++ b/mysql-test/main/sp-cursor.result @@ -923,4 +923,67 @@ SELECT pkg.f1(); pkg.f1() 1 2 DROP PACKAGE pkg; +# +# MDEV-36462: Crash on `DECLARE spvar1 ROW TYPE OF cursor1` after a table recreation +# +CREATE PROCEDURE p1() +BEGIN +DECLARE c CURSOR FOR SELECT a FROM t1; +BEGIN +DECLARE va ROW TYPE OF c; -- the crash happens here +END; +END; +/ +CREATE PROCEDURE p2() +BEGIN +FOR i IN 1..10 DO -- usually it crashes on the third iteration, but not always +SELECT i; +CREATE OR REPLACE TABLE t1 (a INT); +CALL p1; +CALL p1; +END FOR; +END; +/ +CALL p2; +i +1 +i +2 +i +3 +i +4 +i +5 +i +6 +i +7 +i +8 +i +9 +i +10 +# Clean up +DROP PROCEDURE p1; +DROP PROCEDURE p2; +DROP TABLE t1; +# The following test is taken from the task MDEV-36114 which is +# partially a duplicate of the task MDEV-36462 +CREATE PROCEDURE p() +BEGIN +DECLARE cur1 CURSOR FOR SELECT * FROM t; +BEGIN +DECLARE rec1 ROW TYPE OF cur1; +END; +END; +/ +CREATE TABLE t (id INT); +CALL p(); +CREATE OR REPLACE TABLE t (id INT); +CALL p(); +# Clean up +DROP PROCEDURE p; +DROP TABLE t; # End of 11.4 tests diff --git a/mysql-test/main/sp-cursor.test b/mysql-test/main/sp-cursor.test index d15a0c80a6f..b4e73aaf637 100644 --- a/mysql-test/main/sp-cursor.test +++ b/mysql-test/main/sp-cursor.test @@ -891,4 +891,60 @@ DROP PACKAGE pkg; SELECT pkg.f1(); DROP PACKAGE pkg; +--echo # +--echo # MDEV-36462: Crash on `DECLARE spvar1 ROW TYPE OF cursor1` after a table recreation +--echo # + +--delimiter / + +CREATE PROCEDURE p1() +BEGIN + DECLARE c CURSOR FOR SELECT a FROM t1; + BEGIN + DECLARE va ROW TYPE OF c; -- the crash happens here + END; +END; +/ + +CREATE PROCEDURE p2() +BEGIN + FOR i IN 1..10 DO -- usually it crashes on the third iteration, but not always + SELECT i; + CREATE OR REPLACE TABLE t1 (a INT); + CALL p1; + CALL p1; + END FOR; +END; +/ + +--delimiter ; + +CALL p2; + +--echo # Clean up +DROP PROCEDURE p1; +DROP PROCEDURE p2; +DROP TABLE t1; + +--echo # The following test is taken from the task MDEV-36114 which is +--echo # partially a duplicate of the task MDEV-36462 +--delimiter / +CREATE PROCEDURE p() +BEGIN + DECLARE cur1 CURSOR FOR SELECT * FROM t; + BEGIN + DECLARE rec1 ROW TYPE OF cur1; + END; +END; +/ +--delimiter ; +CREATE TABLE t (id INT); +CALL p(); +CREATE OR REPLACE TABLE t (id INT); +CALL p(); + +--echo # Clean up +DROP PROCEDURE p; +DROP TABLE t; + --echo # End of 11.4 tests diff --git a/sql/sp_instr.cc b/sql/sp_instr.cc index 7e403a24281..b15eeef6deb 100644 --- a/sql/sp_instr.cc +++ b/sql/sp_instr.cc @@ -662,11 +662,16 @@ bool sp_lex_instr::setup_table_fields_for_trigger( re-parsing. @param sphead The stored program. + @param[out] new_memroot_allocated true in case a new memory root for + re-parsing was created, else false meaning + that already allocated memory root is + reused @return false on success, true on error (OOM) */ -bool sp_lex_instr::setup_memroot_for_reparsing(sp_head *sphead) +bool sp_lex_instr::setup_memroot_for_reparsing(sp_head *sphead, + bool *new_memroot_allocated) { if (!m_mem_root_for_reparsing) { @@ -711,6 +716,8 @@ bool sp_lex_instr::setup_memroot_for_reparsing(sp_head *sphead) if (!m_mem_root_for_reparsing) return true; + + *new_memroot_allocated= true; } else { @@ -721,6 +728,7 @@ bool sp_lex_instr::setup_memroot_for_reparsing(sp_head *sphead) statement. */ free_root(m_mem_root_for_reparsing, MYF(0)); + *new_memroot_allocated= false; } init_sql_alloc(key_memory_sp_head_main_root, m_mem_root_for_reparsing, @@ -787,7 +795,8 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp, LEX *sp_instr_lex) /* First, set up a men_root for the statement is going to re-compile. */ - if (setup_memroot_for_reparsing(sp)) + bool mem_root_allocated; + if (setup_memroot_for_reparsing(sp, &mem_root_allocated)) return nullptr; /* @@ -842,8 +851,22 @@ LEX* sp_lex_instr::parse_expr(THD *thd, sp_head *sp, LEX *sp_instr_lex) data member. So, for the sp_instr_cpush instruction by the time we reach this block cursor_lex->free_list is already empty. */ - cleanup_items(cursor_lex->free_list); + if (mem_root_allocated) + /* + If the new memory root for re-parsing has been just created, + then delete every item from the free item list of sp_lex_cursor. + In case the memory root for re-parsing is re-used from previous + re-parsing of failed instruction, don't do anything since all memory + allocated for items were already released on calling free_root + inside the method sp_lex_instr::setup_memroot_for_reparsing + */ + cursor_lex->free_items(); + + /* Nullify free_list to don't have a dangling pointer */ + cursor_lex->free_list= nullptr; + cursor_free_list= &cursor_lex->free_list; + cursor_lex->mem_root= m_mem_root_for_reparsing; DBUG_ASSERT(thd->lex == sp_instr_lex); lex_start(thd); } diff --git a/sql/sp_instr.h b/sql/sp_instr.h index a93f681e5d1..5d5af0a18f0 100644 --- a/sql/sp_instr.h +++ b/sql/sp_instr.h @@ -522,7 +522,8 @@ private: THD *thd, sp_head *sp, SQL_I_List *next_trig_items_list); - bool setup_memroot_for_reparsing(sp_head *sphead); + bool setup_memroot_for_reparsing(sp_head *sphead, + bool *new_memroot_allocated); };