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)

mysql-test/r/sp-code.result

@@ -733,4 +733,113 @@ optimizer: keep hreturn
 drop table t1;
 drop procedure proc_26977_broken;
 drop procedure proc_26977_works;
+drop procedure if exists proc_33618_h;
+drop procedure if exists proc_33618_c;
+create procedure proc_33618_h(num int)
+begin
+declare count1 int default '0';
+declare vb varchar(30);
+declare last_row int;
+while(num>=1) do
+set num=num-1;
+begin
+declare cur1 cursor for select `a` from t_33618;
+declare continue handler for not found set last_row = 1;
+set last_row:=0;
+open cur1;
+rep1:
+repeat
+begin
+declare exit handler for 1062 begin end;
+fetch cur1 into vb;
+if (last_row = 1) then
+## should generate a hpop instruction here
+leave rep1;
+end if;
+end;
+until last_row=1
+end repeat;
+close cur1;
+end;
+end while;
+end//
+create procedure proc_33618_c(num int)
+begin
+declare count1 int default '0';
+declare vb varchar(30);
+declare last_row int;
+while(num>=1) do
+set num=num-1;
+begin
+declare cur1 cursor for select `a` from t_33618;
+declare continue handler for not found set last_row = 1;
+set last_row:=0;
+open cur1;
+rep1:
+repeat
+begin
+declare cur2 cursor for select `b` from t_33618;
+fetch cur1 into vb;
+if (last_row = 1) then
+## should generate a cpop instruction here
+leave rep1;
+end if;
+end;
+until last_row=1
+end repeat;
+close cur1;
+end;
+end while;
+end//
+show procedure code proc_33618_h;
+Pos	Instruction
+0	set count1@1 _latin1'0'
+1	set vb@2 NULL
+2	set last_row@3 NULL
+3	jump_if_not 24(24) (num@0 >= 1)
+4	set num@0 (num@0 - 1)
+5	cpush cur1@0
+6	hpush_jump 9 4 CONTINUE
+7	set last_row@3 1
+8	hreturn 4
+9	set last_row@3 0
+10	copen cur1@0
+11	hpush_jump 13 4 EXIT
+12	hreturn 0 17
+13	cfetch cur1@0 vb@2
+14	jump_if_not 17(17) (last_row@3 = 1)
+15	hpop 1
+16	jump 19
+17	hpop 1
+18	jump_if_not 11(19) (last_row@3 = 1)
+19	cclose cur1@0
+20	hpop 1
+21	cpop 1
+22	jump 3
+show procedure code proc_33618_c;
+Pos	Instruction
+0	set count1@1 _latin1'0'
+1	set vb@2 NULL
+2	set last_row@3 NULL
+3	jump_if_not 23(23) (num@0 >= 1)
+4	set num@0 (num@0 - 1)
+5	cpush cur1@0
+6	hpush_jump 9 4 CONTINUE
+7	set last_row@3 1
+8	hreturn 4
+9	set last_row@3 0
+10	copen cur1@0
+11	cpush cur2@1
+12	cfetch cur1@0 vb@2
+13	jump_if_not 16(16) (last_row@3 = 1)
+14	cpop 1
+15	jump 18
+16	cpop 1
+17	jump_if_not 11(18) (last_row@3 = 1)
+18	cclose cur1@0
+19	hpop 1
+20	cpop 1
+21	jump 3
+drop procedure proc_33618_h;
+drop procedure proc_33618_c;
 End of 5.0 tests.

mysql-test/r/sp-error.result

@@ -1465,3 +1465,51 @@ ERROR 42000: You have an error in your SQL syntax; check the manual that corresp
 SELECT ..inexistent();
 ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '.inexistent()' at line 1
 USE test;
+drop procedure if exists proc_33983_a;
+drop procedure if exists proc_33983_b;
+drop procedure if exists proc_33983_c;
+drop procedure if exists proc_33983_d;
+create procedure proc_33983_a()
+begin
+label1:
+begin
+label2:
+begin
+select 1;
+end label1;
+end;
+end|
+ERROR 42000: End-label label1 without match
+create procedure proc_33983_b()
+begin
+label1:
+repeat
+label2:
+repeat
+select 1;
+until FALSE end repeat label1;
+until FALSE end repeat;
+end|
+ERROR 42000: End-label label1 without match
+create procedure proc_33983_c()
+begin
+label1:
+while TRUE do
+label2:
+while TRUE do
+select 1;
+end while label1;
+end while;
+end|
+ERROR 42000: End-label label1 without match
+create procedure proc_33983_d()
+begin
+label1:
+loop
+label2:
+loop
+select 1;
+end loop label1;
+end loop;
+end|
+ERROR 42000: End-label label1 without match

mysql-test/r/sp.result

@@ -6578,6 +6578,41 @@ DROP PROCEDURE db28318_a.t1;
 DROP PROCEDURE db28318_b.t2;
 DROP DATABASE db28318_a;
 DROP DATABASE db28318_b;
+use test;
+drop table if exists t_33618;
+drop procedure if exists proc_33618;
+create table t_33618 (`a` int, unique(`a`), `b` varchar(30)) engine=myisam;
+insert into t_33618 (`a`,`b`) values (1,'1'),(2,'2');
+create procedure proc_33618(num int)
+begin
+declare count1 int default '0';
+declare vb varchar(30);
+declare last_row int;
+while(num>=1) do
+set num=num-1;
+begin
+declare cur1 cursor for select `a` from t_33618;
+declare continue handler for not found set last_row = 1;
+set last_row:=0;
+open cur1;
+rep1:
+repeat
+begin
+declare exit handler for 1062 begin end;
+fetch cur1 into vb;
+if (last_row = 1) then
+leave rep1;
+end if;
+end;
+until last_row=1
+end repeat;
+close cur1;
+end;
+end while;
+end//
+call proc_33618(20);
+drop table t_33618;
+drop procedure proc_33618;
 # ------------------------------------------------------------------
 # -- End of 5.0 tests
 # ------------------------------------------------------------------

