database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2026-10-23 18:49:29 +03:00
Code Activity

MDEV-37563 Crash on a FOR loop with a row comparison in the upper bound

Browse Source 
Both bounds of a FOR loop, and all parameters of a cursor FOR loop,
are parsed before their instructions are added. add_instr() then gave
all their items to the first instruction, and in a trigger also all
NEW and OLD fields. So a row comparison used by a later instruction
was cleaned up by the wrong one, on the next CALL, after its memory
had already been freed. This crashed in Arg_comparator::cleanup().

Give each instruction only the items of its own expression, and its
own trigger fields. To find them, cut THD::free_list at the newest
item of each expression, starting with the oldest expression.
This commit is contained in:
Khaled Riyad
2026-09-23 18:10:45 +03:00
parent 8ee83e87b9
commit 9be46dd6f1
5 changed files with 409 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

149
mysql-test/main/sp-for-loop.result
View File

@@ -236,3 +236,152 @@ DROP PROCEDURE forIterateBug;
#
# End of 10.4 tests
#
#
# MDEV-37563 Crash on a FOR loop with a row comparison in the upper bound
#
CREATE OR REPLACE PROCEDURE p1()
BEGIN
DECLARE i INT;
FOR i IN 0..((1,'b1')=(1,'b1')) DO
SELECT i;
END FOR;
END;
$$
CALL p1;
i
0
i
1
CALL p1;
i
0
i
1
DROP PROCEDURE p1;
# The same for a cursor FOR loop parameter
CREATE OR REPLACE PROCEDURE p2()
BEGIN
DECLARE c CURSOR (p INT) FOR SELECT p;
FOR r IN c((1,'b1')=(1,'b1')) DO
SELECT r.p;
END FOR;
END;
$$
CALL p2;
r.p
1
CALL p2;
r.p
1
DROP PROCEDURE p2;
# The same for a cursor FOR loop with more than one parameter
CREATE OR REPLACE PROCEDURE p3()
BEGIN
DECLARE c CURSOR (a INT, b INT) FOR SELECT a, b;
FOR r IN c(1, (1,'b1')=(1,'b1')) DO
SELECT r.a, r.b;
END FOR;
END;
$$
CALL p3;
r.a r.b
1 1
CALL p3;
r.a r.b
1 1
DROP PROCEDURE p3;
# The same with a row comparison in more than one parameter
CREATE OR REPLACE PROCEDURE p4()
BEGIN
DECLARE c CURSOR (p1 INT, p2 INT, p3 INT) FOR SELECT p1, p2, p3;
FOR r IN c((1,'a')=(1,'a'), 7, (2,'b')=(2,'c')) DO
SELECT r.p1, r.p2, r.p3;
END FOR;
END;
$$
CALL p4;
r.p1 r.p2 r.p3
1 7 0
CALL p4;
r.p1 r.p2 r.p3
1 7 0
DROP PROCEDURE p4;
# The same with a row comparison in a derived table of the lower bound
CREATE OR REPLACE PROCEDURE p5()
BEGIN
FOR i IN 0+(SELECT z FROM (SELECT (1,'b1')=(1,'b1') AS z) dt)..2 DO
SELECT i;
END FOR;
END;
$$
CALL p5;
i
1
i
2
CALL p5;
i
1
i
2
DROP PROCEDURE p5;
# The same for a REVERSE loop, whose target bound is the left one
CREATE OR REPLACE PROCEDURE p6()
BEGIN
FOR i IN REVERSE 0+((1,'b1')=(1,'b1'))..2 DO
SELECT i;
END FOR;
END;
$$
CALL p6;
i
2
i
1
CALL p6;
i
2
i
1
DROP PROCEDURE p6;
# A trigger re-parses the upper bound after ALTER TABLE
CREATE TABLE t1 (a INT);
CREATE TABLE t2 (b INT);
INSERT INTO t2 VALUES (1);
CREATE TRIGGER tr BEFORE INSERT ON t1 FOR EACH ROW
BEGIN
FOR i IN 0..(SELECT NEW.a + COUNT(*) FROM t2) DO
SET @x= i;
END FOR;
END;
$$
INSERT INTO t1 VALUES (1);
ALTER TABLE t2 ADD COLUMN c INT;
INSERT INTO t1 VALUES (2);
INSERT INTO t1 VALUES (3);
SELECT @x;
@x
4
DROP TABLE t1, t2;
# A trigger with NEW fields in more than one cursor parameter
CREATE TABLE t1 (a INT, b INT);
CREATE TRIGGER tr BEFORE INSERT ON t1 FOR EACH ROW
BEGIN
DECLARE c CURSOR (p1 INT, p2 INT) FOR SELECT p1, p2;
FOR r IN c(NEW.a, NEW.b + ((NEW.a,'x')=(NEW.a,'x'))) DO
SET @x= CONCAT(r.p1, ',', r.p2);
END FOR;
END;
$$
INSERT INTO t1 VALUES (1, 10);
SELECT @x;
@x
1,11
INSERT INTO t1 VALUES (2, 20);
SELECT @x;
@x
2,21
DROP TABLE t1;
#
# End of 11.4 tests
#

