Bug#33618 (Crash in sp_rcontext)

Bug 33983 (Stored Procedures: wrong end <label> syntax is accepted)

The server used to crash when REPEAT or another control instruction
was used in conjunction with labels and a LEAVE instruction.

The crash was caused by a missing "pop" of handlers or cursors in the
code representing the stored program. When executing the code in a loop,
this missing "pop" would result in a stack overflow, corrupting memory.

Code generation has been fixed to produce the missing h_pop/c_pop
instructions.

Also, the logic checking that labels at the beginning and the end of a
statement are matched was incorrect, causing Bug 33983.
End labels, when used, must match the label used at the beginning of a block.


mysql-test/r/sp-code.result:
  Bug#33618 (Crash in sp_rcontext)
mysql-test/r/sp-error.result:
  Bug 33983 (Stored Procedures: wrong end <label> syntax is accepted)
mysql-test/r/sp.result:
  Bug#33618 (Crash in sp_rcontext)
mysql-test/t/sp-code.test:
  Bug#33618 (Crash in sp_rcontext)
mysql-test/t/sp-error.test:
  Bug 33983 (Stored Procedures: wrong end <label> syntax is accepted)
mysql-test/t/sp.test:
  Bug#33618 (Crash in sp_rcontext)
sql/sp_head.cc:
  Bug#33618 (Crash in sp_rcontext)
sql/sp_head.h:
  Bug#33618 (Crash in sp_rcontext)
sql/sp_rcontext.cc:
  Bug#33618 (Crash in sp_rcontext)
sql/sp_rcontext.h:
  Bug#33618 (Crash in sp_rcontext)
sql/sql_yacc.yy:
  Bug#33618 (Crash in sp_rcontext)

5 files changed, 166 insertions, 50 deletions

diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index e55ba81b117..74b9b8f6ae3 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -1933,11 +1933,17 @@ sp_head::backpatch(sp_label_t *lab)
   uint dest= instructions();
   List_iterator_fast<bp_t> li(m_backpatch);
 
+  DBUG_ENTER("sp_head::backpatch");
   while ((bp= li++))
   {
     if (bp->lab == lab)
+    {
+      DBUG_PRINT("info", ("backpatch: (m_ip %d, label 0x%lx <%s>) to dest %d",
+                          bp->instr->m_ip, (ulong) lab, lab->name, dest));
       bp->instr->backpatch(dest, lab->ctx);
+    }
   }
