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

MDEV-41228 Check the class of the copy made by Item::shallow_copy()

Item::shallow_copy() was meant to create a copy of the same class as the
item it is called on, but nothing enforced that. A class that does not
declare its own override inherits the one of its nearest concrete parent,
and get_item_copy<Parent>() then silently creates an object of the parent
class, losing the child's virtual behaviour (see MDEV-38801 for an earlier
instance with Item_cache_year).

Add the ITEM_SHALLOW_COPY(CLASS) macro, which declares the override, and
Item::dbug_check_shallow_copy_type(), a debug-only check called from
Query_arena::free_items() for every item freed. Once per dynamic type it
creates a shallow copy, unlinks it from thd->free_list again (the copy shares
pointers with the original and must not be destroyed) and asserts that the
copy has the same typeid as the original. The copy is never destroyed: its
destructor is not run, and its memory is released when the MEM_ROOT it was
allocated on, the current thd->mem_root, is freed. A comment in the check
says so.

The check found these classes copied as their parent class, now fixed with
ITEM_SHALLOW_COPY():
  Item_join_operator_plus, Item_null_result, Item_old_field, Item_datetime,
  Item_empty_string, Item_return_int, Item_trigger_type_of_statement,
  in_string::Item_string_for_in_vector, Item_is_not_null_test,
  Item_interval_DDhhmmssff_typecast, Item_string_xml_non_const,
  Item_func_trt_trx_sees_eq, Item_field_refcursor (type_cursor plugin),
  Item_field_packable_scalar (type_assoc_array plugin).

Item_field_packable_row::shallow_copy() asserted to mark the class as never
copied, which the check trips over since it copies every type it meets. It
now returns nullptr, the existing way for an item to refuse a copy.

No test is added: the check runs under every existing test in a debug build.
main and the SQL-layer suites (compat/*, json, funcs_*, vcol, gcol,
versioning, period, sys_vars, type_*) pass with it in both normal and
--ps-protocol mode.

This commit was prepared with Claude Code (Opus 5), which wrote the runtime
type check, ran the main and SQL-layer suites in normal and --ps-protocol
mode to find the classes that inherited a parent's copy method, added the
ITEM_SHALLOW_COPY overrides for them, changed Item_field_packable_row to
return nullptr, and documented where the copy made by the check is
released.
This commit is contained in:
Rex Johnston
2026-10-19 09:35:25 +13:00
parent c0aa1a1d57
commit 27d093f68e
9 changed files with 86 additions and 5 deletions

View File

@@ -244,6 +244,8 @@ public:
unpack();
return Item_field::save_in_field(field, no_conversions);
}
protected:
ITEM_SHALLOW_COPY(Item_field_packable_scalar)
};
@@ -280,11 +282,7 @@ public:
return false;
}
Item *shallow_copy(THD *thd) const override
{
DBUG_ASSERT(0);
return get_item_copy<Item_field_packable_row>(thd, this);
}
Item *shallow_copy(THD *thd) const override { return nullptr; }
static uint packed_col_length(Field *field)
{

View File

@@ -367,6 +367,8 @@ public:
DBUG_ASSERT(fc);
return fc->resolve_spvar_cursor_rowtype(thd, cursor);
}
protected:
ITEM_SHALLOW_COPY(Item_field_refcursor)
};

View File

@@ -12303,3 +12303,40 @@ bool ignored_list_includes_table(ignored_tables_list_t list, TABLE_LIST *tbl)
}
return false;
}
#ifndef DBUG_OFF
#include <typeindex>
#include <unordered_set>
#include <mutex>
void Item::dbug_check_shallow_copy_type(THD *thd) const
{
static std::unordered_set<std::type_index> checked;
static std::mutex checked_mutex;
{
std::lock_guard<std::mutex> lock(checked_mutex);
if (!checked.insert(typeid(*this)).second)
return;
}
Item *copy= shallow_copy(thd);
if (!copy)
return;
/*
get_item_copy() registered the copy in thd->free_list. Unlink it: it is
only inspected, and destroying a copy that shares pointers with the
original could free them twice. So the copy is never destroyed: its
destructor is not run, and its memory is released when the MEM_ROOT it
was allocated on, the current thd->mem_root, is freed. At most one copy
is made per Item class.
*/
if (thd->free_list == copy)
thd->free_list= copy->next;
if (typeid(*copy) != typeid(*this))
{
fprintf(stderr, "SHALLOW_COPY_TYPE_MISMATCH: %s copied as %s\n",
typeid(*this).name(), typeid(*copy).name());
DBUG_ASSERT(0);
}
}
#endif

