Fix for BUG#13549 "Server crash with nested stored procedures

if inner routine has more local variables than outer one, and
one of its last variables was used as argument to NOT operator".

THD::spcont was non-0 when we were parsing stored routine/trigger
definition during execution of another stored routine. This confused
methods of Item_splocal and forced them use wrong runtime context.
Fix ensures that we always have THD::spcont equal to zero during
routine/trigger body parsing. This also allows to avoid problems
with errors which occur during parsing and SQL exception handlers.


mysql-test/r/sp.result:
  Test suite for bug#13549.
mysql-test/r/trigger.result:
  Test suite for bug#13549.
mysql-test/t/sp.test:
  Test suite for bug#13549.
mysql-test/t/trigger.test:
  Test suite for bug#13549.
sql/item.cc:
  Protection against using wrong context by SP local variable.
sql/item.h:
  Protection against using wrong context by SP local variable.
sql/protocol.cc:
  An incorrect macro name fixed.
sql/protocol.h:
  An incorrect macro name fixed.
sql/sp.cc:
  Do not allow SP which we are parsing to use other SP
  context (BUG#13549).
sql/sp_head.cc:
  Protection against using wrong context by SP local variable.
sql/sp_rcontext.h:
  Protection against using wrong context by SP local variable.
sql/sql_cache.h:
  An incorrect macro name fixed.
sql/sql_class.cc:
  Protection against using wrong context by SP local variable.
sql/sql_class.h:
  Protection against using wrong context by SP local variable.
sql/sql_trigger.cc:
  Do not allow Trigger which we are parsing to use
  other SP context (BUG#13549).
sql/sql_yacc.yy:
  Protection against using wrong context by SP local variable.

12 files changed, 92 insertions, 27 deletions

diff --git a/sql/item.cc b/sql/item.cc
index 966dbbaec53..7a8ce37ce63 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -868,7 +868,7 @@ Item *
 Item_splocal::this_item()
 {
   THD *thd= current_thd;
-
+  DBUG_ASSERT(owner == thd->spcont->owner);
   return thd->spcont->get_item(m_offset);
 }
 
@@ -876,6 +876,7 @@ Item_splocal::this_item()
 Item **
 Item_splocal::this_item_addr(THD *thd, Item **addr)
 {
+  DBUG_ASSERT(owner == thd->spcont->owner);
   return thd->spcont->get_item_addr(m_offset);
 }
 
@@ -884,6 +885,7 @@ Item_splocal::this_const_item() const
 {
   THD *thd= current_thd;
 
+  DBUG_ASSERT(owner == thd->spcont->owner);
   return thd->spcont->get_item(m_offset);
 }
 
@@ -893,7 +895,10 @@ Item_splocal::type() const
   THD *thd= current_thd;
 
   if (thd->spcont)
+  {
+    DBUG_ASSERT(owner == thd->spcont->owner);
     return thd->spcont->get_item(m_offset)->type();
+  }
   return NULL_ITEM;		// Anything but SUBSELECT_ITEM
 }
 
diff --git a/sql/item.h b/sql/item.h
index a8f013f60d4..516cb05c2a2 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -703,6 +703,8 @@ public:
 };
 
 
+class sp_head;
+
 /*
   A reference to local SP variable (incl. reference to SP parameter), used in
   runtime.
@@ -720,6 +722,13 @@ class Item_splocal : public Item
   uint m_offset;
 
 public:
+#ifndef DBUG_OFF
+  /*
+    Routine to which this Item_splocal belongs. Used for checking if correct
+    runtime context is used for variable handling.
+  */
+  sp_head *owner;
+#endif
   LEX_STRING m_name;
 
   /*
diff --git a/sql/protocol.cc b/sql/protocol.cc
index ade94a483a8..490f27ab548 100644
--- a/sql/protocol.cc
+++ b/sql/protocol.cc
@@ -494,7 +494,7 @@ void Protocol::init(THD *thd_arg)
   thd=thd_arg;
   packet= &thd->packet;
   convert= &thd->convert_buffer;
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   field_types= 0;
 #endif
 }
@@ -547,7 +547,7 @@ bool Protocol::send_fields(List<Item> *list, uint flags)
     (void) my_net_write(&thd->net, buff,(uint) (pos-buff));
   }
 
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   field_types= (enum_field_types*) thd->alloc(sizeof(field_types) *
 					      list->elements);
   uint count= 0;
@@ -644,7 +644,7 @@ bool Protocol::send_fields(List<Item> *list, uint flags)
       item->send(&prot, &tmp);			// Send default value
     if (prot.write())
       break;					/* purecov: inspected */
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
     field_types[count++]= field.type;
 #endif
   }
