3258 Jon Olav Hauglid 2011-07-14
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.
modified:
mysql-test/r/sp.result
mysql-test/t/sp.test
sql/sp_head.cc
sql/sp_head.h
sql/sql_yacc.yy
3257 Jon Olav Hauglid 2011-07-14
Bug#11756966 - 48958: STORED PROCEDURES CAN BE LEVERAGED TO BYPASS
DATABASE SECURITY
The problem was that CREATE PROCEDURE/FUCTION could be used to
check the existence of databases for which the user had no
privileges and therefore should not be allowed to see.
The reason was that existence of a given database was checked
before privileges. So trying to create a stored routine in
a non-existent database would give a different error than trying
to create a stored routine in a restricted database.
This patch fixes the problem by changing the order of the checks
for CREATE PROCEDURE/FUNCTION so that privileges are checked first.
This means that trying to create a stored routine in a
non-existent database and in a restricted database both will
give ER_DBACCESS_DENIED_ERROR error.
Test case added to grant.test.
modified:
mysql-test/r/grant.result
mysql-test/r/information_schema.result
mysql-test/t/grant.test
mysql-test/t/information_schema.test
sql/sql_parse.cc
=== modified file 'mysql-test/r/sp.result'
--- a/mysql-test/r/sp.result 2011-06-21 15:32:01 +0000
+++ b/mysql-test/r/sp.result 2011-07-14 08:05:12 +0000
@@ -7573,3 +7573,26 @@ DROP TABLE t1;
DROP PROCEDURE p1;
# End of 5.5 test
+#
+# 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
+Warnings:
+Error 1424 Recursive stored functions and triggers are not allowed.
+Error 1305 FUNCTION test.f1 does not exist
+DROP FUNCTION f1;
=== modified file 'mysql-test/t/sp.test'
--- a/mysql-test/t/sp.test 2011-06-21 15:32:01 +0000
+++ b/mysql-test/t/sp.test 2011-07-14 08:05:12 +0000
@@ -8835,3 +8835,32 @@ DROP PROCEDURE p1;
--echo
--echo # End of 5.5 test
+
+
+--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;
=== modified file 'sql/sp_head.cc'
--- a/sql/sp_head.cc 2011-07-07 09:45:10 +0000
+++ b/sql/sp_head.cc 2011-07-14 08:05:12 +0000
@@ -3547,6 +3547,23 @@ sp_instr_hpush_jump::opt_mark(sp_head *s
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;
}
=== modified file 'sql/sp_head.h'
--- a/sql/sp_head.h 2011-06-30 15:50:45 +0000
+++ b/sql/sp_head.h 2011-07-14 08:05:12 +0000
@@ -1019,7 +1019,7 @@ class sp_instr_hpush_jump : public sp_in
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();
}
@@ -1041,6 +1041,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);
@@ -1050,6 +1059,7 @@ private:
int m_type; ///< Handler type
uint m_frame;
+ uint m_opt_hpop; // hpop marking end of handler scope.
List<struct sp_cond_type> m_cond;
}; // class sp_instr_hpush_jump : public sp_instr_jump
=== modified file 'sql/sql_yacc.yy'
--- a/sql/sql_yacc.yy 2011-07-07 12:44:34 +0000
+++ b/sql/sql_yacc.yy 2011-07-14 08:05:12 +0000
@@ -2774,9 +2774,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
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-trunk branch (jon.hauglid:3257 to 3258) Bug#12663165 | Jon Olav Hauglid | 17 Jul |