List:Commits« Previous MessageNext Message »
From:pem Date:February 15 2006 12:11pm
Subject:bk commit into 5.0 tree (pem:1.2042) BUG#16887
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of pem. When pem 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
  1.2042 06/02/15 12:11:29 pem@stripped +5 -0
  Fixed BUG#16887: Cursor causes server segfault
    The problem was a code generation bug: cpop instructions were not generated
    when using ITERATE back to an outer block from a context with a declared
    cursor; this would make it push a new cursor without popping in-between,
    eventually overrunning the cursor stack with a crash as the result.
    Fixed the calculation of how many cursors to pop (in sp_pcontext.cc:
    diff_cursors()), and also corrected diff_cursors() and diff_handlers()
    to when doing a "leave"; don't include the last context we're leaving
    (we are then jumping to the appropriate pop instructions).

  sql/sql_yacc.yy
    1.454 06/02/15 12:11:25 pem@stripped +4 -4
    Added parameter to diff_handlers/diff_cursors depending on if it's an
    iterate or leave jump.
    For "leave", we don't have to include the last context we're leaving since
    we will jump to the appropriate pop instructions.

  sql/sp_pcontext.h
    1.27 06/02/15 12:11:25 pem@stripped +7 -3
    Added new parameter to sp_pcontext::diff_handlers() and diff_cursors():
    They can either include (for iterate jumps) or exclude (for leave jumps)
    the outer context.

  sql/sp_pcontext.cc
    1.29 06/02/15 12:11:25 pem@stripped +12 -4
    Added new parameter to sp_pcontext::diff_handlers() and diff_cursors():
    They can either include (for iterate jumps) or exclude (for leave jumps)
    the outer context.
    Fixed bug in diff_cursors(); it was just plain wrong and would return
    zero in some situations when it shouldn't.

  mysql-test/t/sp.test
    1.175 06/02/15 12:11:25 pem@stripped +54 -0
    New test case for BUG#16887

  mysql-test/r/sp.result
    1.186 06/02/15 12:11:25 pem@stripped +56 -0
    Updated result for new test case (BUG#16887)

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	pem
# Host:	pem.mysql.com
# Root:	/extern/mysql/5.0/bug16887/mysql-5.0-runtime

--- 1.453/sql/sql_yacc.yy	2006-02-09 13:00:28 +01:00
+++ 1.454/sql/sql_yacc.yy	2006-02-15 12:11:25 +01:00
@@ -2079,10 +2079,10 @@
 	      uint ip= sp->instructions();
 	      uint n;
 
-	      n= ctx->diff_handlers(lab->ctx);
+	      n= ctx->diff_handlers(lab->ctx, TRUE);  /* Exclusive the dest. */
 	      if (n)
 	        sp->add_instr(new sp_instr_hpop(ip++, ctx, n));
-	      n= ctx->diff_cursors(lab->ctx);
+	      n= ctx->diff_cursors(lab->ctx, TRUE);  /* Exclusive the dest. */
 	      if (n)
 	        sp->add_instr(new sp_instr_cpop(ip++, ctx, n));
 	      i= new sp_instr_jump(ip, ctx);
@@ -2108,10 +2108,10 @@
 	      uint ip= sp->instructions();
 	      uint n;
 
-	      n= ctx->diff_handlers(lab->ctx);
+	      n= ctx->diff_handlers(lab->ctx, FALSE);  /* Inclusive the dest. */
 	      if (n)
 	        sp->add_instr(new sp_instr_hpop(ip++, ctx, n));
-	      n= ctx->diff_cursors(lab->ctx);
+	      n= ctx->diff_cursors(lab->ctx, FALSE);  /* Inclusive the dest. */
 	      if (n)
 	        sp->add_instr(new sp_instr_cpop(ip++, ctx, n));
 	      i= new sp_instr_jump(ip, ctx, lab->ip); /* Jump back */

--- 1.185/mysql-test/r/sp.result	2006-02-06 14:09:07 +01:00
+++ 1.186/mysql-test/r/sp.result	2006-02-15 12:11:25 +01:00
@@ -4519,4 +4519,60 @@
 Inner
 drop procedure bug15011|
 drop table t3|
+drop table if exists t3|
+drop procedure if exists bug16887|
+create table t3 ( c varchar(1) )|
+insert into t3 values
+(' '),('.'),(';'),(','),('-'),('_'),('('),(')'),('/'),('\\')|
+create procedure bug16887()
+begin
+declare i int default 10;
+again:
+while i > 0 do
+begin
+declare breakchar varchar(1);
+declare done int default 0;
+declare t3_cursor cursor for select c from t3;
+declare continue handler for not found set done = 1;
+set i = i - 1;
+select i;
+if i = 3 then
+iterate again;
+end if;
+open t3_cursor;
+loop
+fetch t3_cursor into breakchar;
+if done = 1 then
+begin
+close t3_cursor;
+iterate again;
+end;
+end if;
+end loop;
+end;
+end while;
+end|
+call bug16887()|
+i
+9
+i
+8
+i
+7
+i
+6
+i
+5
+i
+4
+i
+3
+i
+2
+i
+1
+i
+0
+drop table t3|
+drop procedure bug16887|
 drop table t1,t2;

--- 1.174/mysql-test/t/sp.test	2006-02-06 14:09:07 +01:00
+++ 1.175/mysql-test/t/sp.test	2006-02-15 12:11:25 +01:00
@@ -5312,6 +5312,60 @@
 
 
 #
+# BUG#16887: Cursor causes server segfault
+#
+--disable_warnings
+drop table if exists t3|
+drop procedure if exists bug16887|
+--enable_warnings
+
+create table t3 ( c varchar(1) )|
+
+insert into t3 values
+  (' '),('.'),(';'),(','),('-'),('_'),('('),(')'),('/'),('\\')|
+
+create procedure bug16887()
+begin
+  declare i int default 10;
+
+ again:
+  while i > 0 do
+  begin
+    declare breakchar varchar(1);
+    declare done int default 0;
+    declare t3_cursor cursor for select c from t3;
+    declare continue handler for not found set done = 1;
+
+    set i = i - 1;
+    select i;
+
+    if i = 3 then
+      iterate again;
+    end if;
+
+    open t3_cursor;
+
+    loop
+      fetch t3_cursor into breakchar;
+
+      if done = 1 then
+        begin
+          close t3_cursor;
+          iterate again;
+        end;
+      end if;
+     end loop;
+   end;
+   end while;
+end|
+
+call bug16887()|
+
+drop table t3|
+drop procedure bug16887|
+
+
+#
 # BUG#NNNN: New bug synopsis
 #
 #--disable_warnings

--- 1.28/sql/sp_pcontext.cc	2005-12-07 15:01:08 +01:00
+++ 1.29/sql/sp_pcontext.cc	2006-02-15 12:11:25 +01:00
@@ -122,30 +122,38 @@
 }
 
 uint
