Commit 898adb8c authored by unknown's avatar unknown
Browse files

Fixed BUG#14498: Stored procedures: hang if undefined variable and exception 

  The problem was to continue at the right place in the code after the
  test expression in a flow control statement fails with an exception
  (internally, the test in sp_instr_jump_if_not), and the exception is
  caught by a continue handler. Execution must then be resumed after the
  the entire flow control statement (END IF, END WHILE, etc).


mysql-test/r/sp.result:
  New test case for BUG#14498.
mysql-test/t/sp.test:
  New test case for BUG#14498.
  (Note that one call is disabled at the moment. Depends on BUG#14643.)
sql/sp_head.cc:
  Added a continuation destination for sp_instr_jump_if_not, for the case when
  an error in the test expression causes a continue handler to catch.
  This includes new members in sp_instr_jump_if_not, adjustment of the optmizer
  (mark and move methods), and separate backpatching code (since we can't use
  the normal one for this).
  
  Also removed the class sp_instr_jump, since it's never used.
  
  ...and added some comments to the optimizer.
sql/sp_head.h:
  Added a continuation destination for sp_instr_jump_if_not, for the case when
  an error in the test expression causes a continue handler to catch.
  This includes new members in sp_instr_jump_if_not, adjustment of the optmizer
  (mark and move methods), and separate backpatching code (since we can't use
  the normal one for this).
  
  Also removed the class sp_instr_jump, since it's never used.
sql/sql_yacc.yy:
  Added backpatching of the continue destination for all conditional statements
  (using sp_instr_jump_if_not).
parent 02ac7bef

mysql-test/r/sp.result

+84 −0
Original line number Diff line number Diff line
@@ -3575,4 +3575,88 @@ DROP VIEW bug13095_v1 
DROP PROCEDURE IF EXISTS bug13095;

DROP VIEW IF EXISTS bug13095_v1;

DROP TABLE IF EXISTS bug13095_t1;

drop procedure if exists bug14498_1|

drop procedure if exists bug14498_2|

drop procedure if exists bug14498_3|

drop procedure if exists bug14498_4|

drop procedure if exists bug14498_5|

create procedure bug14498_1()

begin

declare continue handler for sqlexception select 'error' as 'Handler';

if v then

select 'yes' as 'v';

else

select 'no' as 'v';

end if;

select 'done' as 'End';

end|

create procedure bug14498_2()

begin

declare continue handler for sqlexception select 'error' as 'Handler';

while v do

select 'yes' as 'v';

end while;

select 'done' as 'End';

end|

create procedure bug14498_3()

begin

declare continue handler for sqlexception select 'error' as 'Handler';

repeat

select 'maybe' as 'v';

until v end repeat;

select 'done' as 'End';

end|

create procedure bug14498_4()

begin

declare continue handler for sqlexception select 'error' as 'Handler';

case v

when 1 then

select '1' as 'v';

when 2 then

select '2' as 'v';

else

select '?' as 'v';

end case;

select 'done' as 'End';

end|

create procedure bug14498_5()

begin

declare continue handler for sqlexception select 'error' as 'Handler';

case

when v = 1 then

select '1' as 'v';

when v = 2 then

select '2' as 'v';

else

select '?' as 'v';

end case;

select 'done' as 'End';

end|

call bug14498_1()|

Handler

error

End

done

call bug14498_2()|

Handler

error

End

done

call bug14498_3()|

v

maybe

Handler

error

End

done

call bug14498_5()|

Handler

error

End

done

drop procedure bug14498_1|

drop procedure bug14498_2|

drop procedure bug14498_3|

drop procedure bug14498_4|

drop procedure bug14498_5|

drop table t1,t2;

mysql-test/t/sp.test

+86 −0
Original line number Diff line number Diff line
@@ -4489,6 +4489,92 @@ DROP TABLE IF EXISTS bug13095_t1; 
delimiter |;





#

# BUG#14498: Stored procedures: hang if undefined variable and exception

#

--disable_warnings

drop procedure if exists bug14498_1|

drop procedure if exists bug14498_2|

drop procedure if exists bug14498_3|

drop procedure if exists bug14498_4|

drop procedure if exists bug14498_5|

--enable_warnings



create procedure bug14498_1()

begin

  declare continue handler for sqlexception select 'error' as 'Handler';



  if v then

    select 'yes' as 'v';

  else

    select 'no' as 'v';

  end if;

  select 'done' as 'End';

end|



create procedure bug14498_2()

begin

  declare continue handler for sqlexception select 'error' as 'Handler';



  while v do

    select 'yes' as 'v';

  end while;

  select 'done' as 'End';

end|



create procedure bug14498_3()

begin

  declare continue handler for sqlexception select 'error' as 'Handler';



  repeat

    select 'maybe' as 'v';

  until v end repeat;

  select 'done' as 'End';

end|



create procedure bug14498_4()

begin

  declare continue handler for sqlexception select 'error' as 'Handler';



  case v

  when 1 then

    select '1' as 'v';

  when 2 then

    select '2' as 'v';

  else

    select '?' as 'v';

  end case;

  select 'done' as 'End';

end|



create procedure bug14498_5()

begin

  declare continue handler for sqlexception select 'error' as 'Handler';



  case

  when v = 1 then

    select '1' as 'v';

  when v = 2 then

    select '2' as 'v';

  else

    select '?' as 'v';

  end case;

  select 'done' as 'End';

end|



call bug14498_1()|

call bug14498_2()|

call bug14498_3()|

# QQ We can't call this at the moment, due to a known bug (BUG#14643)

#call bug14498_4()|

call bug14498_5()|



drop procedure bug14498_1|

drop procedure bug14498_2|

drop procedure bug14498_3|

drop procedure bug14498_4|

drop procedure bug14498_5|



#

# BUG#NNNN: New bug synopsis

#

sql/sp_head.cc

+74 −66
Original line number Diff line number Diff line
@@ -437,13 +437,14 @@ sp_head::operator delete(void *ptr, size_t size) 


sp_head::sp_head()

  :Query_arena(&main_mem_root, INITIALIZED_FOR_SP),

   m_flags(0), m_returns_cs(NULL)

   m_flags(0), m_returns_cs(NULL), m_cont_level(0)

{

  extern byte *

    sp_table_key(const byte *ptr, uint *plen, my_bool first);

  DBUG_ENTER("sp_head::sp_head");



  m_backpatch.empty();

  m_cont_backpatch.empty();

  m_lex.empty();

  hash_init(&m_sptabs, system_charset_info, 0, 0, 0, sp_table_key, 0, 0);

  hash_init(&m_sroutines, system_charset_info, 0, 0, 0, sp_sroutine_key, 0, 0);
@@ -1568,6 +1569,39 @@ sp_head::check_backpatch(THD *thd) 
  return 0;

}



void

sp_head::new_cont_backpatch(sp_instr_jump_if_not *i)

{

  m_cont_level+= 1;

  if (i)

  {

    /* Use the cont. destination slot to store the level */

    i->m_cont_dest= m_cont_level;

    (void)m_cont_backpatch.push_front(i);

  }

}



void

sp_head::add_cont_backpatch(sp_instr_jump_if_not *i)

{

  i->m_cont_dest= m_cont_level;

  (void)m_cont_backpatch.push_front(i);

}



void

sp_head::do_cont_backpatch()

{

  uint dest= instructions();

  uint lev= m_cont_level--;

  sp_instr_jump_if_not *i;



  while ((i= m_cont_backpatch.head()) && i->m_cont_dest == lev)

  {

    i->m_cont_dest= dest;

    (void)m_cont_backpatch.pop();

  }

}



void

sp_head::set_info(char *definer, uint definerlen,

		  longlong created, longlong modified,
@@ -1782,7 +1816,10 @@ sp_head::show_create_function(THD *thd) 




/*

  TODO: what does this do??

  Do some minimal optimization of the code:

    1) Mark used instructions

       1.1) While doing this, shortcut jumps to jump instructions

    2) Compact the code, removing unused instructions

*/



void sp_head::optimize()
@@ -1805,7 +1842,7 @@ void sp_head::optimize() 
    else

    {

      if (src != dst)

      {

      {                         // Move the instruction and update prev. jumps

	sp_instr *ibp;

	List_iterator_fast<sp_instr> li(bp);
@@ -1813,8 +1850,7 @@ void sp_head::optimize() 
	while ((ibp= li++))

	{

	  sp_instr_jump *ji= static_cast<sp_instr_jump *>(ibp);

	  if (ji->m_dest == src)

	    ji->m_dest= dst;

          ji->set_destination(src, dst);

	}

      }

      i->opt_move(dst, &bp);
@@ -2144,65 +2180,6 @@ sp_instr_jump::opt_move(uint dst, List<sp_instr> *bp) 
}





/*

  sp_instr_jump_if class functions

*/



int

sp_instr_jump_if::execute(THD *thd, uint *nextp)

{

  DBUG_ENTER("sp_instr_jump_if::execute");

  DBUG_PRINT("info", ("destination: %u", m_dest));

  DBUG_RETURN(m_lex_keeper.reset_lex_and_exec_core(thd, nextp, TRUE, this));

}



int

sp_instr_jump_if::exec_core(THD *thd, uint *nextp)

{

  Item *it;

  int res;



  it= sp_prepare_func_item(thd, &m_expr);

  if (!it)

    res= -1;

  else

  {

    res= 0;

    if (it->val_bool())

      *nextp = m_dest;

    else

      *nextp = m_ip+1;

  }



  return res;

}



void

sp_instr_jump_if::print(String *str)

{

  str->reserve(12);

  str->append("jump_if ");

  str->qs_append(m_dest);

  str->append(' ');

  m_expr->print(str);

}



uint

sp_instr_jump_if::opt_mark(sp_head *sp)

{

  sp_instr *i;



  marked= 1;

  if ((i= sp->get_instr(m_dest)))

  {

    m_dest= i->opt_shortcut_jump(sp, this);

    m_optdest= sp->get_instr(m_dest);

  }

  sp->opt_mark(m_dest);

  return m_ip+1;

}





/*

  sp_instr_jump_if_not class functions

*/
@@ -2224,7 +2201,10 @@ sp_instr_jump_if_not::exec_core(THD *thd, uint *nextp) 


  it= sp_prepare_func_item(thd, &m_expr);

  if (! it)

  {

    res= -1;

    *nextp = m_cont_dest;

  }

  else

  {

    res= 0;
@@ -2241,10 +2221,12 @@ sp_instr_jump_if_not::exec_core(THD *thd, uint *nextp) 
void

sp_instr_jump_if_not::print(String *str)

{

  str->reserve(16);

  str->reserve(24);

  str->append("jump_if_not ");

  str->qs_append(m_dest);

  str->append(' ');

  str->append('(');

  str->qs_append(m_cont_dest);

  str->append(") ");

  m_expr->print(str);

}
@@ -2261,9 +2243,35 @@ sp_instr_jump_if_not::opt_mark(sp_head *sp) 
    m_optdest= sp->get_instr(m_dest);

  }

  sp->opt_mark(m_dest);

  if ((i= sp->get_instr(m_cont_dest)))

  {

    m_cont_dest= i->opt_shortcut_jump(sp, this);

    m_cont_optdest= sp->get_instr(m_cont_dest);

  }

  sp->opt_mark(m_cont_dest);

  return m_ip+1;

}



void

sp_instr_jump_if_not::opt_move(uint dst, List<sp_instr> *bp)

{

  /*

    cont. destinations may point backwards after shortcutting jumps

    during the mark phase. If it's still pointing forwards, only

    push this for backpatching if sp_instr_jump::opt_move() will not

    do it (i.e. if the m_dest points backwards).

   */

  if (m_cont_dest > m_ip)

  {                             // Forward

    if (m_dest < m_ip)

      bp->push_back(this);

  }

  else if (m_cont_optdest)

    m_cont_dest= m_cont_optdest->m_ip; // Backward

  /* This will take care of m_dest and m_ip */

  sp_instr_jump::opt_move(dst, bp);

}





/*

  sp_instr_freturn class functions

sql/sp_head.h

+47 −41
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ sp_get_flags_for_command(LEX *lex); 


struct sp_label;

class sp_instr;

class sp_instr_jump_if_not;

struct sp_cond_type;

struct sp_pvar;
@@ -240,6 +241,18 @@ class sp_head :private Query_arena 
  int

  check_backpatch(THD *thd);



  // Start a new cont. backpatch level. If 'i' is NULL, the level is just incr.

  void

  new_cont_backpatch(sp_instr_jump_if_not *i);



  // Add an instruction to the current level

  void

  add_cont_backpatch(sp_instr_jump_if_not *i);



  // Backpatch (and pop) the current level to the current position.

  void

  do_cont_backpatch();



  char *name(uint *lenp = 0) const

  {

    if (lenp)
@@ -309,6 +322,18 @@ class sp_head :private Query_arena 
    sp_instr *instr;

  } bp_t;

  List<bp_t> m_backpatch;	// Instructions needing backpatching

  /*

    We need a special list for backpatching of conditional jump's continue

    destination (in the case of a continue handler catching an error in

    the test), since it would otherwise interfere with the normal backpatch

    mechanism - jump_if_not instructions have two different destination

    which are to be patched differently.

    Since these occur in a more restricted way (always the same "level" in

    the code), we don't need the label.

   */

  List<sp_instr_jump_if_not> m_cont_backpatch;

  uint m_cont_level;            // The current cont. backpatch level



  /*

    Multi-set representing optimized list of tables to be locked by this

    routine. Does not include tables which are used by invoked routines.
@@ -622,50 +647,17 @@ class sp_instr_jump : public sp_instr 
      m_dest= dest;

  }



protected:



  sp_instr *m_optdest;		// Used during optimization



}; // class sp_instr_jump : public sp_instr





class sp_instr_jump_if : public sp_instr_jump

{

  sp_instr_jump_if(const sp_instr_jump_if &); /* Prevent use of these */

  void operator=(sp_instr_jump_if &);



public:



  sp_instr_jump_if(uint ip, sp_pcontext *ctx, Item *i, LEX *lex)

    : sp_instr_jump(ip, ctx), m_expr(i), m_lex_keeper(lex, TRUE)

  {}



  sp_instr_jump_if(uint ip, sp_pcontext *ctx, Item *i, uint dest, LEX *lex)

    : sp_instr_jump(ip, ctx, dest), m_expr(i), m_lex_keeper(lex, TRUE)

  {}



  virtual ~sp_instr_jump_if()

  {}



  virtual int execute(THD *thd, uint *nextp);



  virtual int exec_core(THD *thd, uint *nextp);



  virtual void print(String *str);



  virtual uint opt_mark(sp_head *sp);



  virtual uint opt_shortcut_jump(sp_head *sp, sp_instr *start)

  virtual void set_destination(uint old_dest, uint new_dest)

  {

    return m_ip;

    if (m_dest == old_dest)

      m_dest= new_dest;

  }



private:

protected:



  Item *m_expr;			// The condition

  sp_lex_keeper m_lex_keeper;

  sp_instr *m_optdest;		// Used during optimization



}; // class sp_instr_jump_if : public sp_instr_jump

}; // class sp_instr_jump : public sp_instr