mysql-test/t/sp-code.test

@@ -520,5 +520,82 @@ drop table t1;
 drop procedure proc_26977_broken;
 drop procedure proc_26977_works;
 
+#
+# Bug#33618 Crash in sp_rcontext
+#
+
+--disable_warnings
+drop procedure if exists proc_33618_h;
+drop procedure if exists proc_33618_c;
+--enable_warnings
+
+delimiter //;
+
+create procedure proc_33618_h(num int)
+begin
+  declare count1 int default '0';
+  declare vb varchar(30);
+  declare last_row int;
+        
+  while(num>=1) do
+    set num=num-1;
+    begin
+      declare cur1 cursor for select `a` from t_33618;
+      declare continue handler for not found set last_row = 1;
+      set last_row:=0;
+      open cur1;
+      rep1:
+      repeat
+        begin
+          declare exit handler for 1062 begin end;
+          fetch cur1 into vb;
+          if (last_row = 1) then
+            ## should generate a hpop instruction here
+            leave rep1;
+          end if;
+        end;
+        until last_row=1
+      end repeat;
+      close cur1;
+    end;
+  end while;
+end//
+
+create procedure proc_33618_c(num int)
+begin
+  declare count1 int default '0';
+  declare vb varchar(30);
+  declare last_row int;
+        
+  while(num>=1) do
+    set num=num-1;
+    begin
+      declare cur1 cursor for select `a` from t_33618;
+      declare continue handler for not found set last_row = 1;
+      set last_row:=0;
+      open cur1;
+      rep1:
+      repeat
+        begin
+          declare cur2 cursor for select `b` from t_33618;
+          fetch cur1 into vb;
+          if (last_row = 1) then
+            ## should generate a cpop instruction here
+            leave rep1;
+          end if;
+        end;
+        until last_row=1
+      end repeat;
+      close cur1;
+    end;
+  end while;
+end//
+delimiter ;//
+
+show procedure code proc_33618_h;
+show procedure code proc_33618_c;
+
+drop procedure proc_33618_h;
+drop procedure proc_33618_c;
 
 --echo End of 5.0 tests.

mysql-test/t/sp-error.test

@@ -2112,6 +2112,69 @@ SELECT .inexistent();
 SELECT ..inexistent();
 USE test;
 
+#
+# Bug#33983 (Stored Procedures: wrong end <label> syntax is accepted)
+#
+
+--disable_warnings
+drop procedure if exists proc_33983_a;
+drop procedure if exists proc_33983_b;
+drop procedure if exists proc_33983_c;
+drop procedure if exists proc_33983_d;
+--enable_warnings
+
+delimiter |;
+
+--error ER_SP_LABEL_MISMATCH
+create procedure proc_33983_a()
+begin
+  label1:
+    begin
+      label2:
+      begin
+        select 1;
+      end label1;
+    end;
+end|
+
+--error ER_SP_LABEL_MISMATCH
+create procedure proc_33983_b()
+begin
+  label1:
+    repeat
+      label2:
+      repeat
+        select 1;
+      until FALSE end repeat label1;
+    until FALSE end repeat;
+end|
+
+--error ER_SP_LABEL_MISMATCH
+create procedure proc_33983_c()
+begin
+  label1:
+    while TRUE do
+      label2:
+      while TRUE do
+        select 1;
+      end while label1;
+    end while;
+end|
+
+--error ER_SP_LABEL_MISMATCH
+create procedure proc_33983_d()
+begin
+  label1:
+    loop
+      label2:
+      loop
+        select 1;
+      end loop label1;
+    end loop;
+end|
+
+delimiter ;|
+
 #
 # BUG#NNNN: New bug synopsis
 #

mysql-test/t/sp.test

@@ -7699,6 +7699,57 @@ DROP PROCEDURE db28318_b.t2;
 DROP DATABASE db28318_a;
 DROP DATABASE db28318_b;
 
+#
+# Bug#33618 Crash in sp_rcontext
+#
+
+use test;
+
+--disable_warnings
+drop table if exists t_33618;
+drop procedure if exists proc_33618;
+--enable_warnings
+
+create table t_33618 (`a` int, unique(`a`), `b` varchar(30)) engine=myisam;
+insert into t_33618 (`a`,`b`) values (1,'1'),(2,'2');
+
+delimiter //;
+
+create procedure proc_33618(num int)
+begin
+  declare count1 int default '0';
+  declare vb varchar(30);
+  declare last_row int;
+        
+  while(num>=1) do
+    set num=num-1;
+    begin
+      declare cur1 cursor for select `a` from t_33618;
+      declare continue handler for not found set last_row = 1;
+      set last_row:=0;
+      open cur1;
+      rep1:
+      repeat
+        begin
+          declare exit handler for 1062 begin end;
+          fetch cur1 into vb;
+          if (last_row = 1) then
+            leave rep1;
+          end if;
+        end;
+        until last_row=1
+      end repeat;
+      close cur1;
+    end;
+  end while;
+end//
+
+delimiter ;//
+
+call proc_33618(20);
+
+drop table t_33618;
+drop procedure proc_33618;
 
 --echo # ------------------------------------------------------------------
 --echo # -- End of 5.0 tests

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;
 }
 
 /*

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;
   }
 
   /*

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;
 }
 
 

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

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;