Below is the list of changes that have just been committed into a local
5.0 repository of malff. When malff does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet@stripped, 2008-01-09 16:48:32-07:00, malff@stripped. +9 -0
Bug#33618 (Crash in sp_rcontext)
There are several layers of issues that caused this crash.
1)
The server crash in sp_rcontext, due to a stack overflow in the sp_rcontext
handler stack.
2)
The stack overflow was caused by incorrect generated code, where a hpop
instruction was missing for a LEAVE. Repeated execution of this LEAVE
statement in a loop caused wide spread memory corruption.
3)
(See the bug report for the code exposing the bug)
The missing hpop was caused by the call to sp_head::diff_handlers(),
which considered that there is no handler to pop since exclusive is true.
Code generation for the LEAVE statement assumed that the handler will be
removed by the code generated at the end of a begin-end block,
which did not happen when jumping to a REPEAT-UNTIL "rep1:" label.
4)
Resolution of forward jumps with the backpacth list (sp_head::backpatch)
was not working properly, and generated jumps to the wrong instruction.
This is because backpatches for begin-end blocks label were resolved
*twice*, and pointed to different locations:
- before the end hpop/cpop instructions ending a block
- after the end hpop/cpop instructions ending a block
This caused the jump for the LEAVE statement to the "rep1:" label to
point *after* the hpop instruction.
5)
Issue 4) in the code itself was hidden by a poor implementation
of sp_instr_jump:backpatch(), which accepted multiple destination for the
same instruction.
6)
Iteration labels like "rep1:" were initially typed as SP_LAB_ITER,
but later corrupted to SP_LAB_BEGIN in the parser grammar rules,
adding to the confusion when generating code.
With this fix,
(1) is fixed by enforcing integrity of every internal stack in sp_rcontext
with DBUG_ASSERT statements.
(2) and (3) are fixed by this line of code:
bool exclusive= (lab->type == SP_LAB_BEGIN);
(4) is fixed by implementing the following grammar rules in the parser:
- sp_unlabeled_block
- sp_labeled_block
these rules resolve the sp_head::backpatch() *inside* sp_block_content,
while rules for control flow operations do that *ousdide*
sp_unlabeled_control
In all cases, backpatch is done only once for a given label.
(5) is fixed in sp_instr_jump:backpatch() with a DBUG_ASSERT.
(6) is fixed by assigning SP_LAB_BEGIN only to block labels
mysql-test/r/sp-code.result@stripped, 2008-01-09 16:48:27-07:00,
malff@stripped. +109 -0
Bug#33618 (Crash in sp_rcontext)
mysql-test/r/sp.result@stripped, 2008-01-09 16:48:27-07:00,
malff@stripped. +35 -0
Bug#33618 (Crash in sp_rcontext)
mysql-test/t/sp-code.test@stripped, 2008-01-09 16:48:27-07:00,
malff@stripped. +77 -0
Bug#33618 (Crash in sp_rcontext)
mysql-test/t/sp.test@stripped, 2008-01-09 16:48:27-07:00, malff@stripped.
+51 -0
Bug#33618 (Crash in sp_rcontext)
sql/sp_head.cc@stripped, 2008-01-09 16:48:28-07:00, malff@stripped. +6 -0
Bug#33618 (Crash in sp_rcontext)
sql/sp_head.h@stripped, 2008-01-09 16:48:28-07:00, malff@stripped. +3 -2
Bug#33618 (Crash in sp_rcontext)
sql/sp_rcontext.cc@stripped, 2008-01-09 16:48:28-07:00, malff@stripped.
+75 -1
Bug#33618 (Crash in sp_rcontext)
sql/sp_rcontext.h@stripped, 2008-01-09 16:48:28-07:00, malff@stripped. +6
-34
Bug#33618 (Crash in sp_rcontext)
sql/sql_yacc.yy@stripped, 2008-01-09 16:48:28-07:00, malff@stripped. +76
-8
Bug#33618 (Crash in sp_rcontext)
diff -Nrup a/mysql-test/r/sp-code.result b/mysql-test/r/sp-code.result
--- a/mysql-test/r/sp-code.result 2007-05-07 02:23:08 -06:00
+++ b/mysql-test/r/sp-code.result 2008-01-09 16:48:27 -07:00
@@ -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.
diff -Nrup a/mysql-test/r/sp.result b/mysql-test/r/sp.result
--- a/mysql-test/r/sp.result 2007-10-29 04:58:08 -06:00
+++ b/mysql-test/r/sp.result 2008-01-09 16:48:27 -07:00
@@ -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
# ------------------------------------------------------------------
diff -Nrup a/mysql-test/t/sp-code.test b/mysql-test/t/sp-code.test
--- a/mysql-test/t/sp-code.test 2007-06-07 14:23:46 -06:00
+++ b/mysql-test/t/sp-code.test 2008-01-09 16:48:27 -07:00
@@ -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.
diff -Nrup a/mysql-test/t/sp.test b/mysql-test/t/sp.test
--- a/mysql-test/t/sp.test 2007-10-29 04:58:08 -06:00
+++ b/mysql-test/t/sp.test 2008-01-09 16:48:27 -07:00
@@ -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
diff -Nrup a/sql/sp_head.cc b/sql/sp_head.cc
--- a/sql/sp_head.cc 2007-11-27 08:56:40 -07:00
+++ b/sql/sp_head.cc 2008-01-09 16:48:28 -07:00
@@ -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 -Nrup a/sql/sp_head.h b/sql/sp_head.h
--- a/sql/sp_head.h 2007-11-21 01:58:41 -07:00
+++ b/sql/sp_head.h 2008-01-09 16:48:28 -07:00
@@ -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 -Nrup a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
--- a/sql/sp_rcontext.cc 2007-11-10 12:44:14 -07:00
+++ b/sql/sp_rcontext.cc 2008-01-09 16:48:28 -07:00
@@ -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 -Nrup a/sql/sp_rcontext.h b/sql/sp_rcontext.h
--- a/sql/sp_rcontext.h 2007-11-10 12:44:15 -07:00
+++ b/sql/sp_rcontext.h 2008-01-09 16:48:28 -07:00
@@ -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 -Nrup a/sql/sql_yacc.yy b/sql/sql_yacc.yy
--- a/sql/sql_yacc.yy 2007-12-13 03:49:13 -07:00
+++ b/sql/sql_yacc.yy 2008-01-09 16:48:28 -07:00
@@ -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 */
@@ -2598,15 +2616,62 @@ 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;
+
+ 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)
+ {
+ my_error(ER_SP_LABEL_MISMATCH, MYF(0), $5.str);
+ MYSQL_YYABORT;
+ }
+ }
+ lex->spcont->pop_label();
+ }
+ ;
+
+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 +2701,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;