+  DBUG_VOID_RETURN;
 }
 
 /*
diff --git a/sql/sp_head.h b/sql/sp_head.h
index a46ec9433d7..0e710196603 100644
--- a/sql/sp_head.h
+++ b/sql/sp_head.h
@@ -779,8 +779,9 @@ public:
 
   virtual void backpatch(uint dest, sp_pcontext *dst_ctx)
   {
-    if (m_dest == 0)		// Don't reset
-      m_dest= dest;
+    /* Calling backpatch twice is a logic flaw in jump resolution. */
+    DBUG_ASSERT(m_dest == 0);
+    m_dest= dest;
   }
 
   /*
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index 54e016f6099..129aaa46de6 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -334,17 +334,91 @@ sp_rcontext::handle_error(uint sql_errno,
 void
 sp_rcontext::push_cursor(sp_lex_keeper *lex_keeper, sp_instr_cpush *i)
 {
+  DBUG_ENTER("sp_rcontext::push_cursor");
+  DBUG_ASSERT(m_ccount < m_root_parsing_ctx->max_cursor_index());
   m_cstack[m_ccount++]= new sp_cursor(lex_keeper, i);
+  DBUG_PRINT("info", ("m_ccount: %d", m_ccount));
+  DBUG_VOID_RETURN;
 }
 
-
 void
 sp_rcontext::pop_cursors(uint count)
 {
+  DBUG_ENTER("sp_rcontext::pop_cursors");
+  DBUG_ASSERT(m_ccount >= count);
   while (count--)
   {
     delete m_cstack[--m_ccount];
   }
+  DBUG_PRINT("info", ("m_ccount: %d", m_ccount));
+  DBUG_VOID_RETURN;
+}
+
+void
+sp_rcontext::push_handler(struct sp_cond_type *cond, uint h, int type, uint f)
+{
+  DBUG_ENTER("sp_rcontext::push_handler");
+  DBUG_ASSERT(m_hcount < m_root_parsing_ctx->max_handler_index());
+
+  m_handler[m_hcount].cond= cond;
+  m_handler[m_hcount].handler= h;
+  m_handler[m_hcount].type= type;
+  m_handler[m_hcount].foffset= f;
+  m_hcount+= 1;
+
+  DBUG_PRINT("info", ("m_hcount: %d", m_hcount));
+  DBUG_VOID_RETURN;
+}
+
+void
+sp_rcontext::pop_handlers(uint count)
+{
+  DBUG_ENTER("sp_rcontext::pop_handlers");
+  DBUG_ASSERT(m_hcount >= count);
+  m_hcount-= count;
+  DBUG_PRINT("info", ("m_hcount: %d", m_hcount));
+  DBUG_VOID_RETURN;
+}
+
+void
+sp_rcontext::push_hstack(uint h)
+{
+  DBUG_ENTER("sp_rcontext::push_hstack");
+  DBUG_ASSERT(m_hsp < m_root_parsing_ctx->max_handler_index());
+  m_hstack[m_hsp++]= h;
+  DBUG_PRINT("info", ("m_hsp: %d", m_hsp));
+  DBUG_VOID_RETURN;
+}
+
+uint
+sp_rcontext::pop_hstack()
+{
+  uint handler;
+  DBUG_ENTER("sp_rcontext::pop_hstack");
+  DBUG_ASSERT(m_hsp);
+  handler= m_hstack[--m_hsp];
+  DBUG_PRINT("info", ("m_hsp: %d", m_hsp));
+  DBUG_RETURN(handler);
+}
+
+void
+sp_rcontext::enter_handler(int hid)
+{
+  DBUG_ENTER("sp_rcontext::enter_handler");
+  DBUG_ASSERT(m_ihsp < m_root_parsing_ctx->max_handler_index());
+  m_in_handler[m_ihsp++]= hid;
+  DBUG_PRINT("info", ("m_ihsp: %d", m_ihsp));
+  DBUG_VOID_RETURN;
+}
+
+void
+sp_rcontext::exit_handler()
+{
+  DBUG_ENTER("sp_rcontext::exit_handler");
+  DBUG_ASSERT(m_ihsp);
+  m_ihsp-= 1;
+  DBUG_PRINT("info", ("m_ihsp: %d", m_ihsp));
+  DBUG_VOID_RETURN;
 }
 
 
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index 43102cfeeb2..368a017da21 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -107,21 +107,9 @@ class sp_rcontext : public Sql_alloc
     return m_return_value_set;
   }
 
-  inline void
-  push_handler(struct sp_cond_type *cond, uint h, int type, uint f)
-  {
-    m_handler[m_hcount].cond= cond;
-    m_handler[m_hcount].handler= h;
-    m_handler[m_hcount].type= type;
-    m_handler[m_hcount].foffset= f;
-    m_hcount+= 1;
-  }
+  void push_handler(struct sp_cond_type *cond, uint h, int type, uint f);
 
-  inline void
-  pop_handlers(uint count)
-  {
-    m_hcount-= count;
-  }
+  void pop_handlers(uint count);
 
   // Returns 1 if a handler was found, 0 otherwise.
   bool
@@ -158,29 +146,13 @@ class sp_rcontext : public Sql_alloc
     m_hfound= -1;
   }
 
-  inline void
-  push_hstack(uint h)
-  {
-    m_hstack[m_hsp++]= h;
-  }
+  void push_hstack(uint h);
 
-  inline uint
-  pop_hstack()
-  {
-    return m_hstack[--m_hsp];
-  }
+  uint pop_hstack();
 
-  inline void
-  enter_handler(int hid)
-  {
-    m_in_handler[m_ihsp++]= hid;
-  }
+  void enter_handler(int hid);
 
-  inline void
-  exit_handler()
-  {
-    m_ihsp-= 1;
-  }
+  void exit_handler();
 
   void
   push_cursor(sp_lex_keeper *lex_keeper, sp_instr_cpush *i);
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index bd85ce55edb..fdea950adf0 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -2214,6 +2214,10 @@ sp_proc_stmt:
 
 	    lex->sphead->backpatch(lex->spcont->pop_label());
 	  }
+        | sp_labeled_block
+          {}
+        | sp_unlabeled_block
+          {}
 	| LEAVE_SYM label_ident
 	  {
 	    LEX *lex= Lex;
@@ -2231,9 +2235,17 @@ sp_proc_stmt:
 	      sp_instr_jump *i;
 	      uint ip= sp->instructions();
 	      uint n;
+              /*
+                When jumping to a BEGIN-END block end, the target jump
+                points to the block hpop/cpop cleanup instructions,
+                so we should exclude the block context here.
+                When jumping to something else (i.e., SP_LAB_ITER),
+                there are no hpop/cpop at the jump destination,
+                so we should include the block context here for cleanup.
+              */
+              bool exclusive= (lab->type == SP_LAB_BEGIN);
 
-	      n= ctx->diff_handlers(lab->ctx, TRUE);  /* Exclusive the dest. */
-
+	      n= ctx->diff_handlers(lab->ctx, exclusive);
 	      if (n)
               {
                 sp_instr_hpop *hpop= new sp_instr_hpop(ip++, ctx, n);
@@ -2241,10 +2253,12 @@ sp_proc_stmt:
                   MYSQL_YYABORT;
 	        sp->add_instr(hpop);
               }