class sp_instr_jump_if_not : public sp_instr_jump
@@ -675,12 +667,16 @@ class sp_instr_jump_if_not : public sp_instr_jump 


public:



  uint m_cont_dest;             // Where continue handlers will go



  sp_instr_jump_if_not(uint ip, sp_pcontext *ctx, Item *i, LEX *lex)

    : sp_instr_jump(ip, ctx), m_expr(i), m_lex_keeper(lex, TRUE)

    : sp_instr_jump(ip, ctx), m_cont_dest(0), m_expr(i),

      m_lex_keeper(lex, TRUE), m_cont_optdest(0)

  {}



  sp_instr_jump_if_not(uint ip, sp_pcontext *ctx, Item *i, uint dest, LEX *lex)

    : sp_instr_jump(ip, ctx, dest), m_expr(i), m_lex_keeper(lex, TRUE)

    : sp_instr_jump(ip, ctx, dest), m_cont_dest(0), m_expr(i),

      m_lex_keeper(lex, TRUE), m_cont_optdest(0)

  {}



  virtual ~sp_instr_jump_if_not()
@@ -699,10 +695,20 @@ class sp_instr_jump_if_not : public sp_instr_jump 
    return m_ip;

  }



  virtual void opt_move(uint dst, List<sp_instr> *ibp);



  virtual void set_destination(uint old_dest, uint new_dest)

  {

    sp_instr_jump::set_destination(old_dest, new_dest);

    if (m_cont_dest == old_dest)

      m_cont_dest= new_dest;

  }



