From: Jon Olav Hauglid
Date: July 14 2011 8:05am
Subject: bzr push into mysql-trunk branch (jon.hauglid:3257 to 3258) Bug#12663165
List-Archive: http://lists.mysql.com/commits/140309
X-Bug: 12663165
Message-Id: <201107140806.p6E868iO031951@acsmt356.oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

 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).