From b8291e2b60b311b621b15aff1dfec817da1bbf4c Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Mon, 9 Jan 2012 11:28:02 +0100 Subject: [PATCH] Backport from mysql-trunk of: ------------------------------------------------------------ revno: 3258 committer: Jon Olav Hauglid branch nick: mysql-trunk-bug12663165 timestamp: Thu 2011-07-14 10:05:12 +0200 message: Bug#12663165 SP DEAD CODE REMOVAL DOESN'T UNDERSTAND CONTINUE HANDLERS When stored routines are loaded, a simple optimizer tries to locate and remove dead code. The problem was that this dead code removal did not work correctly with CONTINUE handlers. If a statement triggers a CONTINUE handler, the following statement will be executed after the handler statement has completed. This means that the following statement is not dead code even if the previous statement unconditionally alters control flow. This fact was lost on the dead code removal routine, which ended up with removing instructions that could have been executed. This could then lead to assertions, crashes and generally bad behavior when the stored routine was executed. This patch fixes the problem by marking as live code all stored routine instructions that are in the same scope as a CONTINUE handler. Test case added to sp.test. --- mysql-test/r/sp.result | 20 ++++++++++++++++++++ mysql-test/t/sp.test | 29 +++++++++++++++++++++++++++++ sql/sp_head.cc | 17 +++++++++++++++++ sql/sp_head.h | 12 +++++++++++- sql/sql_yacc.yy | 12 +++++++++--- 5 files changed, 86 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index f5ed4fbd901..11d6ff02756 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -7087,6 +7087,26 @@ COUNT(DISTINCT d) 2 DROP FUNCTION f1; DROP TABLE t1, t2; +# +# Bug#12663165 SP DEAD CODE REMOVAL DOESN'T UNDERSTAND CONTINUE HANDLERS +# +DROP FUNCTION IF EXISTS f1; +CREATE FUNCTION f1() RETURNS INT +BEGIN +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION BEGIN END; +BEGIN +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION RETURN f1(); +BEGIN +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION RETURN f1(); +RETURN f1(); +END; +END; +RETURN 1; +END $ +SELECT f1(); +f1() +1 +DROP FUNCTION f1; # ------------------------------------------------------------------ # -- End of 5.1 tests # ------------------------------------------------------------------ diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 141d1604065..ae4e1dd588e 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -8397,6 +8397,35 @@ DROP FUNCTION f1; DROP TABLE t1, t2; +--echo # +--echo # Bug#12663165 SP DEAD CODE REMOVAL DOESN'T UNDERSTAND CONTINUE HANDLERS +--echo # + +--disable_warnings +DROP FUNCTION IF EXISTS f1; +--enable_warnings + +delimiter $; +CREATE FUNCTION f1() RETURNS INT +BEGIN + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION BEGIN END; + BEGIN + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION RETURN f1(); + BEGIN + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION RETURN f1(); + RETURN f1(); + END; + END; +RETURN 1; +END $ +delimiter ;$ + +# This used to cause an assertion. +SELECT f1(); + +DROP FUNCTION f1; + + --echo # ------------------------------------------------------------------ --echo # -- End of 5.1 tests --echo # ------------------------------------------------------------------ diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 8bea0be0f56..dec76615ec7 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -3356,6 +3356,23 @@ sp_instr_hpush_jump::opt_mark(sp_head *sp, List *leads) m_optdest= sp->get_instr(m_dest); } sp->add_mark_lead(m_dest, leads); + + /* + For continue handlers, all instructions in the scope of the handler + are possible leads. For example, the instruction after freturn might + be executed if the freturn triggers the condition handled by the + continue handler. + + m_dest marks the start of the handler scope. It's added as a lead + above, so we start on m_dest+1 here. + m_opt_hpop is the hpop marking the end of the handler scope. + */ + if (m_type == SP_HANDLER_CONTINUE) + { + for (uint scope_ip= m_dest+1; scope_ip <= m_opt_hpop; scope_ip++) + sp->add_mark_lead(scope_ip, leads); + } + return m_ip+1; } diff --git a/sql/sp_head.h b/sql/sp_head.h index da490527f42..a026ac47221 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -1000,7 +1000,7 @@ class sp_instr_hpush_jump : public sp_instr_jump public: sp_instr_hpush_jump(uint ip, sp_pcontext *ctx, int htype, uint fp) - : sp_instr_jump(ip, ctx), m_type(htype), m_frame(fp) + : sp_instr_jump(ip, ctx), m_type(htype), m_frame(fp), m_opt_hpop(0) { m_cond.empty(); } @@ -1022,6 +1022,15 @@ public: return m_ip; } + virtual void backpatch(uint dest, sp_pcontext *dst_ctx) + { + DBUG_ASSERT(!m_dest || !m_opt_hpop); + if (!m_dest) + m_dest= dest; + else + m_opt_hpop= dest; + } + inline void add_condition(struct sp_cond_type *cond) { m_cond.push_front(cond); @@ -1031,6 +1040,7 @@ private: int m_type; ///< Handler type uint m_frame; + uint m_opt_hpop; // hpop marking end of handler scope. List m_cond; }; // class sp_instr_hpush_jump : public sp_instr_jump diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index facbc6d3e95..7e7ff7e91ca 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -2532,9 +2532,15 @@ sp_decl: sp_instr_hpush_jump *i= new sp_instr_hpush_jump(sp->instructions(), ctx, $2, ctx->current_var_count()); - if (i == NULL || - sp->add_instr(i) || - sp->push_backpatch(i, ctx->push_label((char *)"", 0))) + if (i == NULL || sp->add_instr(i)) + MYSQL_YYABORT; + + /* For continue handlers, mark end of handler scope. */ + if ($2 == SP_HANDLER_CONTINUE && + sp->push_backpatch(i, ctx->last_label())) + MYSQL_YYABORT; + + if (sp->push_backpatch(i, ctx->push_label(empty_c_string, 0))) MYSQL_YYABORT; } sp_hcond_list sp_proc_stmt