private:



  Item *m_expr;			// The condition

  sp_lex_keeper m_lex_keeper;

  sp_instr *m_cont_optdest;     // Used during optimization



}; // class sp_instr_jump_if_not : public sp_instr_jump

sql/sql_yacc.yy

+17 −3
Original line number Diff line number Diff line
@@ -2009,14 +2009,21 @@ sp_proc_stmt: 
	    }

	    sp->restore_lex(YYTHD);

	  }

	| IF sp_if END IF {}

        | IF

          { Lex->sphead->new_cont_backpatch(NULL); }

          sp_if END IF

          { Lex->sphead->do_cont_backpatch(); }

	| CASE_SYM WHEN_SYM

	  {

	    Lex->sphead->m_flags&= ~sp_head::IN_SIMPLE_CASE;

            Lex->sphead->new_cont_backpatch(NULL);

	  }

	  sp_case END CASE_SYM {}

          sp_case END CASE_SYM { Lex->sphead->do_cont_backpatch(); }

        | CASE_SYM

          { Lex->sphead->reset_lex(YYTHD); }

          {

            Lex->sphead->reset_lex(YYTHD);

            Lex->sphead->new_cont_backpatch(NULL);

          }

          expr WHEN_SYM

	  {

	    /* We "fake" this by using an anonymous variable which we
@@ -2038,6 +2045,7 @@ sp_proc_stmt: 
	  sp_case END CASE_SYM

	  {

	    Lex->spcont->pop_pvar();

            Lex->sphead->do_cont_backpatch();

	  }

	| sp_labeled_control

	  {}
@@ -2306,6 +2314,7 @@ sp_if: 
                                                               $2, lex);



	    sp->push_backpatch(i, ctx->push_label((char *)"", 0));

            sp->add_cont_backpatch(i);

            sp->add_instr(i);

            sp->restore_lex(YYTHD);

	  }
@@ -2360,6 +2369,7 @@ sp_case: 
              lex->variables_used= 1;

	    }

	    sp->push_backpatch(i, ctx->push_label((char *)"", 0));

            sp->add_cont_backpatch(i);

            sp->add_instr(i);

            sp->restore_lex(YYTHD);

	  }
@@ -2489,6 +2499,7 @@ sp_unlabeled_control: 


	    /* Jumping forward */

	    sp->push_backpatch(i, lex->spcont->last_label());

            sp->new_cont_backpatch(i);

            sp->add_instr(i);

            sp->restore_lex(YYTHD);

	  }
@@ -2500,6 +2511,7 @@ sp_unlabeled_control: 
	    sp_instr_jump *i = new sp_instr_jump(ip, lex->spcont, lab->ip);



	    lex->sphead->add_instr(i);

            lex->sphead->do_cont_backpatch();

	  }

        | REPEAT_SYM sp_proc_stmts1 UNTIL_SYM 

          { Lex->sphead->reset_lex(YYTHD); }
@@ -2513,6 +2525,8 @@ sp_unlabeled_control: 
                                                               lex);

            lex->sphead->add_instr(i);

            lex->sphead->restore_lex(YYTHD);

            /* We can shortcut the cont_backpatch here */

            i->m_cont_dest= ip+1;

	  }

	;