142
mysql-test/main/sp-for-loop.test
View File

@@ -248,3 +248,145 @@ DROP PROCEDURE forIterateBug;
--echo #
--echo # End of 10.4 tests
--echo #
--echo #
--echo # MDEV-37563 Crash on a FOR loop with a row comparison in the upper bound
--echo #
DELIMITER $$;
CREATE OR REPLACE PROCEDURE p1()
BEGIN
DECLARE i INT;
FOR i IN 0..((1,'b1')=(1,'b1')) DO
SELECT i;
END FOR;
END;
$$
DELIMITER ;$$
CALL p1;
CALL p1;
DROP PROCEDURE p1;
--echo # The same for a cursor FOR loop parameter
DELIMITER $$;
CREATE OR REPLACE PROCEDURE p2()
BEGIN
DECLARE c CURSOR (p INT) FOR SELECT p;
FOR r IN c((1,'b1')=(1,'b1')) DO
SELECT r.p;
END FOR;
END;
$$
DELIMITER ;$$
CALL p2;
CALL p2;
DROP PROCEDURE p2;
--echo # The same for a cursor FOR loop with more than one parameter
DELIMITER $$;
CREATE OR REPLACE PROCEDURE p3()
BEGIN
DECLARE c CURSOR (a INT, b INT) FOR SELECT a, b;
FOR r IN c(1, (1,'b1')=(1,'b1')) DO
SELECT r.a, r.b;
END FOR;
END;
$$
DELIMITER ;$$
CALL p3;
CALL p3;
DROP PROCEDURE p3;
--echo # The same with a row comparison in more than one parameter
DELIMITER $$;
CREATE OR REPLACE PROCEDURE p4()
BEGIN
DECLARE c CURSOR (p1 INT, p2 INT, p3 INT) FOR SELECT p1, p2, p3;
FOR r IN c((1,'a')=(1,'a'), 7, (2,'b')=(2,'c')) DO
SELECT r.p1, r.p2, r.p3;
END FOR;
END;
$$
DELIMITER ;$$
CALL p4;
CALL p4;
DROP PROCEDURE p4;
--echo # The same with a row comparison in a derived table of the lower bound
DELIMITER $$;
CREATE OR REPLACE PROCEDURE p5()
BEGIN
FOR i IN 0+(SELECT z FROM (SELECT (1,'b1')=(1,'b1') AS z) dt)..2 DO
SELECT i;
END FOR;
END;
$$
DELIMITER ;$$
CALL p5;
CALL p5;
DROP PROCEDURE p5;
--echo # The same for a REVERSE loop, whose target bound is the left one
DELIMITER $$;
CREATE OR REPLACE PROCEDURE p6()
BEGIN
FOR i IN REVERSE 0+((1,'b1')=(1,'b1'))..2 DO
SELECT i;
END FOR;
END;
$$
DELIMITER ;$$
CALL p6;
CALL p6;
DROP PROCEDURE p6;
--echo # A trigger re-parses the upper bound after ALTER TABLE
CREATE TABLE t1 (a INT);
CREATE TABLE t2 (b INT);
INSERT INTO t2 VALUES (1);
DELIMITER $$;
CREATE TRIGGER tr BEFORE INSERT ON t1 FOR EACH ROW
BEGIN
FOR i IN 0..(SELECT NEW.a + COUNT(*) FROM t2) DO
SET @x= i;
END FOR;
END;
$$
DELIMITER ;$$
INSERT INTO t1 VALUES (1);
ALTER TABLE t2 ADD COLUMN c INT;
INSERT INTO t1 VALUES (2);
INSERT INTO t1 VALUES (3);
SELECT @x;
DROP TABLE t1, t2;
--echo # A trigger with NEW fields in more than one cursor parameter
CREATE TABLE t1 (a INT, b INT);
DELIMITER $$;
CREATE TRIGGER tr BEFORE INSERT ON t1 FOR EACH ROW
BEGIN
DECLARE c CURSOR (p1 INT, p2 INT) FOR SELECT p1, p2;
FOR r IN c(NEW.a, NEW.b + ((NEW.a,'x')=(NEW.a,'x'))) DO
SET @x= CONCAT(r.p1, ',', r.p2);
END FOR;
END;
$$
DELIMITER ;$$
INSERT INTO t1 VALUES (1, 10);
SELECT @x;
INSERT INTO t1 VALUES (2, 20);
SELECT @x;
DROP TABLE t1;
--echo #
--echo # End of 11.4 tests
--echo #

93
sql/sp_head.cc
View File

