[MDEV-32225] Test case from opt_tvc.test fails with statement memory protection - Jira

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Critical
Resolution: Fixed
Affects Version/s: 10.4(EOL), 10.5, 10.6, 10.11, 11.1(EOL)
Fix Version/s: 10.4.32, 10.5.23, 10.6.16, 10.10.7, 10.11.6, 11.0.4, 11.1.3
Component/s: Optimizer
Labels:
None

Description

The following test case from opt_tvc.test fails if the server is built with-DWITH_PROTECT_STATEMENT_MEMROOT:BOOL=ON

create table t1 (a int, b int);

insert into t1

values (1,2), (4,6), (9,7), (1,1), (2,5), (7,8);

set @@in_predicate_conversion_threshold= 2;

prepare stmt from "select * from t1 where a in (1,2)";

execute stmt;

execute stmt;

The test case fails with the assertion abort that points to statement memory allocation at the second execution in this code of table_value_constr::prepare()

  Type_holder *holders;

...

  if (!(holders= new (thd->stmt_arena->mem_root) Type_holder[cnt]) ||

      join_type_handlers_for_tvc(thd, li, holders, cnt) ||

      get_type_attributes_for_tvc(thd, li, holders,

                                                  lists_of_values.elements, cnt))

Attachments

Issue Links

causes

MDEV-33477 Prepare statement with only placeholders in Table Value Constructor discards values silently

Confirmed

MDEV-35747 WITH ... AS ... prepared statements broken since 10.11.6

Confirmed

Activity

Ascending order - Click to sort in descending order

Igor Babaev (Inactive) added a comment - 2023-09-22 05:09

The following patch fixes this problem:

diff --git a/libmariadb b/libmariadb

--- a/libmariadb

+++ b/libmariadb

@@ -1 +1 @@

-Subproject commit 3393fe35d378744e12636766931cf5109cc6c2e5

+Subproject commit 3393fe35d378744e12636766931cf5109cc6c2e5-dirty

diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc

index 71377d3..13dfde6 100644

--- a/sql/sql_tvc.cc

+++ b/sql/sql_tvc.cc