-sp_pcontext::diff_handlers(sp_pcontext *ctx)
+sp_pcontext::diff_handlers(sp_pcontext *ctx, bool exclusive)
 {
   uint n= 0;
   sp_pcontext *pctx= this;
+  sp_pcontext *last_ctx= NULL;
 
   while (pctx && pctx != ctx)
   {
     n+= pctx->m_handlers;
+    last_ctx= pctx;
     pctx= pctx->parent_context();
   }
   if (pctx)
-    return n;
+    return (exclusive && last_ctx ? n - last_ctx->m_handlers : n);
   return 0;			// Didn't find ctx
 }
 
 uint
-sp_pcontext::diff_cursors(sp_pcontext *ctx)
+sp_pcontext::diff_cursors(sp_pcontext *ctx, bool exclusive)
 {
+  uint n= 0;
   sp_pcontext *pctx= this;
+  sp_pcontext *last_ctx= NULL;
 
   while (pctx && pctx != ctx)
+  {
+    n+= pctx->m_cursor.elements;
+    last_ctx= pctx;
     pctx= pctx->parent_context();
+  }
   if (pctx)
-    return ctx->current_cursors() - pctx->current_cursors();
+    return  (exclusive && last_ctx ? n - last_ctx->m_cursor.elements : n);
   return 0;			// Didn't find ctx
 }
 

--- 1.26/sql/sp_pcontext.h	2006-01-16 15:25:36 +01:00
+++ 1.27/sql/sp_pcontext.h	2006-02-15 12:11:25 +01:00
@@ -119,11 +119,15 @@
     return m_parent;
   }
 
+  /*
+    Number of handlers/cursors to pop between this context and 'ctx'.
+    If 'exclusive' is true, don't count the last block we are leaving;
+    this is used for LEAVE where we will jump to the cpop/hpop instructions.
+  */
   uint
-  diff_handlers(sp_pcontext *ctx);
-
+  diff_handlers(sp_pcontext *ctx, bool exclusive);
   uint
-  diff_cursors(sp_pcontext *ctx);
+  diff_cursors(sp_pcontext *ctx, bool exclusive);
 
 
   //
Thread
bk commit into 5.0 tree (pem:1.2042) BUG#16887pem15 Feb