-	      n= ctx->diff_cursors(lab->ctx, TRUE);  /* Exclusive the dest. */
+	      n= ctx->diff_cursors(lab->ctx, exclusive);
 	      if (n)
               {
                 sp_instr_cpop *cpop= new sp_instr_cpop(ip++, ctx, n);
+                if (cpop == NULL)
+                  MYSQL_YYABORT;
 	        sp->add_instr(cpop);
               }
 	      i= new sp_instr_jump(ip, ctx);
@@ -2276,12 +2290,16 @@ sp_proc_stmt:
 	      if (n)
               {
                 sp_instr_hpop *hpop= new sp_instr_hpop(ip++, ctx, n);
+                if (hpop == NULL)
+                  MYSQL_YYABORT;
 	        sp->add_instr(hpop);
               }
 	      n= ctx->diff_cursors(lab->ctx, FALSE);  /* Inclusive the dest. */
 	      if (n)
               {
                 sp_instr_cpop *cpop= new sp_instr_cpop(ip++, ctx, n);
+                if (cpop == NULL)
+                  MYSQL_YYABORT;
 	        sp->add_instr(cpop);
               }
 	      i= new sp_instr_jump(ip, ctx, lab->ip); /* Jump back */
@@ -2577,19 +2595,17 @@ sp_labeled_control:
 	  sp_unlabeled_control sp_opt_label
 	  {
 	    LEX *lex= Lex;
+            sp_label_t *lab= lex->spcont->pop_label();
 
 	    if ($5.str)
 	    {
-	      sp_label_t *lab= lex->spcont->find_label($5.str);
-
-	      if (!lab ||
-	          my_strcasecmp(system_charset_info, $5.str, lab->name) != 0)
+	      if (my_strcasecmp(system_charset_info, $5.str, lab->name) != 0)
 	      {
 	        my_error(ER_SP_LABEL_MISMATCH, MYF(0), $5.str);
 	        MYSQL_YYABORT;
 	      }
 	    }
-	    lex->sphead->backpatch(lex->spcont->pop_label());
+	    lex->sphead->backpatch(lab);
 	  }
 	;
 
@@ -2598,15 +2614,59 @@ sp_opt_label:
         | label_ident   { $$= $1; }
 	;
 
-sp_unlabeled_control:
+sp_labeled_block:
+          label_ident ':'
+          {
+            LEX *lex= Lex;
+            sp_pcontext *ctx= lex->spcont;
+            sp_label_t *lab= ctx->find_label($1.str);
+
+            if (lab)
+            {
+              my_error(ER_SP_LABEL_REDEFINE, MYF(0), $1.str);
+              MYSQL_YYABORT;
+            }
+
+            lab= lex->spcont->push_label($1.str,
+                                         lex->sphead->instructions());
+            lab->type= SP_LAB_BEGIN;
+          }
+          sp_block_content sp_opt_label
+          {
+            LEX *lex= Lex;
+            sp_label_t *lab= lex->spcont->pop_label();
+
+            if ($5.str)
+            {
+              if (my_strcasecmp(system_charset_info, $5.str, lab->name) != 0)
+              {
+                my_error(ER_SP_LABEL_MISMATCH, MYF(0), $5.str);
+                MYSQL_YYABORT;
+              }
+            }
+          }
+        ;
+
+sp_unlabeled_block:
+          { /* Unlabeled blocks get a secret label. */
+            LEX *lex= Lex;
+            uint ip= lex->sphead->instructions();
+            sp_label_t *lab= lex->spcont->push_label((char *)"", ip);
+            lab->type= SP_LAB_BEGIN;
+          }
+          sp_block_content
+          {
+            LEX *lex= Lex;
+            lex->spcont->pop_label();
+          }
+        ;
+
+sp_block_content:
 	  BEGIN_SYM
 	  { /* QQ This is just a dummy for grouping declarations and statements
 	       together. No [[NOT] ATOMIC] yet, and we need to figure out how
 	       make it coexist with the existing BEGIN COMMIT/ROLLBACK. */
 	    LEX *lex= Lex;
-	    sp_label_t *lab= lex->spcont->last_label();
-
-	    lab->type= SP_LAB_BEGIN;
 	    lex->spcont= lex->spcont->push_context(LABEL_DEFAULT_SCOPE);
 	  }
 	  sp_decls
@@ -2636,7 +2696,10 @@ sp_unlabeled_control:
             }
 	    lex->spcont= ctx->pop_context();
 	  }
-	| LOOP_SYM
+        ;
+
+sp_unlabeled_control:
+	  LOOP_SYM
 	  sp_proc_stmts1 END LOOP_SYM
 	  {
 	    LEX *lex= Lex;