@@ -3210,6 +3210,74 @@ int sp_head::add_instr(sp_instr_cursor_copy_struct *instr)
}
/**
In a trigger, add_instr() also takes the NEW and OLD fields in
m_cur_instr_trig_field_items. Rebuild this list from THD::free_list
after a split, so the fields go with their items.
*/
static void set_trig_field_items(THD *thd, sp_head *sp)
{
if (sp->m_handler->type() != SP_TYPE_TRIGGER)
return;
sp->m_cur_instr_trig_field_items.empty();
for (Item *item= thd->free_list; item; item= item->next)
{
if (item->type() == Item::TRIGGER_FIELD_ITEM)
{
Item_trigger_field *field= static_cast<Item_trigger_field*>(item);
sp->m_cur_instr_trig_field_items.link_in_list(field,
&field->next_trg_field);
}
}
}
/**
Move the items of an expression from an item list to THD::free_list.
The list starts with the newest item, and the parser creates the parts
of an item before the item itself. So the first item in the list that
walk() finds in expr, and all items after it, belong to expr, even
those walk() does not visit, like the LIMIT of a subquery.
@param thd Thread handle
@param list Item list to take the items from
@param expr Root of the oldest expression that has items in list
*/
void sp_head::move_expr_items_to_free_list(THD *thd, Item **list, Item *expr)
{
DBUG_ASSERT(!thd->free_list || thd->free_list == *list);
Item **pos= list;
while (*pos && !expr->walk(&Item::find_item_processor, true, *pos))
pos= &(*pos)->next;
DBUG_ASSERT(*pos); // expr itself is always found
thd->free_list= *pos;
*pos= NULL;
set_trig_field_items(thd, this);
}
/**
Put an item list back on THD::free_list, so that the next added
instruction owns the items, or on an error the routine frees them.
*/
void sp_head::add_to_free_list(THD *thd, Item *items)
{
if (items)
{
Item *last= items;
while (last->next)
last= last->next;
last->next= thd->free_list;
thd->free_list= items;
}
set_trig_field_items(thd, this);
}
/**
Add instruction to SP.
@@ -3971,6 +4039,15 @@ sp_head::add_set_for_loop_cursor_param_variables(THD *thd,
Item_args *parameters)
{
DBUG_ASSERT(param_spcont->context_var_count() == parameters->argument_count());
/*
All parameters were parsed as one expression, so THD::free_list holds:
[items of no parameter] [items of arg N-1] ... [items of arg 0]
Give each sp_instr_set the items of the argument it evaluates.
*/
Item *rest= thd->free_list;
thd->free_list= NULL;
for (uint idx= 0; idx < parameters->argument_count(); idx ++)
{
/*
@@ -3980,13 +4057,29 @@ sp_head::add_set_for_loop_cursor_param_variables(THD *thd,
*/
bool last= idx + 1 == parameters->argument_count();
sp_variable *spvar= param_spcont->get_context_variable(idx);
/*
The last instruction also takes the Item_func_sp of the cursor call
and anything that belongs to no argument.
*/
if (last)
{
add_to_free_list(thd, rest);
rest= NULL;
}
else
move_expr_items_to_free_list(thd, &rest, parameters->arguments()[idx]);
if (set_local_variable(thd, param_spcont,
&sp_rcontext_handler_local,
spvar, parameters->arguments()[idx],
param_lex, last,
param_lex->get_expr_str()))
{
add_to_free_list(thd, rest);
return true;
}
}
/* With no parameters, nothing took the items */
add_to_free_list(thd, rest);
return false;
}

3
sql/sp_head.h
View File

@@ -406,6 +406,9 @@ public:
int
add_instr(sp_instr_cursor_copy_struct *instr);
void move_expr_items_to_free_list(THD *thd, Item **list, Item *expr);
void add_to_free_list(THD *thd, Item *items);
bool
add_instr_jump(THD *thd, sp_pcontext *spcont);

22
sql/sql_lex.cc
View File

@@ -7224,12 +7224,34 @@ bool LEX::sp_for_loop_intrange_declarations(THD *thd, Lex_for_loop_st *loop,
my_error(ER_SP_UNDECLARED_VAR, MYF(0), item->full_name());
return true;
}
/*
Both bounds were parsed before any instruction was added, so give each
bound instruction only the items of its own bound. The left bound was
parsed first, so split there; with REVERSE the index is the right bound.
Splitting here, not in the grammar, keeps the items on THD::free_list,
so a parse error cannot leak them.
*/
Item *rest= thd->free_list;
sphead->move_expr_items_to_free_list(thd, &rest, bounds.m_direction > 0 ?
bounds.m_index->get_item() :
bounds.m_target_bound->get_item());
if (bounds.m_direction < 0)
{
Item *left= thd->free_list;
thd->free_list= NULL;
sphead->add_to_free_list(thd, rest);
rest= left;
}
if (!(loop->m_index=
bounds.m_index->sp_add_for_loop_variable(thd, index,
bounds.m_index->get_item(),
bounds.m_index->get_expr_str())
))
{
sphead->add_to_free_list(thd, rest);
return true;
}
sphead->add_to_free_list(thd, rest);
if (unlikely(!(loop->m_target_bound=
bounds.m_target_bound->
sp_add_for_loop_target_bound(thd,