View File

@@ -1979,6 +1979,12 @@ public:
bool check_deep_copy(THD *thd, Item *clone, const char *what);
/* walk() processor used by check_deep_copy() */
bool deep_copy_check_processor(void *arg);
/*
Check, once per dynamic type, that shallow_copy() creates an object of
the same class as this item, i.e. that the class did not inherit
shallow_copy() from a concrete parent.
*/
void dbug_check_shallow_copy_type(THD *thd) const;
#endif
/*
@@ -2922,6 +2928,15 @@ inline T* get_item_copy (THD *thd, const T* item)
return copy;
}
/*
Every instantiable Item class must declare its own shallow_copy(): one
inherited from a concrete parent creates an object of the parent class.
Item::dbug_check_shallow_copy_type() asserts that in debug builds.
*/
#define ITEM_SHALLOW_COPY(CLASS) \
Item *shallow_copy(THD *thd) const override \
{ return get_item_copy<CLASS>(thd, this); }
#ifndef DBUG_OFF
/**
@@ -4223,6 +4238,8 @@ public:
private:
uchar *saved_row_ref;
protected:
ITEM_SHALLOW_COPY(Item_old_field)
};
/**
@@ -4339,6 +4356,8 @@ public:
str->append("(+)"_LEX_CSTRING);
}
static List<Item> *make_as_item_list(THD *thd);
protected:
ITEM_SHALLOW_COPY(Item_join_operator_plus)
};
@@ -4369,6 +4388,8 @@ public:
{
return mark_unsupported_function(full_name(), arg, VCOL_IMPOSSIBLE);
}
protected:
ITEM_SHALLOW_COPY(Item_null_result)
};
/*
@@ -5054,6 +5075,8 @@ public:
return false;
}
void print(String *str, enum_query_type query_type) override;
protected:
ITEM_SHALLOW_COPY(Item_datetime)
};
@@ -5465,6 +5488,8 @@ public:
length * cs->mbmaxlen, cs)
{ }
void make_send_field(THD *thd, Send_field *field) override;
protected:
ITEM_SHALLOW_COPY(Item_empty_string)
};
@@ -5484,6 +5509,8 @@ public:
Type_handler::get_handler_by_field_type(int_field_type);
return unsigned_flag ? h->type_handler_unsigned() : h;
}
protected:
ITEM_SHALLOW_COPY(Item_return_int)
};
@@ -7989,6 +8016,8 @@ public:
private:
THD *m_thd;
active_dml_stmt m_trigger_stmt_type;
protected:
ITEM_SHALLOW_COPY(Item_trigger_type_of_statement)
};

View File

@@ -1699,6 +1699,8 @@ class in_string :public in_vector
str_value= *str;
collation.set(str->charset());
}
protected:
ITEM_SHALLOW_COPY(Item_string_for_in_vector)
};
public:
in_string(THD *thd, uint elements, qsort_cmp2 cmp_func, CHARSET_INFO *cs);
@@ -3005,6 +3007,8 @@ public:
table_map used_tables() const override
{ return used_tables_cache | RAND_TABLE_BIT; }
bool const_item() const override { return FALSE; }
protected:
ITEM_SHALLOW_COPY(Item_is_not_null_test)
};

View File

@@ -1505,6 +1505,8 @@ public:
null_value= !it.is_valid_interval_DDhhmmssff();
return it.to_string(to, m_fsp);
}
protected:
ITEM_SHALLOW_COPY(Item_interval_DDhhmmssff_typecast)
};

View File

@@ -148,6 +148,8 @@ public:
static LEX_CSTRING name= {STRING_WITH_LEN("trt_trx_sees_eq") };
return name;
}
protected:
ITEM_SHALLOW_COPY(Item_func_trt_trx_sees_eq)
};
#endif /* ITEM_VERS_INCLUDED */

View File

@@ -614,6 +614,8 @@ public:
*/
return this;
}
protected:
ITEM_SHALLOW_COPY(Item_string_xml_non_const)
};

View File

@@ -4350,6 +4350,11 @@ void Query_arena::free_items()
{
Item *next;
DBUG_ENTER("Query_arena::free_items");
#ifndef DBUG_OFF
if (THD *thd= current_thd)
for (Item *item= free_list; item; item= item->next)
item->dbug_check_shallow_copy_type(thd);
#endif
/* This works because items are allocated on THD::mem_root */
for (; free_list; free_list= next)
{