@@ -227,7 +227,7 @@ bool table_value_constr::prepare(THD *thd, SELECT_LEX *sl,

   List_item *first_elem= li++;

   uint cnt= first_elem->elements;

-  Type_holder *holders;

+  Type_holder *holders= type_holders;

   if (cnt == 0)

@@ -238,32 +238,35 @@ bool table_value_constr::prepare(THD *thd, SELECT_LEX *sl,

   if (fix_fields_for_tvc(thd, li))

     DBUG_RETURN(true);

-  if (!(holders= new (thd->stmt_arena->mem_root) Type_holder[cnt]) ||

-       join_type_handlers_for_tvc(thd, li, holders, cnt) ||

-       get_type_attributes_for_tvc(thd, li, holders,

-                                  lists_of_values.elements, cnt))

-    DBUG_RETURN(true);

-  List_iterator_fast<Item> it(*first_elem);

-  Item *item;

-  Query_arena *arena, backup;

-  arena=thd->activate_stmt_arena_if_needed(&backup);

-  sl->item_list.empty();

-  for (uint pos= 0; (item= it++); pos++)

+  if (!holders)

-    /* Error's in 'new' will be detected after loop */

-    Item_type_holder *new_holder= new (thd->mem_root)

-                      Item_type_holder(thd, item, holders[pos].type_handler(),

-                                       &holders[pos]/*Type_all_attributes*/,

-                                       holders[pos].get_maybe_null());

-    sl->item_list.push_back(new_holder);

-  }

-  if (arena)

-    thd->restore_active_arena(arena, &backup);

-  if (unlikely(thd->is_fatal_error))

-    DBUG_RETURN(true); // out of memory

+    holders= type_holders= new (thd->stmt_arena->mem_root) Type_holder[cnt];

+    if (!holders ||

+         join_type_handlers_for_tvc(thd, li, holders, cnt) ||

+         get_type_attributes_for_tvc(thd, li, holders,

+                                    lists_of_values.elements, cnt))

+       DBUG_RETURN(true);

+    List_iterator_fast<Item> it(*first_elem);

+    Item *item;

+    Query_arena *arena, backup;

+    arena=thd->activate_stmt_arena_if_needed(&backup);

+    sl->item_list.empty();

+    for (uint pos= 0; (item= it++); pos++)

+    {

+      /* Error's in 'new' will be detected after loop */

+      Item_type_holder *new_holder= new (thd->mem_root)

+                        Item_type_holder(thd, item, holders[pos].type_handler(),

+                                         &holders[pos]/*Type_all_attributes*/,

+                                         holders[pos].get_maybe_null());

+      sl->item_list.push_back(new_holder);

+    }

+    if (arena)

+      thd->restore_active_arena(arena, &backup);

+    if (unlikely(thd->is_fatal_error))

+      DBUG_RETURN(true); // out of memory

-  if (arena)

-    thd->restore_active_arena(arena, &backup);

-  if (unlikely(thd->is_fatal_error))

-    DBUG_RETURN(true); // out of memory

   result= tmp_result;

diff --git a/sql/sql_tvc.h b/sql/sql_tvc.h

index 594a77a..cdd5f52 100644

--- a/sql/sql_tvc.h

+++ b/sql/sql_tvc.h

@@ -24,6 +24,7 @@ class Explain_query;

 class Item_func_in;

 class st_select_lex_unit;

 typedef class st_select_lex SELECT_LEX;

+class Type_holder;

/**

   @class table_value_constr

@@ -38,6 +39,7 @@ class table_value_constr : public Sql_alloc

   List<List_item> lists_of_values;

   select_result *result;

   SELECT_LEX *select_lex;

+  Type_holder *type_holders;

   enum { QEP_NOT_PRESENT_YET, QEP_AVAILABLE} have_query_plan;

@@ -46,7 +48,7 @@ class table_value_constr : public Sql_alloc

   table_value_constr(List<List_item> tvc_values, SELECT_LEX *sl,

                     ulonglong select_options_arg) :

-    lists_of_values(tvc_values), result(0), select_lex(sl),

+    lists_of_values(tvc_values), result(0), select_lex(sl), type_holders(0),

     have_query_plan(QEP_NOT_PRESENT_YET), explain(0),

     select_options(select_options_arg)

   { };

Igor Babaev (Inactive) added a comment - 2023-09-22 05:09 The following patch fixes this problem: diff --git a/libmariadb b/libmariadb --- a/libmariadb +++ b/libmariadb @@ -1 +1 @@ -Subproject commit 3393fe35d378744e12636766931cf5109cc6c2e5 +Subproject commit 3393fe35d378744e12636766931cf5109cc6c2e5-dirty diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc index 71377d3..13dfde6 100644 --- a/sql/sql_tvc.cc +++ b/sql/sql_tvc.cc @@ -227,7 +227,7 @@ bool table_value_constr::prepare(THD *thd, SELECT_LEX *sl, List_item *first_elem= li++; uint cnt= first_elem->elements; - Type_holder *holders; + Type_holder *holders= type_holders; if (cnt == 0) { @@ -238,32 +238,35 @@ bool table_value_constr::prepare(THD *thd, SELECT_LEX *sl, if (fix_fields_for_tvc(thd, li)) DBUG_RETURN(true); - if (!(holders= new (thd->stmt_arena->mem_root) Type_holder[cnt]) || - join_type_handlers_for_tvc(thd, li, holders, cnt) || - get_type_attributes_for_tvc(thd, li, holders, - lists_of_values.elements, cnt)) - DBUG_RETURN(true); - - List_iterator_fast<Item> it(*first_elem); - Item *item; - Query_arena *arena, backup; - arena=thd->activate_stmt_arena_if_needed(&backup); - - sl->item_list.empty(); - for (uint pos= 0; (item= it++); pos++) + if (!holders) { - /* Error's in 'new' will be detected after loop */ - Item_type_holder *new_holder= new (thd->mem_root) - Item_type_holder(thd, item, holders[pos].type_handler(), - &holders[pos]/*Type_all_attributes*/, - holders[pos].get_maybe_null()); - sl->item_list.push_back(new_holder); - } - if (arena) - thd->restore_active_arena(arena, &backup); - - if (unlikely(thd->is_fatal_error)) - DBUG_RETURN(true); // out of memory + holders= type_holders= new (thd->stmt_arena->mem_root) Type_holder[cnt]; + if (!holders || + join_type_handlers_for_tvc(thd, li, holders, cnt) || + get_type_attributes_for_tvc(thd, li, holders, + lists_of_values.elements, cnt)) + DBUG_RETURN(true); + List_iterator_fast<Item> it(*first_elem); + Item *item; + Query_arena *arena, backup; + arena=thd->activate_stmt_arena_if_needed(&backup); + + sl->item_list.empty(); + for (uint pos= 0; (item= it++); pos++) + { + /* Error's in 'new' will be detected after loop */ + Item_type_holder *new_holder= new (thd->mem_root) + Item_type_holder(thd, item, holders[pos].type_handler(), + &holders[pos]/*Type_all_attributes*/, + holders[pos].get_maybe_null()); + sl->item_list.push_back(new_holder); + } + if (arena) + thd->restore_active_arena(arena, &backup); + + if (unlikely(thd->is_fatal_error)) + DBUG_RETURN(true); // out of memory } - if (arena) - thd->restore_active_arena(arena, &backup); - - if (unlikely(thd->is_fatal_error)) - DBUG_RETURN(true); // out of memory result= tmp_result; diff --git a/sql/sql_tvc.h b/sql/sql_tvc.h index 594a77a..cdd5f52 100644 --- a/sql/sql_tvc.h +++ b/sql/sql_tvc.h @@ -24,6 +24,7 @@ class Explain_query; class Item_func_in; class st_select_lex_unit; typedef class st_select_lex SELECT_LEX; +class Type_holder; /** @class table_value_constr @@ -38,6 +39,7 @@ class table_value_constr : public Sql_alloc List<List_item> lists_of_values; select_result *result; SELECT_LEX *select_lex; + Type_holder *type_holders; enum { QEP_NOT_PRESENT_YET, QEP_AVAILABLE} have_query_plan; @@ -46,7 +48,7 @@ class table_value_constr : public Sql_alloc table_value_constr(List<List_item> tvc_values, SELECT_LEX *sl, ulonglong select_options_arg) : - lists_of_values(tvc_values), result(0), select_lex(sl), + lists_of_values(tvc_values), result(0), select_lex(sl), type_holders(0), have_query_plan(QEP_NOT_PRESENT_YET), explain(0), select_options(select_options_arg) { };

Igor Babaev (Inactive) added a comment - 2023-09-22 05:11

@sanja: please review the patch in my last comment

Igor Babaev (Inactive) added a comment - 2023-09-22 05:11 @sanja: please review the patch in my last comment

Oleksandr Byelkin added a comment - 2023-09-22 06:48

OK to push

Oleksandr Byelkin added a comment - 2023-09-22 06:48 OK to push

Igor Babaev (Inactive) added a comment - 2023-09-26 05:12

A fix for this bug was pushed into 10.4

Igor Babaev (Inactive) added a comment - 2023-09-26 05:12 A fix for this bug was pushed into 10.4

MariaDB Server

Test case from opt_tvc.test fails with statement memory protection

Details

Description

Attachments

Issue Links

Activity

People

Dates

Git Integration