@@ -728,14 +728,14 @@ bool Protocol::store(I_List<i_string>* str_list)
 void Protocol_simple::prepare_for_resend()
 {
   packet->length(0);
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   field_pos= 0;
 #endif
 }
 
 bool Protocol_simple::store_null()
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   field_pos++;
 #endif
   char buff[1];
@@ -769,7 +769,7 @@ bool Protocol::store_string_aux(const char *from, uint length,
 bool Protocol_simple::store(const char *from, uint length,
 			    CHARSET_INFO *fromcs, CHARSET_INFO *tocs)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_DECIMAL ||
               field_types[field_pos] == MYSQL_TYPE_BIT ||
@@ -786,7 +786,7 @@ bool Protocol_simple::store(const char *from, uint length,
 			    CHARSET_INFO *fromcs)
 {
   CHARSET_INFO *tocs= this->thd->variables.character_set_results;
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_DECIMAL ||
               field_types[field_pos] == MYSQL_TYPE_BIT ||
@@ -801,7 +801,7 @@ bool Protocol_simple::store(const char *from, uint length,
 
 bool Protocol_simple::store_tiny(longlong from)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 || field_types[field_pos] == MYSQL_TYPE_TINY);
   field_pos++;
 #endif
@@ -813,7 +813,7 @@ bool Protocol_simple::store_tiny(longlong from)
 
 bool Protocol_simple::store_short(longlong from)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_YEAR ||
 	      field_types[field_pos] == MYSQL_TYPE_SHORT);
@@ -827,7 +827,7 @@ bool Protocol_simple::store_short(longlong from)
 
 bool Protocol_simple::store_long(longlong from)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
               field_types[field_pos] == MYSQL_TYPE_INT24 ||
               field_types[field_pos] == MYSQL_TYPE_LONG);
@@ -841,7 +841,7 @@ bool Protocol_simple::store_long(longlong from)
 
 bool Protocol_simple::store_longlong(longlong from, bool unsigned_flag)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_LONGLONG);
   field_pos++;
@@ -856,7 +856,7 @@ bool Protocol_simple::store_longlong(longlong from, bool unsigned_flag)
 
 bool Protocol_simple::store_decimal(const my_decimal *d)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
               field_types[field_pos] == MYSQL_TYPE_NEWDECIMAL);
   field_pos++;
@@ -870,7 +870,7 @@ bool Protocol_simple::store_decimal(const my_decimal *d)
 
 bool Protocol_simple::store(float from, uint32 decimals, String *buffer)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_FLOAT);
   field_pos++;
@@ -882,7 +882,7 @@ bool Protocol_simple::store(float from, uint32 decimals, String *buffer)
 
 bool Protocol_simple::store(double from, uint32 decimals, String *buffer)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_DOUBLE);
   field_pos++;
@@ -896,7 +896,7 @@ bool Protocol_simple::store(Field *field)
 {
   if (field->is_null())
     return store_null();
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   field_pos++;
 #endif
   char buff[MAX_FIELD_WIDTH];
@@ -917,7 +917,7 @@ bool Protocol_simple::store(Field *field)
 
 bool Protocol_simple::store(TIME *tm)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_DATETIME ||
 	      field_types[field_pos] == MYSQL_TYPE_TIMESTAMP);
@@ -940,7 +940,7 @@ bool Protocol_simple::store(TIME *tm)
 
 bool Protocol_simple::store_date(TIME *tm)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_DATE);
   field_pos++;
@@ -959,7 +959,7 @@ bool Protocol_simple::store_date(TIME *tm)
 
 bool Protocol_simple::store_time(TIME *tm)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
 	      field_types[field_pos] == MYSQL_TYPE_TIME);
   field_pos++;
