Re: [Maria-developers] [Commits] 475cab8: MDEV-10050: Crash in subselect
Hi Sanja, On Wed, Jun 22, 2016 at 02:17:06PM +0200, Oleksandr Byelkin wrote:
revision-id: 475cab835fb48c91d5cca649ab93917ec1718d75 (mariadb-5.5.50-6-g475cab8) parent(s): a482e76e65a4fee70479e877929381c86b1ec62f committer: Oleksandr Byelkin timestamp: 2016-06-22 14:17:06 +0200 message:
MDEV-10050: Crash in subselect
thd should not be taken earlier then fix_field and reset on fix_fields if it is needed.
--- sql/item_subselect.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index ba67474..60cdd3f 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -79,7 +79,9 @@ void Item_subselect::init(st_select_lex *select_lex, DBUG_PRINT("enter", ("select_lex: 0x%lx this: 0x%lx", (ulong) select_lex, (ulong) this)); unit= select_lex->master_unit(); - thd= unit->thd; +#ifndef DBUG_OFF + thd= 0; +#endif
So I've applied the patch, and I'm debugging this statement: prepare s from 'select a, (select max(one_k.a) from one_k where one_k.a <ten.a) from ten'; (this is just the first subquery I tried, nothing special about it). We arrive at the above #ifndef, and I have thd=0xa5a5a5a5a5, that is, it's uninitialized data. I let it execute further...
if (unit->item) { /* Item can be changed in JOIN::prepare while engine in JOIN::optimize => we do not copy old_engine here */ engine= unit->item->engine; own_engine= FALSE; parsing_place= unit->item->parsing_place; unit->thd->change_item_tree((Item**)&unit->item, this); engine->change_result(this, result, TRUE); } else { SELECT_LEX *outer_select= unit->outer_select(); /* do not take into account expression inside aggregate functions because they can access original table fields */ parsing_place= (outer_select->in_sum_expr ? NO_MATTER : outer_select->parsing_place); if (unit->is_union()) engine= new subselect_union_engine(thd, unit, result, this); else engine= new subselect_single_select_engine(thd, select_lex, result, this);
and it arrives here. Stepping into this call, I eventually reach where this stack trace shows: (gdb) wher #0 select_result::set_thd (this=0x7fffb002c130, thd_arg=0x0) at /home/psergey/dev-git/5.5/sql/sql_class.h:3316 #1 0x000000000086fd30 in subselect_engine::set_thd (this=0x7fffb002c150, thd_arg=0x0) at /home/psergey/dev-git/5.5/sql/item_subselect.cc:2892 #2 0x00000000007648e9 in subselect_engine::subselect_engine (this=0x7fffb002c150, thd_arg=0x0, si=0x7fffb002c010, res=0x7fffb002c130) at /home/psergey/dev-git/5.5/sql/item_subselect.h:726 #3 0x000000000086fd66 in subselect_single_select_engine::subselect_single_select_engine (this=0x7fffb002c150, thd_arg=0x0, select=0x7fffb002a8e0, result_arg=0x7fffb002c130, item_arg=0x7fffb002c010) at /home/psergey/dev-git/5.5/sql/item_subselect.cc:2902 #4 0x0000000000868232 in Item_subselect::init (this=0x7fffb002c010, select_lex=0x7fffb002a8e0, result=0x7fffb002c130) at /home/psergey/dev-git/5.5/sql/item_subselect.cc:111 #5 0x000000000086a0d1 in Item_singlerow_subselect::Item_singlerow_subselect (this=0x7fffb002c010, select_lex=0x7fffb002a8e0) at /home/psergey/dev-git/5.5/sql/item_subselect.cc:906 #6 0x000000000077d36f in MYSQLparse (thd=0x2fd99e0) at /home/psergey/dev-git/5.5/sql/sql_yacc.yy:8366 #7 0x00000000006214f2 in parse_sql (thd=0x2fd99e0, parser_state=0x7ffff7f81800, creation_ctx=0x0) at /home/psergey/dev-git/5.5/sql/sql_parse.cc:7818 #8 0x0000000000633dda in Prepared_statement::prepare (this=0x7fffb00167f0, packet=0x7fffb00063f8 "select a, (select max(one_k.a) from one_k where one_k.a <ten.a) from ten", packet_len=72) at /home/psergey/dev-git/5.5/sql/sql_prepare.cc:3353 #9 0x0000000000631a89 in mysql_sql_stmt_prepare (thd=0x2fd99e0) at /home/psergey/dev-git/5.5/sql/sql_prepare.cc:2458 #10 0x000000000061324d in mysql_execute_command (thd=0x2fd99e0) at /home/psergey/dev-git/5.5/sql/sql_parse.cc:2239 #11 0x000000000061d58d in mysql_parse (thd=0x2fd99e0, rawbuf=0x7fffb00062c8 "prepare s from 'select a, (select max(one_k.a) from one_k where one_k.a <ten.a) from ten'", length=89, parser_state=0x7ffff7f82540) at /home/psergey/dev-git/5.5/sql/sql_parse.cc:5934 Note that thd_arg is passing around un-initialized data all of this time. Is this how it is intended to work? This looks scary, a lot of functions are passed garbage as parameter. I assume the test suite passed with this patch, so these function don't do anything with the parameter (they probably save away the THD pointer, and then somebody assigns the pointer a valid value before it is used). Still, this looks really scary, we need to either fix this (strongly preferable), or add a lot of comments about it (acceptable as a last resort measure).
if (unit->item) { @@ -90,7 +92,7 @@ void Item_subselect::init(st_select_lex *select_lex, engine= unit->item->engine; own_engine= FALSE; parsing_place= unit->item->parsing_place; - thd->change_item_tree((Item**)&unit->item, this); + unit->thd->change_item_tree((Item**)&unit->item, this); engine->change_result(this, result, TRUE); } else @@ -220,6 +222,10 @@ bool Item_subselect::fix_fields(THD *thd_param, Item **ref) uint8 uncacheable; bool res;
+ thd= thd_param; + + DBUG_ASSERT(unit->thd == thd); + status_var_increment(thd_param->status_var.feature_subquery);
DBUG_ASSERT(fixed == 0); @@ -2651,7 +2657,10 @@ bool Item_in_subselect::fix_fields(THD *thd_arg, Item **ref) { uint outer_cols_num; List<Item> *inner_cols; - char const *save_where= thd->where; + char const *save_where= thd_arg->where; + + thd= thd_arg; + DBUG_ASSERT(unit->thd == thd);
if (test_strategy(SUBS_SEMI_JOIN)) return !( (*ref)= new Item_int(1)); _______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
-- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog
Hi! On 23.06.2016 23:49, Sergey Petrunia wrote:
Hi Sanja,
On Wed, Jun 22, 2016 at 02:17:06PM +0200, Oleksandr Byelkin wrote:
revision-id: 475cab835fb48c91d5cca649ab93917ec1718d75 (mariadb-5.5.50-6-g475cab8) parent(s): a482e76e65a4fee70479e877929381c86b1ec62f committer: Oleksandr Byelkin timestamp: 2016-06-22 14:17:06 +0200 message:
MDEV-10050: Crash in subselect
thd should not be taken earlier then fix_field and reset on fix_fields if it is needed.
--- sql/item_subselect.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index ba67474..60cdd3f 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -79,7 +79,9 @@ void Item_subselect::init(st_select_lex *select_lex, DBUG_PRINT("enter", ("select_lex: 0x%lx this: 0x%lx", (ulong) select_lex, (ulong) this)); unit= select_lex->master_unit(); - thd= unit->thd; +#ifndef DBUG_OFF + thd= 0; +#endif So I've applied the patch, and I'm debugging this statement:
prepare s from 'select a, (select max(one_k.a) from one_k where one_k.a <ten.a) from ten';
(this is just the first subquery I tried, nothing special about it).
We arrive at the above #ifndef, and I have thd=0xa5a5a5a5a5, that is, it's uninitialized data.
I let it execute further...
You are absolutely right, I forgot that engines also took thd on cration instead of prepare. Now it is fixed. revision-id: 79f852a069fb6ba5e18fd66ea2a24fa91c245c24 (mariadb-5.5.50-7-g79f852a) parent(s): ef92aaf9ece92c873ae0f3448ab2274c958ba3fe committer: Oleksandr Byelkin timestamp: 2016-06-24 14:15:35 +0200 message: MDEV-10050: Crash in subselect thd should not be taken earlier then fix_field and reset on fix_fields if it is needed. [skip]
participants (2)
-
Oleksandr Byelkin
-
Sergey Petrunia