@@ -1084,7 +1084,7 @@ bool Protocol_prep::store_longlong(longlong from, bool unsigned_flag)
 
 bool Protocol_prep::store_decimal(const my_decimal *d)
 {
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   DBUG_ASSERT(field_types == 0 ||
               field_types[field_pos] == MYSQL_TYPE_NEWDECIMAL);
   field_pos++;
diff --git a/sql/protocol.h b/sql/protocol.h
index c00bbba4cc9..8d9da5774b2 100644
--- a/sql/protocol.h
+++ b/sql/protocol.h
@@ -31,7 +31,7 @@ protected:
   String *packet;
   String *convert;
   uint field_pos;
-#ifndef DEBUG_OFF
+#ifndef DBUG_OFF
   enum enum_field_types *field_types;
 #endif
   uint field_count;
diff --git a/sql/sp.cc b/sql/sp.cc
index 4f7b544f5c7..4a2da1bb8e6 100644
--- a/sql/sp.cc
+++ b/sql/sp.cc
@@ -380,6 +380,7 @@ db_find_routine(THD *thd, int type, sp_name *name, sp_head **sphp)
   {
     String defstr;
     LEX *oldlex= thd->lex;
+    sp_rcontext *save_spcont= thd->spcont;
     char olddb[128];
     bool dbchanged;
     enum enum_sql_command oldcmd= thd->lex->sql_command;
@@ -422,6 +423,7 @@ db_find_routine(THD *thd, int type, sp_name *name, sp_head **sphp)
       thd->lex->found_semicolon= tmpfsc;
     }
 
+    thd->spcont= 0;
     if (yyparse(thd) || thd->is_fatal_error || thd->lex->sphead == NULL)
     {
       LEX *newlex= thd->lex;
@@ -439,12 +441,14 @@ db_find_routine(THD *thd, int type, sp_name *name, sp_head **sphp)
     else
     {
       if (dbchanged && (ret= mysql_change_db(thd, olddb, 1)))
-	goto done;
+	goto db_done;
       *sphp= thd->lex->sphead;
       (*sphp)->set_info((char *)definer, (uint)strlen(definer),
 			created, modified, &chistics, sql_mode);
       (*sphp)->optimize();
     }
+db_done:
+    thd->spcont= save_spcont;
     thd->lex->sql_command= oldcmd;
     thd->variables.sql_mode= old_sql_mode;
     thd->variables.select_limit= select_limit;
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 671acbc2a0c..3e25544839f 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -1112,6 +1112,9 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   // QQ Should have some error checking here? (types, etc...)
   if (!(nctx= new sp_rcontext(csize, hmax, cmax)))
     goto end;
+#ifndef DBUG_OFF
+  nctx->owner= this;
+#endif
   for (i= 0 ; i < argcount ; i++)
   {
     sp_pvar_t *pvar = m_pcont->find_pvar(i);
@@ -1256,6 +1259,9 @@ int sp_head::execute_procedure(THD *thd, List<Item> *args)
   {				// Create a temporary old context
     if (!(octx= new sp_rcontext(csize, hmax, cmax)))
       DBUG_RETURN(-1);
+#ifndef DBUG_OFF
+    octx->owner= 0;
+#endif
     thd->spcont= octx;
 
     /* set callers_arena to thd, for upper-level function to work */
@@ -1267,6 +1273,9 @@ int sp_head::execute_procedure(THD *thd, List<Item> *args)
     thd->spcont= save_spcont;
     DBUG_RETURN(-1);
   }
+#ifndef DBUG_OFF
+  nctx->owner= this;
+#endif
 
   if (csize > 0 || hmax > 0 || cmax > 0)
   {
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index 22fa4f6e865..2988793083e 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -66,6 +66,14 @@ class sp_rcontext : public Sql_alloc
   */
   Query_arena *callers_arena;
 
+#ifndef DBUG_OFF
+  /*
+    Routine to which this Item_splocal belongs. Used for checking if correct
+    runtime context is used for variable handling.
+  */
+  sp_head *owner;
+#endif
+
   sp_rcontext(uint fsize, uint hmax, uint cmax);
 
   ~sp_rcontext()
diff --git a/sql/sql_cache.h b/sql/sql_cache.h
index 123d16b606d..69a0d6cd05d 100644
--- a/sql/sql_cache.h
+++ b/sql/sql_cache.h
@@ -410,7 +410,7 @@ protected:
 
   /*
     The following functions are only used when debugging
-    We don't protect these with ifndef DEBUG_OFF to not have to recompile
+    We don't protect these with ifndef DBUG_OFF to not have to recompile
     everything if we want to add checks of the cache at some places.
   */
   void wreck(uint line, const char *message);
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 2917626ff35..32bbb456689 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -1490,7 +1490,13 @@ int select_dumpvar::prepare(List<Item> &list, SELECT_LEX_UNIT *u)
   {
     my_var *mv= gl++;
     if (mv->local)
-      (void)local_vars.push_back(new Item_splocal(mv->s, mv->offset));
+    {
+      Item_splocal *var;
+      (void)local_vars.push_back(var= new Item_splocal(mv->s, mv->offset));
+#ifndef DEBUG_OFF
+      var->owner= mv->owner;
+#endif
+    }
     else
     {
       Item_func_set_user_var *var= new Item_func_set_user_var(mv->s, item);
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 7cbfc19123f..56cb606558d 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -2075,6 +2075,13 @@ public:
 class my_var : public Sql_alloc  {
 public:
   LEX_STRING s;
+#ifndef DEBUG_OFF
+  /*
+    Routine to which this Item_splocal belongs. Used for checking if correct
+    runtime context is used for variable handling.
+  */
+  sp_head *owner;
+#endif
   bool local;
   uint offset;
   enum_field_types type;
diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
index df8de59508d..27bee87c012 100644
--- a/sql/sql_trigger.cc
+++ b/sql/sql_trigger.cc
@@ -646,6 +646,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db,
       char *trg_name_buff;
       List_iterator_fast<ulonglong> itm(triggers->definition_modes_list);
       LEX *old_lex= thd->lex, lex;
+      sp_rcontext *save_spcont= thd->spcont;
       ulong save_sql_mode= thd->variables.sql_mode;
 
       thd->lex= &lex;
@@ -660,6 +661,7 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db,
         thd->variables.sql_mode= (ulong)*trg_sql_mode;
         lex_start(thd, (uchar*)trg_create_str->str, trg_create_str->length);
 
+	thd->spcont= 0;
         if (yyparse((void *)thd) || thd->is_fatal_error)
         {
           /*
@@ -712,6 +714,7 @@ err_with_lex_cleanup:
       // QQ: anything else ?
       lex_end(&lex);
       thd->lex= old_lex;
+      thd->spcont= save_spcont;
       thd->variables.sql_mode= save_sql_mode;
       thd->db= save_db.str;
       thd->db_length= save_db.length;
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index f28fbe5c803..298c282d625 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -2351,8 +2351,12 @@ sp_case:
 
 	      ivar.str= (char *)"_tmp_";
 	      ivar.length= 5;
-	      Item *var= (Item*) new Item_splocal(ivar, 
-						  ctx->current_pvars()-1);
+	      Item_splocal *var= new Item_splocal(ivar,
+                                                  ctx->current_pvars()-1);
+#ifndef DEBUG_OFF
+              if (var)
+                var->owner= sp;
+#endif
 	      Item *expr= new Item_func_eq(var, $2);
 
 	      i= new sp_instr_jump_if_not(ip, ctx, expr, lex);
@@ -5925,7 +5929,13 @@ select_var_ident:
 	       YYABORT;
 	     else
 	     {
-	       ((select_dumpvar *)lex->result)->var_list.push_back( new my_var($1,1,t->offset,t->type));
+               my_var *var;
+	       ((select_dumpvar *)lex->result)->
+                 var_list.push_back(var= new my_var($1,1,t->offset,t->type));
+#ifndef DEBUG_OFF
+	       if (var)
+		 var->owner= lex->sphead;
+#endif
 	     }
 	   }
            ;
@@ -7224,6 +7234,10 @@ simple_ident:
             Item_splocal *splocal;
             splocal= new Item_splocal($1, spv->offset, lex->tok_start_prev - 
                                       lex->sphead->m_tmp_query);
+#ifndef DEBUG_OFF
+            if (splocal)
+              splocal->owner= lex->sphead;
+#endif
 	    $$ = (Item*) splocal;
             lex->variables_used= 1;
 	    lex->safe_to_cache_query=0;